no-extra-arguments: provide secondary issue locations (#97)

Author: andrea-guarino-sonarsource

src/rules/no-extra-arguments.ts

@@ -28,8 +28,17 @@ import {
   isIdentifier,
   isBlockStatement,
 } from "../utils/nodes";
+import { report, issueLocation, getMainFunctionTokenLocation, IssueLocation } from "../utils/locations";
 
 const rule: Rule.RuleModule = {
+  meta: {
+    schema: [
+      {
+        // internal parameter
+        enum: ["sonar-runtime"],
+      },
+    ],
+  },
   create(context: Rule.RuleContext) {
     const callExpressionsToCheck: Array<{
       callExpr: estree.SimpleCallExpression;
@@ -78,7 +87,7 @@ const rule: Rule.RuleModule = {
       "Program:exit"() {
         callExpressionsToCheck.forEach(({ callExpr, functionNode }) => {
           if (!usingArguments.has(functionNode) && !emptyFunctions.has(functionNode)) {
-            report(callExpr, functionNode.params.length, callExpr.arguments.length);
+            reportIssue(callExpr, functionNode);
           }
         });
       },
@@ -118,22 +127,52 @@ const rule: Rule.RuleModule = {
       }
     }
 
-    function report(callExpr: estree.SimpleCallExpression, expected: number, provided: number) {
+    function reportIssue(callExpr: estree.SimpleCallExpression, functionNode: estree.Function) {
+      const paramLength = functionNode.params.length;
+      const argsLength = callExpr.arguments.length;
       // prettier-ignore
       const expectedArguments = 
-        expected === 0 ? "no arguments" : 
-        expected === 1 ? "1 argument" : 
-        `${expected} arguments`;
+        paramLength === 0 ? "no arguments" : 
+        paramLength === 1 ? "1 argument" : 
+        `${paramLength} arguments`;
 
       // prettier-ignore
       const providedArguments = 
-        provided === 0 ? "none was" : 
-        provided === 1 ? "1 was" : 
-        `${provided} were`;
+        argsLength === 0 ? "none was" : 
+        argsLength === 1 ? "1 was" : 
+        `${argsLength} were`;
 
       const message = `This function expects ${expectedArguments}, but ${providedArguments} provided.`;
 
-      context.report({ message, node: callExpr });
+      report(
+        context,
+        {
+          message,
+          node: callExpr,
+        },
+        getSecondaryLocations(functionNode),
+      );
+    }
+
+    function getSecondaryLocations(functionNode: estree.Function) {
+      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"));
+        }
+      } 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"));
+        }
+      }
+      return secondaryLocations;
     }
   },
 };

src/utils/locations.ts

@@ -20,6 +20,20 @@
 import { Rule } from "eslint";
 import * as estree from "estree";
 
+export interface IssueLocation {
+  column: number;
+  line: number;
+  endColumn: number;
+  endLine: number;
+  message?: string;
+}
+
+export interface EncodedMessage {
+  message: string;
+  cost?: number;
+  secondaryLocations: IssueLocation[];
+}
+
 /**
  * Returns a location of the "main" function token:
  * - function name for a function declaration, method or accessor
@@ -61,6 +75,46 @@ export function getMainFunctionTokenLocation(
   return location!;
 }
 
+// As `ReportDescriptor` may contain either `message` or `messageId` prop,
+// we force the presence of `message` property by using the following type alias.
+export type ReportDescriptor = Rule.ReportDescriptor & { message: string };
+
+/**
+ * Wrapper for `context.report`, supporting secondary locations and cost.
+ * Encode those extra information in the issue message when rule is executed
+ * in Sonar* environment.
+ */
+export function report(
+  context: Rule.RuleContext,
+  reportDescriptor: ReportDescriptor,
+  secondaryLocations: IssueLocation[] = [],
+  cost?: number,
+) {
+  const { message } = reportDescriptor;
+  if (context.options[context.options.length - 1] === "sonar-runtime") {
+    const encodedMessage: EncodedMessage = { secondaryLocations, message, cost };
+    reportDescriptor.message = JSON.stringify(encodedMessage);
+  }
+  context.report(reportDescriptor);
+}
+
+/**
+ * Converts `SourceLocation` range into `IssueLocation`
+ */
+export function issueLocation(
+  startLoc: estree.SourceLocation,
+  endLoc: estree.SourceLocation = startLoc,
+  message = "",
+): IssueLocation {
+  return {
+    line: startLoc.start.line,
+    column: startLoc.start.column,
+    endLine: endLoc.end.line,
+    endColumn: endLoc.end.column,
+    message,
+  };
+}
+
 function getTokenByValue(node: estree.Node, value: string, context: Rule.RuleContext) {
   return context
     .getSourceCode()

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

@@ -18,6 +18,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 import { RuleTester } from "eslint";
+import { IssueLocation } from "../../src/utils/locations";
 
 const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
 import rule = require("../../src/rules/no-extra-arguments");
@@ -87,6 +88,18 @@ ruleTester.run("no-extra-arguments", rule, {
         message(2, 4, { line: 4, column: 9, endColumn: 24 }),
       ],
     },
+    {
+      code: `
+        function foo(p1, p2) {}
+        //           ^^^^^^>
+        foo(1, 2, 3);
+      //^^^^^^^^^^^^  
+      `,
+      errors: [
+        encodedMessage(2, 3, [{ line: 2, column: 21, endLine: 2, endColumn: 27, message: "Formal parameters" }]),
+      ],
+      options: ["sonar-runtime"],
+    },
     {
       code: `
         var foo = function() {
@@ -96,6 +109,20 @@ ruleTester.run("no-extra-arguments", rule, {
       `,
       errors: [message(0, 1, { line: 5, column: 9, endColumn: 15 })],
     },
+    {
+      code: `
+        foo(1);
+      //^^^^^^
+        var foo = function() {
+          //      ^^^^^^^^>
+          console.log('hello');
+        }
+      `,
+      errors: [
+        encodedMessage(0, 1, [{ line: 4, column: 18, endLine: 4, endColumn: 26, message: "Formal parameters" }]),
+      ],
+      options: ["sonar-runtime"],
+    },
     {
       code: `
         function foo(arguments) {
@@ -172,3 +199,7 @@ function message(
     ...extra,
   };
 }
+
+function encodedMessage(expected: number, provided: number, secondaryLocations: IssueLocation[]) {
+  return JSON.stringify({ secondaryLocations, message: message(expected, provided).message });
+}