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

Improve highlighting for no-extra-arguments #178

Closed
andrey-tyukin-sonarsource opened this issue Sep 4, 2020 · 0 comments · Fixed by #250
Closed

Improve highlighting for no-extra-arguments #178

andrey-tyukin-sonarsource opened this issue Sep 4, 2020 · 0 comments · Fixed by #250
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@andrey-tyukin-sonarsource
Copy link

andrey-tyukin-sonarsource commented Sep 4, 2020

Highlighting of function invocations with too many arguments can be counter-intuitive if the function call stretches over multiple lines.

Minimal Example:

    function functionName(a, b){}
//           ^^^^^^^^^^^^^^^^^^             Secondary location is the function declaration.

    functionName(
      arg1,
      arg2,
      arg3,
      arg4
    );
//  ^ Primary issue location at the end of the call expression 
//     (the closing parenthesis is the last token of the call expression).

A real-world example is described and discussed here.

The proposed solution is to change the primary location and introduce more secondary locations, as follows:

  • Primary location should be moved to the callee, so that the main error message appears before the opening parenthesis.
  • Every superfluous argument should be reported with a secondary location.
  • If the callee is a single identifier, then this identifier should be used in the error message: "The function {identifier} expects {N} arguments, but {M} were provided."; That is, if the call looks like f(x), then f should be the primary location.
  • If the callee is a member expression, then the method name should be used in the same message template. That is, if the call looks like expr.methodName(a1, ..., aN), then expr.methodName should be the primary location, and methodName should be used in the message.
  • In all other cases (where no good name can be found), the original message "This function expects {N} arguments, but {M} were provided." should be preserved.

For the example above, it would mean:

    function functionName(a, b){}
//           ^^^^^^^^^^^^^^^^^^             Secondary location at the function declaration.

    functionName(
//  ^^^^^^^^^^^^  primary location at the callee
      arg1,
      arg2,
      arg3,
 //   ^^^^ secondary location at superfluous argument
      arg4
 //   ^^^^ secondary location at superfluous argument
    );
@andrey-tyukin-sonarsource andrey-tyukin-sonarsource added the enhancement New feature or request label Sep 4, 2020
@andrey-tyukin-sonarsource andrey-tyukin-sonarsource changed the title Rule S930: highlighting of secondary locations should be improved for no-extra-arguments Rule S930: improve highlighting for no-extra-arguments Sep 4, 2020
@yassin-kammoun-sonarsource yassin-kammoun-sonarsource added this to the 0.9.0 milestone Jun 30, 2021
@vilchik-elena vilchik-elena changed the title Rule S930: improve highlighting for no-extra-arguments Improve highlighting for no-extra-arguments Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants