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

eslint · Apr 23, 2020 · 5c1bdeb · 5c1bdeb
1 parent 56d2bee
commit 5c1bdeb
Show file tree

Hide file tree

Showing 2 changed files with 117 additions and 18 deletions.
diff --git a/lib/rules/getter-return.js b/lib/rules/getter-return.js
@@ -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
 //------------------------------------------------------------------------------
@@ -75,6 +64,7 @@ module.exports = {
     create(context) {
 
         const options = context.options[0] || { allowImplicit: false };
+        const sourceCode = context.getSourceCode();
 
         let funcInfo = {
             upper: null,
@@ -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)

diff --git a/tests/lib/rules/getter-return.js b/tests/lib/rules/getter-return.js
@@ -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] },
@@ -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] },
 
@@ -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" }] },