-
Notifications
You must be signed in to change notification settings - Fork 184
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
JS-312 Improve S1607 (no-skipped-tests
): Allow explanation comments adjacent to the disabled test
#4804
Conversation
…the disabled test
e484a51
to
363eb6d
Compare
no-skipped-tests
): Allow explanation comments after the disabled testno-skipped-tests
): Allow explanation comments after the disabled test
@yassin-kammoun-sonarsource Great idea to tackle the line after, but why not handle when the explanation comment is on the same line as well? |
function isAdjacent(comment: estree.Comment, node: estree.Node) { | ||
const commentLine = comment.loc!.end.line; | ||
const nodeLine = node.loc!.start.line; | ||
return commentLine === nodeLine - 1 || commentLine === nodeLine + 1; |
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.
If we consider my comment here,
This could be:
return commentLine === nodeLine - 1 || commentLine === nodeLine + 1; | |
return Math.abs(commentLine - nodeLine) <= 1; |
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.
I'm not sure how it could be tested in the comment-based framework though.
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.
Yeah, I was going to, but I just don't know how to test that.
Any suggestions?
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.
Another option is to use relative line increments (@+line_increment
) or decrements (@-line_decrement
):
// Noncompliant@+1
some.faulty.code();
another.faulty.code();
// another comment
// Noncompliant@-2
from DEV.md
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.
Thank you so much for remembering it!
Quality Gate passedIssues Measures |
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.
LGTM, good job!
no-skipped-tests
): Allow explanation comments after the disabled testno-skipped-tests
): Allow explanation comments adjacent to the disabled test
While validating S1607 on Peach, I encountered some false positives where a disabled test has an explanation comment that follows the test, such as this issue.
RSPEC's change to review