Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Improve report location for getter-return (refs #12334) #13164

Merged
merged 1 commit into from Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 2 additions & 12 deletions lib/rules/getter-return.js
Expand Up @@ -25,17 +25,6 @@ function isReachable(segment) {
return segment.reachable;
}

/**
* Gets a readable location.
*
* - FunctionExpression -> the function name or `function` keyword.
* @param {ASTNode} node A function node to get.
* @returns {ASTNode|Token} The node or the token of a location.
*/
function getId(node) {
return node.id || node;
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -75,6 +64,7 @@ module.exports = {
create(context) {

const options = context.options[0] || { allowImplicit: false };
const sourceCode = context.getSourceCode();

let funcInfo = {
upper: null,
Expand All @@ -99,7 +89,7 @@ module.exports = {
) {
context.report({
node,
loc: getId(node).loc.start,
loc: astUtils.getFunctionHeadLoc(node, sourceCode),
messageId: funcInfo.hasReturn ? "expectedAlways" : "expected",
data: {
name: astUtils.getFunctionNameWithKind(funcInfo.node)
Expand Down
121 changes: 115 additions & 6 deletions tests/lib/rules/getter-return.js
Expand Up @@ -81,9 +81,56 @@ ruleTester.run("getter-return", rule, {
* test obj: get
* option: {allowImplicit: false}
*/
{ code: "var foo = { get bar() {} };", errors: [expectedError] },
{ code: "var foo = { get bar(){if(baz) {return true;}} };", errors: [expectedAlwaysError] },
{ code: "var foo = { get bar() { ~function () {return true;}} };", errors: [expectedError] },
{
code: "var foo = { get bar() {} };",
errors: [{
...expectedError,
line: 1,
column: 13,
endLine: 1,
endColumn: 20
}]
},
{
code: "var foo = { get\n bar () {} };",
errors: [{
...expectedError,
line: 1,
column: 13,
endLine: 2,
endColumn: 6
}]
},
{
code: "var foo = { get bar(){if(baz) {return true;}} };",
errors: [{
...expectedAlwaysError,
line: 1,
column: 13,
endLine: 1,
endColumn: 20
}]
},
{
code: "var foo = { get bar() { ~function () {return true;}} };",
errors: [{
...expectedError,
line: 1,
column: 13,
endLine: 1,
endColumn: 20
}]
},
{
code: "var foo = { get bar() { return; } };",
errors: [{
...expectedError,
line: 1,
column: 25,
endLine: 1,
endColumn: 32
}]
},

// option: {allowImplicit: true}
{ code: "var foo = { get bar() {} };", options, errors: [expectedError] },
Expand All @@ -93,7 +140,27 @@ ruleTester.run("getter-return", rule, {
* test class: get
* option: {allowImplicit: false}
*/
{ code: "class foo { get bar(){} }", errors: [expectedError] },
{
code: "class foo { get bar(){} }",
errors: [{
...expectedError,
line: 1,
column: 13,
endLine: 1,
endColumn: 20
}]
},
{
code: "var foo = class {\n static get\nbar(){} }",
errors: [{
messageId: "expected",
data: { name: "static getter 'bar'" },
line: 2,
column: 3,
endLine: 3,
endColumn: 4
}]
},
{ code: "class foo { get bar(){ if (baz) { return true; }}}", errors: [expectedAlwaysError] },
{ code: "class foo { get bar(){ ~function () { return true; }()}}", errors: [expectedError] },

Expand All @@ -105,8 +172,50 @@ ruleTester.run("getter-return", rule, {
* test object.defineProperty(s)
* option: {allowImplicit: false}
*/
{ code: "Object.defineProperty(foo, \"bar\", { get: function (){}});", errors: [{ messageId: "expected", data: { name: "method 'get'" } }] },
{ code: "Object.defineProperty(foo, \"bar\", { get: () => {}});", errors: [{ messageId: "expected", data: { name: "arrow function 'get'" } }] },
{
code: "Object.defineProperty(foo, 'bar', { get: function (){}});",
errors: [{
messageId: "expected",
data: { name: "method 'get'" },
line: 1,
column: 37,
endLine: 1,
endColumn: 51
}]
},
{
code: "Object.defineProperty(foo, 'bar', { get: function getfoo (){}});",
errors: [{
messageId: "expected",
data: { name: "method 'getfoo'" },
line: 1,
column: 37,
endLine: 1,
endColumn: 58
}]
},
{
code: "Object.defineProperty(foo, 'bar', { get(){} });",
errors: [{
messageId: "expected",
data: { name: "method 'get'" },
line: 1,
column: 37,
endLine: 1,
endColumn: 40
}]
},
{
code: "Object.defineProperty(foo, 'bar', { get: () => {}});",
errors: [{
messageId: "expected",
data: { name: "arrow function 'get'" },
line: 1,
column: 45,
endLine: 1,
endColumn: 47
}]
},
{ code: "Object.defineProperty(foo, \"bar\", { get: function (){if(bar) {return true;}}});", errors: [{ messageId: "expectedAlways" }] },
{ code: "Object.defineProperty(foo, \"bar\", { get: function (){ ~function () { return true; }()}});", errors: [{ messageId: "expected" }] },
{ code: "Object.defineProperties(foo, { bar: { get: function () {}} });", options, errors: [{ messageId: "expected" }] },
Expand Down