SONARJAVA-5537 implement S7476: comments must start with correct number of slashes#5151
SONARJAVA-5537 implement S7476: comments must start with correct number of slashes#5151erwan-serandour merged 9 commits intomasterfrom
Conversation
romainbrenguier
left a comment
romainbrenguier
left a comment
There was a problem hiding this comment.
There is nothing really blocking, but I think we coud have a quickfix for this.
| public class CommentsMustStartWithCorrectNumberOfSlashesCheck extends IssuableSubscriptionVisitor { | ||
| private static final String BEFORE_JAVA_23 = "A single-line comment should start with exactly two slashes, no more."; | ||
| private static final String AFTER_JAVA_23 = "Markdown documentation should start with exactly three slashes, no more."; | ||
| private static final String SLASHES_BEFORE_JAVA_23 = "///"; |
There was a problem hiding this comment.
Maybe add something like INCORRECT_ as prefix to the name, to make the purpose clearer.
| public void visitTrivia(SyntaxTrivia syntaxTrivia) { | ||
| if (syntaxTrivia.isComment(SyntaxTrivia.CommentKind.LINE) && syntaxTrivia.comment().startsWith(SLASHES_BEFORE_JAVA_23)) { | ||
| var span = LineSpan.fromComment(syntaxTrivia, 0, 0, SLASHES_BEFORE_JAVA_23.length()); | ||
| ((DefaultJavaFileScannerContext) this.context).reportIssue(issueSingleLine(span, BEFORE_JAVA_23)); |
There was a problem hiding this comment.
Since it's used several times, could you declare a variable for ((DefaultJavaFileScannerContext) this.context) or a method to report issues.
| public void visitTrivia(SyntaxTrivia syntaxTrivia) { | ||
| if (syntaxTrivia.isComment(SyntaxTrivia.CommentKind.LINE) && syntaxTrivia.comment().startsWith(SLASHES_BEFORE_JAVA_23)) { | ||
| var span = LineSpan.fromComment(syntaxTrivia, 0, 0, SLASHES_BEFORE_JAVA_23.length()); | ||
| ((DefaultJavaFileScannerContext) this.context).reportIssue(issueSingleLine(span, BEFORE_JAVA_23)); |
There was a problem hiding this comment.
Since the reportIssue method used is declared in DefaultModuleScannerContext, it would make more sense to cast to that.
| } | ||
|
|
||
| private AnalyzerMessage issueSingleLine(LineSpan span, String message) { | ||
| AnalyzerMessage.TextSpan textSpan = new AnalyzerMessage.TextSpan(span.line, span.start, span.line, span.end); |
There was a problem hiding this comment.
you could use var here, since you used it in other places
| "ruleSpecification": "RSPEC-7476", | ||
| "sqKey": "S7476", | ||
| "scope": "All", | ||
| "quickfix": "unknown", |
There was a problem hiding this comment.
could we consider a quickfix for this?
There was a problem hiding this comment.
The quickfix api does not support trivia nodes. It would mean changing it. Quite some work and I don't think it is worth it for the rule.
There was a problem hiding this comment.
We can keep it unknown for now.
|
2 participants
SONARJAVA-5537
Part of