Skip to content

Commit 5f71110

Browse files
graememorganError Prone Team
graememorgan
authored and
Error Prone Team
committed
Add a check to reformat poorly formatted EP tests.
PiperOrigin-RevId: 683676067
1 parent cd6ae3b commit 5f71110

File tree

4 files changed

+334
-0
lines changed

4 files changed

+334
-0
lines changed

‎core/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@
6868
<version>${project.version}</version>
6969
<scope>test</scope>
7070
</dependency>
71+
<dependency>
72+
<!-- Apache 2.0 -->
73+
<groupId>com.google.googlejavaformat</groupId>
74+
<artifactId>google-java-format</artifactId>
75+
<version>1.19.1</version>
76+
</dependency>
7177
<dependency>
7278
<!-- MIT -->
7379
<groupId>org.pcollections</groupId>

‎core/src/main/java/com/google/errorprone/bugpatterns/MisformattedTestData.java‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright 2024 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
20+
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.matchers.Matchers.anyOf;
22+
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
23+
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
24+
import static com.google.errorprone.util.ErrorProneTokens.getTokens;
25+
import static com.google.errorprone.util.SourceVersion.supportsTextBlocks;
26+
import static java.util.stream.Collectors.joining;
27+
28+
import com.google.common.base.Splitter;
29+
import com.google.common.escape.Escaper;
30+
import com.google.common.escape.Escapers;
31+
import com.google.errorprone.BugPattern;
32+
import com.google.errorprone.VisitorState;
33+
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
34+
import com.google.errorprone.fixes.SuggestedFix;
35+
import com.google.errorprone.matchers.Description;
36+
import com.google.errorprone.matchers.Matcher;
37+
import com.google.googlejavaformat.java.Formatter;
38+
import com.google.googlejavaformat.java.FormatterException;
39+
import com.sun.source.tree.ExpressionTree;
40+
import com.sun.source.tree.LiteralTree;
41+
import com.sun.source.tree.MethodInvocationTree;
42+
import com.sun.tools.javac.parser.Tokens.TokenKind;
43+
44+
/** See the summary. */
45+
@BugPattern(
46+
severity = WARNING,
47+
summary = "This test data will be more readable if correctly formatted.")
48+
public final class MisformattedTestData extends BugChecker implements MethodInvocationTreeMatcher {
49+
@Override
50+
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
51+
// We're not only matching text blocks below, just taking the fact that it's a single literal
52+
// argument containing source code as a sign that it should be formatted in a text block.
53+
if (!supportsTextBlocks(state.context)) {
54+
return NO_MATCH;
55+
}
56+
if (!ADD_SOURCE_CALL.matches(tree, state)) {
57+
return NO_MATCH;
58+
}
59+
if (tree.getArguments().size() != 2) {
60+
return NO_MATCH;
61+
}
62+
var sourceTree = tree.getArguments().get(1);
63+
if (!(sourceTree instanceof LiteralTree)) {
64+
return NO_MATCH;
65+
}
66+
var sourceValue = ((LiteralTree) sourceTree).getValue();
67+
if (!(sourceValue instanceof String)) {
68+
return NO_MATCH;
69+
}
70+
71+
Formatter formatter = new Formatter();
72+
String formattedSource;
73+
try {
74+
formattedSource = formatter.formatSource((String) sourceValue);
75+
} catch (FormatterException exception) {
76+
return NO_MATCH;
77+
}
78+
if (formattedSource.equals(sourceValue)) {
79+
return NO_MATCH;
80+
}
81+
// This is a bit crude: but tokenize between the comma and the 2nd argument in order to work out
82+
// an appropriate indent level for the text block. This is assuming that the source has already
83+
// been formatted so that the arguments are nicely indented.
84+
int startPos = state.getEndPosition(tree.getArguments().get(0));
85+
int endPos = getStartPosition(tree.getArguments().get(1));
86+
var tokens =
87+
getTokens(
88+
state.getSourceCode().subSequence(startPos, endPos).toString(),
89+
startPos,
90+
state.context);
91+
var afterCommaPos =
92+
tokens.reverse().stream()
93+
.filter(t -> t.kind().equals(TokenKind.COMMA))
94+
.findFirst()
95+
.orElseThrow()
96+
.endPos();
97+
var spaces =
98+
state.getSourceCode().subSequence(afterCommaPos, endPos).toString().replace("\n", "");
99+
return describeMatch(
100+
sourceTree,
101+
SuggestedFix.replace(
102+
sourceTree,
103+
"\"\"\"\n"
104+
+ LINE_SPLITTER
105+
.splitToStream(escape(formattedSource))
106+
.map(line -> line.isEmpty() ? "" : spaces + line)
107+
.collect(joining("\n"))
108+
+ spaces
109+
+ "\"\"\""));
110+
}
111+
112+
// TODO(ghm): Consider generalising this via an annotation.
113+
private static final Matcher<ExpressionTree> ADD_SOURCE_CALL =
114+
anyOf(
115+
instanceMethod()
116+
.onExactClass("com.google.errorprone.CompilationTestHelper")
117+
.named("addSourceLines"),
118+
instanceMethod()
119+
.onExactClass("com.google.errorprone.BugCheckerRefactoringTestHelper")
120+
.named("addInputLines"),
121+
instanceMethod()
122+
.onExactClass("com.google.errorprone.BugCheckerRefactoringTestHelper")
123+
.named("addOutputLines"));
124+
125+
private static final Splitter LINE_SPLITTER = Splitter.on('\n');
126+
127+
private static String escape(String line) {
128+
return ESCAPER.escape(line).replace("\"\"\"", "\\\"\"\"");
129+
}
130+
131+
private static final Escaper ESCAPER = Escapers.builder().addEscape('\\', "\\\\").build();
132+
}

