integrated-application-development · cirras · Mar 28, 2025 · Mar 18, 2025
diff --git 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
@@ -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<String> 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
@@ -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();
+  }
 }