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
Rule S930: improve highlighting for no-extra-arguments #250
Conversation
Actual Excess parameters are now highlighted as secondary locations. The primary location is therefore reduced to the callee expression.
src/rules/no-extra-arguments.ts
Outdated
// find actual extra arguments to highlight | ||
callExpr.arguments.forEach((argument, index) => { | ||
if (index >= paramLength) { | ||
secondaryLocations.push(issueLocation(argument.loc, argument.loc, 'Extra argument')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toSecondaryLocation
seems better here
]), | ||
}, | ||
], | ||
options: ['sonar-runtime'], | ||
}, | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why those 2 new tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contributor probably wanted to demonstrate that the rule also works with the special arguments
object. Since it's unrelated to the ticket, I will drop those.
], | ||
}, | ||
{ | ||
code: ` | ||
function foo(p1, p2) {} | ||
// ^^^^^^> | ||
foo(1, 2, 3); | ||
//^^^^^^^^^^^^ | ||
//^^^ ^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be //^^^ <^
to show that second piece is a secondary location
Kudos, SonarCloud Quality Gate passed! |
Fixes #178
The ticket mentions customising the message of the primary location with the function name depending on the callee type, which this user contribution does not include. I think we don't need to go that far as the location-based highlighting of an issue would be enough.