‎core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@
235235
import com.google.errorprone.bugpatterns.MemberName;
236236
import com.google.errorprone.bugpatterns.MemoizeConstantVisitorStateLookups;
237237
import com.google.errorprone.bugpatterns.MethodCanBeStatic;
238+
import com.google.errorprone.bugpatterns.MisformattedTestData;
238239
import com.google.errorprone.bugpatterns.MissingBraces;
239240
import com.google.errorprone.bugpatterns.MissingCasesInEnumSwitch;
240241
import com.google.errorprone.bugpatterns.MissingDefault;
@@ -997,6 +998,7 @@ public static ScannerSupplier warningChecks() {
997998
MalformedInlineTag.class,
998999
MathAbsoluteNegative.class,
9991000
MemoizeConstantVisitorStateLookups.class,
1001+
MisformattedTestData.class,
10001002
MissingCasesInEnumSwitch.class,
10011003
MissingFail.class,
10021004
MissingImplementsComparable.class,

‎core/src/test/java/com/google/errorprone/bugpatterns/MisformattedTestDataTest.java‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/*
2+
* Copyright 2024 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.common.truth.TruthJUnit.assume;
20+
import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
21+
22+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
23+
import com.google.errorprone.CompilationTestHelper;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.junit.runners.JUnit4;
27+
28+
@RunWith(JUnit4.class)
29+
public final class MisformattedTestDataTest {
30+
private final CompilationTestHelper compilationHelper =
31+
CompilationTestHelper.newInstance(MisformattedTestData.class, getClass());
32+
private final BugCheckerRefactoringTestHelper refactoringHelper =
33+
BugCheckerRefactoringTestHelper.newInstance(MisformattedTestData.class, getClass());
34+
35+
@Test
36+
public void alreadyFormatted_noFinding() {
37+
assume().that(Runtime.version().feature()).isAtLeast(14);
38+
39+
compilationHelper
40+
.addSourceLines(
41+
"Test.java",
42+
"""
43+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
44+
45+
class Test {
46+
void method(BugCheckerRefactoringTestHelper h) {
47+
h.addInputLines(
48+
"Test.java",
49+
\"""
50+
package foo;
51+
52+
class Test {
53+
void method() {
54+
int a = 1;
55+
}
56+
}
57+
\""");
58+
}
59+
}
60+
""")
61+
.doTest();
62+
}
63+
64+
@Test
65+
public void misformatted_suggestsFix() {
66+
assume().that(Runtime.version().feature()).isAtLeast(14);
67+
68+
refactoringHelper
69+
.addInputLines(
70+
"Test.java",
71+
"""
72+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
73+
74+
class Test {
75+
void method(BugCheckerRefactoringTestHelper h) {
76+
h.addInputLines(
77+
"Test.java",
78+
\"""
79+
package foo;
80+
class Test {
81+
void method() {
82+
int a =
83+
1;
84+
}
85+
}
86+
\""");
87+
}
88+
}
89+
""")
90+
.addOutputLines(
91+
"Test.java",
92+
"""
93+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
94+
95+
class Test {
96+
void method(BugCheckerRefactoringTestHelper h) {
97+
h.addInputLines(
98+
"Test.java",
99+
\"""
100+
package foo;
101+
102+
class Test {
103+
void method() {
104+
int a = 1;
105+
}
106+
}
107+
\""");
108+
}
109+
}
110+
""")
111+
.doTest(TEXT_MATCH);
112+
}
113+
114+
@Test
115+
public void onlyDiffersByIndentation_notReindented() {
116+
assume().that(Runtime.version().feature()).isAtLeast(14);
117+
118+
refactoringHelper
119+
.addInputLines(
120+
"Test.java",
121+
"""
122+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
123+
124+
class Test {
125+
void method(BugCheckerRefactoringTestHelper h) {
126+
h.addInputLines(
127+
"Test.java",
128+
\"""
129+
package foo;
130+
131+
class Test {
132+
void method() {
133+
int a = 1;
134+
}
135+
}
136+
""\");
137+
}
138+
}
139+
""")
140+
.expectUnchanged()
141+
.doTest(TEXT_MATCH);
142+
}
143+
144+
@Test
145+
public void escapesSpecialCharacters() {
146+
assume().that(Runtime.version().feature()).isAtLeast(14);
147+
148+
refactoringHelper
149+
.addInputLines(
150+
"Test.java",
151+
"""
152+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
153+
154+
class Test {
155+
void method(BugCheckerRefactoringTestHelper h) {
156+
h.addInputLines(
157+
"Test.java",
158+
\"""
159+
package foo;
160+
161+
class Test {
162+
void method() {
163+
var foo
164+
= "foo\\\\nbar";
165+
}
166+
}
167+
\""");
168+
}
169+
}
170+
""")
171+
.addOutputLines(
172+
"Test.java",
173+
"""
174+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
175+
176+
class Test {
177+
void method(BugCheckerRefactoringTestHelper h) {
178+
h.addInputLines(
179+
"Test.java",
180+
\"""
181+
package foo;
182+
183+
class Test {
184+
void method() {
185+
var foo = "foo\\\\nbar";
186+
}
187+
}
188+
\""");
189+
}
190+
}
191+
""")
192+
.doTest(TEXT_MATCH);
193+
}
194+
}

0 commit comments

Comments
 (0)