Add quick fix for S1186 ('no-empty-function') #3056
Conversation
yassin-kammoun-sonarsource
force-pushed
the
issue-3005
branch
from
a76cf6f
to
20d05bf
Compare
yassin-kammoun-sonarsource
force-pushed
the
issue-3005
branch
from
20d05bf
to
1a2f8f6
Compare
|
Inspired from EmptyMethodsCheck (SonarJava). |
| export function decorateNoEmptyFunction(rule: Rule.RuleModule): Rule.RuleModule { | ||
| rule.meta!.hasSuggestions = true; | ||
| return interceptReport(rule, (context, reportDescriptor) => { | ||
| if ('node' in reportDescriptor) { |
There was a problem hiding this comment.
what will happen if node is not there? Issue will not be reported at all?
There was a problem hiding this comment.
Yes, the issue won't be reported. I am starting to think that this condition is useless because once we intercept a report, we know for sure that there is a node, at least for this rule. Should I just remove the condition ?
| return interceptReport(rule, (context, reportDescriptor) => { | ||
| if ('node' in reportDescriptor) { | ||
| const func = reportDescriptor.node as FunctionLike; | ||
| const name = reportDescriptor.data!.name; |
There was a problem hiding this comment.
I think we should be consistent, if we check node presence, let's also check data.name presence. Or let's assume that they are always available (I tend to second option)
| const name = reportDescriptor.data!.name; | ||
| const openingBrace = context | ||
| .getSourceCode() | ||
| .getFirstToken(func.body, token => token.value === '{')!; |
There was a problem hiding this comment.
I think we can remove filter by value as we know that body is empty
| } else { | ||
| const columnOffset = closingBrace.loc.start.column; | ||
| const padding = ' '.repeat(columnOffset); | ||
| commentPlaceholder = `\n${padding} // TODO document why this ${name} is empty\n${padding}`; |
There was a problem hiding this comment.
I see that java impl has that, but I don't get why we need second new line. Do you?
There was a problem hiding this comment.
No idea. SONARJAVA-3906 mentions that there are situations when a newline is needed, but I didn't see an example in the unit tests.
| } | ||
| }; | ||
| `, | ||
| errors: [ |
There was a problem hiding this comment.
can we add the arrow function to see what will be the name of it?
|
SonarQube Quality Gate
|
Fixes #3005