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

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

Merged
merged 6 commits into from Jul 1, 2021

Conversation

yassin-kammoun-sonarsource
Copy link
Contributor

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.

Loxos and others added 3 commits July 1, 2021 11:06
Actual Excess parameters are now highlighted as secondary locations. The primary location is therefore reduced to the callee expression.
// find actual extra arguments to highlight
callExpr.arguments.forEach((argument, index) => {
if (index >= paramLength) {
secondaryLocations.push(issueLocation(argument.loc, argument.loc, 'Extra argument'));
Copy link
Contributor

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'],
},
{
Copy link
Contributor

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?

Copy link
Contributor Author

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);
//^^^^^^^^^^^^
//^^^ ^
Copy link
Contributor

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

@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

92.9% 92.9% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve highlighting for no-extra-arguments
3 participants