Skip to content

Commit 30ec100

Browse files
cirrascirras
committed
Detect trailing whitespace within comments in TrailingWhitespace 
1 parent 22b859a commit 30ec100

File tree

3 files changed

+176
-10
lines changed

3 files changed

+176
-10
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2525
- False positives from case statements in `LoopExecutingAtMostOnce`.
2626
- False positives from nested finally-except blocks in `RedundantJump`.
2727
- False positives around wrapped type declarations in `VisibilityKeywordIndentation`.
28+
- Trailing whitespace within comments not recognized in `TrailingWhitespace`.
2829
- Several compiler directives were not being recognized:
2930
- `E`
3031
- `F`

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheck.java‎

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@
2020

2121
import static java.util.regex.Pattern.compile;
2222

23+
import com.google.common.base.CharMatcher;
2324
import com.google.common.base.Splitter;
25+
import java.util.List;
2426
import java.util.regex.Pattern;
25-
import org.apache.commons.lang3.StringUtils;
2627
import org.sonar.check.Rule;
2728
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
2829
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
2930
import org.sonar.plugins.communitydelphi.api.check.FilePosition;
31+
import org.sonar.plugins.communitydelphi.api.reporting.QuickFix;
32+
import org.sonar.plugins.communitydelphi.api.reporting.QuickFixEdit;
3033
import org.sonar.plugins.communitydelphi.api.token.DelphiToken;
3134
import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
3235

