Add quick fix for S4623 for ('no-undefined-argument') #3040
Conversation
yassin-kammoun-sonarsource
force-pushed
the
issue-3028
branch
from
7382adb
to
94245f3
Compare
| { | ||
| messageId: 'suggestRemoveUndefined', | ||
| fix: fixer => { | ||
| const singleArgument = call.arguments.length === 1; |
| messages: { | ||
| removeUndefined: 'Remove this redundant "undefined".', | ||
| suggestRemoveUndefined: 'Remove "undefined"', |
There was a problem hiding this comment.
I would say Remove this redundant argument
| errors: 2, | ||
| errors: [ | ||
| { | ||
| messageId: 'removeUndefined', |
There was a problem hiding this comment.
I would drop this extra assertion of issue message
| }, | ||
| ], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Can we have test with trailing comma?
| desc: 'Remove this redundant argument', | ||
| output: ` | ||
| function fun(p = 42) {} | ||
| fun(,); |
There was a problem hiding this comment.
this is syntax error, we can't have such fix
There was a problem hiding this comment.
add test with trailing comma when there are several arguments
yassin-kammoun-sonarsource
changed the title
| desc: 'Remove this redundant argument', | ||
| output: ` | ||
| function fun(p, q, r = 42) {} | ||
| fun(1, 2); |
There was a problem hiding this comment.
IMHO we should keep trailing comma here. That would be most probably used for multiline and then it's expected to have comma after each argument:
foo(
1,
2,
undefined,
);
// will become
foo(
1,
2
);
// and this will violate style... while it's probably pretty minor
WDYT?
There was a problem hiding this comment.
I am fine with the behaviour you are suggesting.
yassin-kammoun-sonarsource
force-pushed
the
issue-3028
branch
from
cde0e0e
to
7d47447
Compare
…e of optional parameters
yassin-kammoun-sonarsource
force-pushed
the
issue-3028
branch
from
7d47447
to
8600bba
Compare
yassin-kammoun-sonarsource
force-pushed
the
issue-3028
branch
from
8600bba
to
920ff1b
Compare
There was a problem hiding this comment.
@yassin-kammoun-sonarsource I think you misunderstood me. I wanted to keep the styling: so if trailing comma is there we should keep it, if there is no trailing comma, there should be none in the result as well.
|
SonarQube Quality Gate
|
Fixes #3028