Rule S930: improve highlighting for no-extra-arguments (#250)

Co-authored-by: Daniel Graf Hoyos <[email protected]>

Author: yassin-kammoun-sonarsource

src/rules/no-extra-arguments.ts

@@ -32,6 +32,7 @@ import {
   issueLocation,
   getMainFunctionTokenLocation,
   IssueLocation,
+  toSecondaryLocation,
 } from '../utils/locations';
 import { Rule } from '../utils/types';
 import docsUrl from '../utils/docs-url';
@@ -171,30 +172,35 @@ const rule: Rule.RuleModule = {
         context,
         {
           message,
-          node: callExpr,
+          node: callExpr.callee,
         },
-        getSecondaryLocations(functionNode),
+        getSecondaryLocations(callExpr, functionNode),
       );
     }
 
-    function getSecondaryLocations(functionNode: TSESTree.FunctionLike) {
+    function getSecondaryLocations(
+      callExpr: TSESTree.CallExpression,
+      functionNode: TSESTree.FunctionLike,
+    ) {
       const paramLength = functionNode.params.length;
       const secondaryLocations: IssueLocation[] = [];
       if (paramLength > 0) {
         const startLoc = functionNode.params[0].loc;
         const endLoc = functionNode.params[paramLength - 1].loc;
-        // defensive check as `loc` property may be undefined according to
-        // its type declaration
-        if (startLoc && endLoc) {
-          secondaryLocations.push(issueLocation(startLoc, endLoc, 'Formal parameters'));
-        }
+        secondaryLocations.push(issueLocation(startLoc, endLoc, 'Formal parameters'));
       } else {
         // as we're not providing parent node, `getMainFunctionTokenLocation` may return `undefined`
         const fnToken = getMainFunctionTokenLocation(functionNode, undefined, context);
         if (fnToken) {
           secondaryLocations.push(issueLocation(fnToken, fnToken, 'Formal parameters'));
         }
       }
+      // find actual extra arguments to highlight
+      callExpr.arguments.forEach((argument, index) => {
+        if (index >= paramLength) {
+          secondaryLocations.push(toSecondaryLocation(argument, 'Extra argument'));
+        }
+      });
       return secondaryLocations;
     }
   },

src/utils/locations.ts

@@ -128,11 +128,11 @@ export function toEncodedMessage(
   return JSON.stringify(encodedMessage);
 }
 
-function toSecondaryLocation(
+export function toSecondaryLocation(
   locationHolder: TSESLint.AST.Token | TSESTree.Node,
   message?: string,
 ): IssueLocation {
-  const loc = locationHolder.loc!;
+  const loc = locationHolder.loc;
   return {
     message,
     column: loc.start.column,

tests/rules/no-extra-arguments.test.ts

@@ -83,21 +83,22 @@ ruleTester.run('no-extra-arguments', rule, {
         foo(1, 2, 3, 4);
       `,
       errors: [
-        message(2, 3, { line: 3, column: 9, endColumn: 21 }),
-        message(2, 4, { line: 4, column: 9, endColumn: 24 }),
+        message(2, 3, { line: 3, column: 9, endColumn: 12 }),
+        message(2, 4, { line: 4, column: 9, endColumn: 12 }),
       ],
     },
     {
       code: `
         function foo(p1, p2) {}
         //           ^^^^^^>
         foo(1, 2, 3);
-      //^^^^^^^^^^^^
+      //^^^      <^
       `,
       errors: [
         {
           message: encodedMessage(2, 3, [
             { line: 2, column: 21, endLine: 2, endColumn: 27, message: 'Formal parameters' },
+            { message: 'Extra argument', column: 18, line: 4, endColumn: 19, endLine: 4 },
           ]),
         },
       ],
@@ -110,12 +111,12 @@ ruleTester.run('no-extra-arguments', rule, {
         }
         foo(1);
       `,
-      errors: [message(0, 1, { line: 5, column: 9, endColumn: 15 })],
+      errors: [message(0, 1, { line: 5, column: 9, endColumn: 12 })],
     },
     {
       code: `
         foo(1);
-      //^^^^^^
+      //    ^
         var foo = function() {
           //      ^^^^^^^^>
           console.log('hello');
@@ -125,43 +126,43 @@ ruleTester.run('no-extra-arguments', rule, {
         {
           message: encodedMessage(0, 1, [
             { line: 4, column: 18, endLine: 4, endColumn: 26, message: 'Formal parameters' },
+            { message: 'Extra argument', column: 12, line: 2, endColumn: 13, endLine: 2 },
           ]),
         },
       ],
       options: ['sonar-runtime'],
     },
-
     {
       code: `
         (function(p1, p2){
           doSomething1;
           doSomething2;
         })(1, 2, 3);
       `,
-      errors: [message(2, 3, { line: 2, column: 9, endLine: 5, endColumn: 20 })],
+      errors: [message(2, 3, { line: 2, column: 10, endLine: 5, endColumn: 10 })],
     },
     {
       code: `
         let x = function(a, b) {
           return a + b;
         }(1, 2, 3);
       `,
-      errors: [message(2, 3, { line: 2, column: 17, endLine: 4, endColumn: 19 })],
+      errors: [message(2, 3, { line: 2, column: 17, endLine: 4, endColumn: 10 })],
     },
     {
       code: `
         ((a, b) => {
           return a + b;
         })(1, 2, 3);
       `,
-      errors: [message(2, 3, { line: 2, column: 9, endLine: 4, endColumn: 20 })],
+      errors: [message(2, 3, { line: 2, column: 10, endLine: 4, endColumn: 10 })],
     },
     {
       code: `
         let arrow_function = (a, b) => {};
         arrow_function(1, 2, 3);
       `,
-      errors: [message(2, 3, { line: 3, column: 9, endColumn: 32 })],
+      errors: [message(2, 3, { line: 3, column: 9, endColumn: 23 })],
     },
   ],
 });
@@ -176,7 +177,7 @@ ruleTesterScript.run('no-extra-arguments script', rule, {
         }
         foo(1, 2);
       `,
-      errors: [message(1, 2, { line: 5, column: 9, endColumn: 18 })],
+      errors: [message(1, 2, { line: 5, column: 9, endColumn: 12 })],
     },
     {
       code: `
@@ -186,7 +187,7 @@ ruleTesterScript.run('no-extra-arguments script', rule, {
         }
         foo(1, 2);
       `,
-      errors: [message(0, 2, { line: 6, column: 9, endColumn: 18 })],
+      errors: [message(0, 2, { line: 6, column: 9, endColumn: 12 })],
     },
   ],
 });