diff --git a/CHANGELOG.md b/CHANGELOG.md index c0586b574..ff55d2cc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - False positives from case statements in `LoopExecutingAtMostOnce`. - False positives from nested finally-except blocks in `RedundantJump`. - False positives around wrapped type declarations in `VisibilityKeywordIndentation`. +- Trailing whitespace within comments not recognized in `TrailingWhitespace`. - Several compiler directives were not being recognized: - `E` - `F` diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheck.java index b0760bbef..658e82f65 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheck.java @@ -20,13 +20,16 @@ import static java.util.regex.Pattern.compile; +import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; +import java.util.List; import java.util.regex.Pattern; -import org.apache.commons.lang3.StringUtils; import org.sonar.check.Rule; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; import org.sonar.plugins.communitydelphi.api.check.FilePosition; +import org.sonar.plugins.communitydelphi.api.reporting.QuickFix; +import org.sonar.plugins.communitydelphi.api.reporting.QuickFixEdit; import org.sonar.plugins.communitydelphi.api.token.DelphiToken; import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; @@ -36,9 +39,19 @@ public class TrailingWhitespaceCheck extends DelphiCheck { private static final String MESSAGE = "Remove this trailing whitespace."; private static final Pattern NEW_LINE_DELIMITER = compile("\r\n?|\n"); + private static final Splitter NEW_LINE_SPLITTER = Splitter.on(NEW_LINE_DELIMITER); + private static final CharMatcher WHITESPACE_MATCHER = + CharMatcher.inRange((char) 0x00, (char) 0x20) + .and(CharMatcher.isNot('\r')) + .and(CharMatcher.isNot('\n')); @Override public void visitToken(DelphiToken token, DelphiCheckContext context) { + visitWhitespace(token, context); + visitComment(token, context); + } + + private static void visitWhitespace(DelphiToken token, DelphiCheckContext context) { if (!token.isWhitespace()) { return; } @@ -47,21 +60,70 @@ public void visitToken(DelphiToken token, DelphiCheckContext context) { return; } - int line = token.getBeginLine(); - int column = token.getBeginColumn(); + String image = WHITESPACE_MATCHER.trimTrailingFrom(token.getImage()); + List whitespaces = NEW_LINE_SPLITTER.splitToList(image); + + for (int i = 0; i < whitespaces.size(); ++i) { + String whitespace = whitespaces.get(i); + if (whitespace.isEmpty()) { + continue; + } + + int line = token.getBeginLine() + i; + int column; + + if (i == 0) { + column = token.getBeginColumn(); + } else { + column = 0; + } + + var whitespacePosition = FilePosition.from(line, column, line, column + whitespace.length()); + + context + .newIssue() + .onFilePosition(whitespacePosition) + .withMessage(MESSAGE) + .withQuickFixes( + QuickFix.newFix("Remove trailing whitespace") + .withEdit(QuickFixEdit.delete(whitespacePosition))) + .report(); + } + } + + private static void visitComment(DelphiToken token, DelphiCheckContext context) { + if (!token.isComment()) { + return; + } + + var commentLines = NEW_LINE_SPLITTER.splitToList(token.getImage()); + + for (int i = 0; i < commentLines.size(); ++i) { + String commentLine = commentLines.get(i); + String trimmedCommentLine = WHITESPACE_MATCHER.trimTrailingFrom(commentLine); + int whitespaceLength = commentLine.length() - trimmedCommentLine.length(); + + if (whitespaceLength > 0) { + int line = token.getBeginLine() + i; + int column; + + if (i == 0) { + column = token.getBeginColumn() + trimmedCommentLine.length(); + } else { + column = trimmedCommentLine.length(); + } + + var whitespacePosition = FilePosition.from(line, column, line, column + whitespaceLength); - String image = StringUtils.stripEnd(token.getImage(), " \t\f"); - var parts = Splitter.on(NEW_LINE_DELIMITER).split(image); - for (String whitespace : parts) { - if (!whitespace.isEmpty()) { context .newIssue() - .onFilePosition(FilePosition.from(line, column, line, column + whitespace.length())) + .onFilePosition(whitespacePosition) .withMessage(MESSAGE) + .withQuickFixes( + QuickFix.newFix("Remove trailing whitespace") + .withEdit(QuickFixEdit.delete(whitespacePosition))) .report(); } - ++line; - column = 0; } } } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheckTest.java index a549a86ef..397925fc0 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheckTest.java @@ -29,6 +29,7 @@ void testTrailingSpaceShouldAddIssue() { .withCheck(new TrailingWhitespaceCheck()) .onFile( new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:17 to +2:18] <<>>") .appendImpl("// Noncompliant@+1") .appendImpl("var Foo: TObject; ")) .verifyIssues(); @@ -40,6 +41,7 @@ void testTrailingTabShouldAddIssue() { .withCheck(new TrailingWhitespaceCheck()) .onFile( new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:17 to +2:18] <<>>") .appendImpl("// Noncompliant@+1") .appendImpl("var Foo: TObject;\t")) .verifyIssues(); @@ -51,11 +53,25 @@ void testTrailingMixedWhitespaceShouldAddIssue() { .withCheck(new TrailingWhitespaceCheck()) .onFile( new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:17 to +2:26] <<>>") .appendImpl("// Noncompliant@+1") .appendImpl("var Foo: TObject;\t \t\t \t ")) .verifyIssues(); } + @Test + void testTrailingSpaceOnFollowingLineShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+3:0 to +3:3] <<>>") + .appendImpl("// Noncompliant@+2") + .appendImpl("var Foo: TObject;") + .appendImpl(" ")) + .verifyIssues(); + } + @Test void testNoTrailingWhitespaceShouldNotAddIssue() { CheckVerifier.newVerifier() @@ -71,4 +87,91 @@ void testLeadingWhitespaceShouldNotAddIssue() { .onFile(new DelphiTestUnitBuilder().appendImpl("\t \t \t var Foo: TObject;")) .verifyNoIssues(); } + + @Test + void testNoTrailingSpaceInLineCommentShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder().appendImpl("// there is no trailing whitespace in here")) + .verifyNoIssues(); + } + + @Test + void testTrailingSpaceInLineCommentShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:42 to +2:43] <<>>") + .appendImpl("// Noncompliant@+1") + .appendImpl("// hey, there's a trailing spaces in here! ")) + .verifyIssues(); + } + + @Test + void testTrailingTabsInLineCommentShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:39 to +2:40] <<>>") + .appendImpl("// Noncompliant@+1") + .appendImpl("// hey, there's a trailing tab in here!\t")) + .verifyIssues(); + } + + @Test + void testNoTrailingSpaceInMultilineCommentShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("{") + .appendImpl(" there is no trailing whitespace in here") + .appendImpl("}")) + .verifyNoIssues(); + } + + @Test + void testTrailingSpaceInMultilineCommentShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+3:40 to +3:41] <<>>") + .appendImpl("// Noncompliant@+2") + .appendImpl("{") + .appendImpl(" hey, there's a trailing space in here! ") + .appendImpl("}")) + .verifyIssues(); + } + + @Test + void testTrailingTabInMultilineCommentShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+3:38 to +3:39] <<>>") + .appendImpl("// Noncompliant@+2") + .appendImpl("{") + .appendImpl(" hey, there's a trailing tab in here!\t") + .appendImpl("}")) + .verifyIssues(); + } + + @Test + void testTrailingSpaceOnFirstLineOfMultilineCommentWithOffsetShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new TrailingWhitespaceCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("// Fix qf@[+2:6 to +2:7] <<>>") + .appendImpl("// Noncompliant@+1") + .appendImpl(" {\t") + .appendImpl(" hey, there's a trailing tab up there!") + .appendImpl("}")) + .verifyIssues(); + } }