@@ -36,9 +39,19 @@ public class TrailingWhitespaceCheck extends DelphiCheck {
3639
private static final String MESSAGE = "Remove this trailing whitespace.";
3740

3841
private static final Pattern NEW_LINE_DELIMITER = compile("\r\n?|\n");
42+
private static final Splitter NEW_LINE_SPLITTER = Splitter.on(NEW_LINE_DELIMITER);
43+
private static final CharMatcher WHITESPACE_MATCHER =
44+
CharMatcher.inRange((char) 0x00, (char) 0x20)
45+
.and(CharMatcher.isNot('\r'))
46+
.and(CharMatcher.isNot('\n'));
3947

4048
@Override
4149
public void visitToken(DelphiToken token, DelphiCheckContext context) {
50+
visitWhitespace(token, context);
51+
visitComment(token, context);
52+
}
53+
54+
private static void visitWhitespace(DelphiToken token, DelphiCheckContext context) {
4255
if (!token.isWhitespace()) {
4356
return;
4457
}
@@ -47,21 +60,70 @@ public void visitToken(DelphiToken token, DelphiCheckContext context) {
4760
return;
4861
}
4962

50-
int line = token.getBeginLine();
51-
int column = token.getBeginColumn();
63+
String image = WHITESPACE_MATCHER.trimTrailingFrom(token.getImage());
64+
List<String> whitespaces = NEW_LINE_SPLITTER.splitToList(image);
65+
66+
for (int i = 0; i < whitespaces.size(); ++i) {
67+
String whitespace = whitespaces.get(i);
68+
if (whitespace.isEmpty()) {
69+
continue;
70+
}
71+
72+
int line = token.getBeginLine() + i;
73+
int column;
74+
75+
if (i == 0) {
76+
column = token.getBeginColumn();
77+
} else {
78+
column = 0;
79+
}
80+
81+
var whitespacePosition = FilePosition.from(line, column, line, column + whitespace.length());
82+
83+
context
84+
.newIssue()
85+
.onFilePosition(whitespacePosition)
86+
.withMessage(MESSAGE)
87+
.withQuickFixes(
88+
QuickFix.newFix("Remove trailing whitespace")
89+
.withEdit(QuickFixEdit.delete(whitespacePosition)))
90+
.report();
91+
}
92+
}
93+
94+
private static void visitComment(DelphiToken token, DelphiCheckContext context) {
95+
if (!token.isComment()) {
96+
return;
97+
}
98+
99+
var commentLines = NEW_LINE_SPLITTER.splitToList(token.getImage());
100+
101+
for (int i = 0; i < commentLines.size(); ++i) {
102+
String commentLine = commentLines.get(i);
103+
String trimmedCommentLine = WHITESPACE_MATCHER.trimTrailingFrom(commentLine);
104+
int whitespaceLength = commentLine.length() - trimmedCommentLine.length();
105+
106+
if (whitespaceLength > 0) {
107+
int line = token.getBeginLine() + i;
108+
int column;
109+
110+
if (i == 0) {
111+
column = token.getBeginColumn() + trimmedCommentLine.length();
112+
} else {
113+
column = trimmedCommentLine.length();
114+
}
115+
116+
var whitespacePosition = FilePosition.from(line, column, line, column + whitespaceLength);
52117

53-
String image = StringUtils.stripEnd(token.getImage(), " \t\f");
54-
var parts = Splitter.on(NEW_LINE_DELIMITER).split(image);
55-
for (String whitespace : parts) {
56-
if (!whitespace.isEmpty()) {
57118
context
58119
.newIssue()
59-
.onFilePosition(FilePosition.from(line, column, line, column + whitespace.length()))
120+
.onFilePosition(whitespacePosition)
60121
.withMessage(MESSAGE)
122+
.withQuickFixes(
123+
QuickFix.newFix("Remove trailing whitespace")
124+
.withEdit(QuickFixEdit.delete(whitespacePosition)))
61125
.report();
62126
}
63-
++line;
64-
column = 0;
65127
}
66128
}
67129
}

‎delphi-checks/src/test/java/au/com/integradev/delphi/checks/TrailingWhitespaceCheckTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ void testTrailingSpaceShouldAddIssue() {
2929
.withCheck(new TrailingWhitespaceCheck())
3030
.onFile(
3131
new DelphiTestUnitBuilder()
32+
.appendImpl("// Fix qf@[+2:17 to +2:18] <<>>")
3233
.appendImpl("// Noncompliant@+1")
3334
.appendImpl("var Foo: TObject; "))
3435
.verifyIssues();
@@ -40,6 +41,7 @@ void testTrailingTabShouldAddIssue() {
4041
.withCheck(new TrailingWhitespaceCheck())
4142
.onFile(
4243
new DelphiTestUnitBuilder()
44+
.appendImpl("// Fix qf@[+2:17 to +2:18] <<>>")
4345
.appendImpl("// Noncompliant@+1")
4446
.appendImpl("var Foo: TObject;\t"))
4547
.verifyIssues();
@@ -51,11 +53,25 @@ void testTrailingMixedWhitespaceShouldAddIssue() {
5153
.withCheck(new TrailingWhitespaceCheck())
5254
.onFile(
5355
new DelphiTestUnitBuilder()
56+
.appendImpl("// Fix qf@[+2:17 to +2:26] <<>>")
5457
.appendImpl("// Noncompliant@+1")
5558
.appendImpl("var Foo: TObject;\t \t\t \t "))
5659
.verifyIssues();
5760
}
5861

62+
@Test
63+
void testTrailingSpaceOnFollowingLineShouldAddIssue() {
64+
CheckVerifier.newVerifier()
65+
.withCheck(new TrailingWhitespaceCheck())
66+
.onFile(
67+
new DelphiTestUnitBuilder()
68+
.appendImpl("// Fix qf@[+3:0 to +3:3] <<>>")
69+
.appendImpl("// Noncompliant@+2")
70+
.appendImpl("var Foo: TObject;")
71+
.appendImpl(" "))
72+
.verifyIssues();
73+
}
74+
5975
@Test
6076
void testNoTrailingWhitespaceShouldNotAddIssue() {
6177
CheckVerifier.newVerifier()
@@ -71,4 +87,91 @@ void testLeadingWhitespaceShouldNotAddIssue() {
7187
.onFile(new DelphiTestUnitBuilder().appendImpl("\t \t \t var Foo: TObject;"))
7288
.verifyNoIssues();
7389
}
90+
91+
@Test
92+
void testNoTrailingSpaceInLineCommentShouldNotAddIssue() {
93+
CheckVerifier.newVerifier()
94+
.withCheck(new TrailingWhitespaceCheck())
95+
.onFile(
96+
new DelphiTestUnitBuilder().appendImpl("// there is no trailing whitespace in here"))
97+
.verifyNoIssues();
98+
}
99+
100+
@Test
101+
void testTrailingSpaceInLineCommentShouldAddIssue() {
102+
CheckVerifier.newVerifier()
103+
.withCheck(new TrailingWhitespaceCheck())
104+
.onFile(
105+
new DelphiTestUnitBuilder()
106+
.appendImpl("// Fix qf@[+2:42 to +2:43] <<>>")
107+
.appendImpl("// Noncompliant@+1")
108+
.appendImpl("// hey, there's a trailing spaces in here! "))
109+
.verifyIssues();
110+
}
111+
112+
@Test
113+
void testTrailingTabsInLineCommentShouldAddIssue() {
114+
CheckVerifier.newVerifier()
115+
.withCheck(new TrailingWhitespaceCheck())
116+
.onFile(
117+
new DelphiTestUnitBuilder()
118+
.appendImpl("// Fix qf@[+2:39 to +2:40] <<>>")
119+
.appendImpl("// Noncompliant@+1")
120+
.appendImpl("// hey, there's a trailing tab in here!\t"))
121+
.verifyIssues();
122+
}
123+
124+
@Test
125+
void testNoTrailingSpaceInMultilineCommentShouldNotAddIssue() {
126+
CheckVerifier.newVerifier()
127+
.withCheck(new TrailingWhitespaceCheck())
128+
.onFile(
129+
new DelphiTestUnitBuilder()
130+
.appendImpl("{")
131+
.appendImpl(" there is no trailing whitespace in here")
132+
.appendImpl("}"))
133+
.verifyNoIssues();
134+
}
135+
136+
@Test
137+
void testTrailingSpaceInMultilineCommentShouldAddIssue() {
138+
CheckVerifier.newVerifier()
139+
.withCheck(new TrailingWhitespaceCheck())
140+
.onFile(
141+
new DelphiTestUnitBuilder()
142+
.appendImpl("// Fix qf@[+3:40 to +3:41] <<>>")
143+
.appendImpl("// Noncompliant@+2")
144+
.appendImpl("{")
145+
.appendImpl(" hey, there's a trailing space in here! ")
146+
.appendImpl("}"))
147+
.verifyIssues();
148+
}
149+
150+
@Test
151+
void testTrailingTabInMultilineCommentShouldAddIssue() {
152+
CheckVerifier.newVerifier()
153+
.withCheck(new TrailingWhitespaceCheck())
154+
.onFile(
155+
new DelphiTestUnitBuilder()
156+
.appendImpl("// Fix qf@[+3:38 to +3:39] <<>>")
157+
.appendImpl("// Noncompliant@+2")
158+
.appendImpl("{")
159+
.appendImpl(" hey, there's a trailing tab in here!\t")
160+
.appendImpl("}"))
161+
.verifyIssues();
162+
}
163+
164+
@Test
165+
void testTrailingSpaceOnFirstLineOfMultilineCommentWithOffsetShouldAddIssue() {
166+
CheckVerifier.newVerifier()
167+
.withCheck(new TrailingWhitespaceCheck())
168+
.onFile(
169+
new DelphiTestUnitBuilder()
170+
.appendImpl("// Fix qf@[+2:6 to +2:7] <<>>")
171+
.appendImpl("// Noncompliant@+1")
172+
.appendImpl(" {\t")
173+
.appendImpl(" hey, there's a trailing tab up there!")
174+
.appendImpl("}"))
175+
.verifyIssues();
176+
}
74177
}

0 commit comments

Comments
 (0)