Skip to content

Commit 150417a

Browse files
graememorganError Prone Team
authored andcommitted
Fix a crash when finding the enclosing method (for a second time).
This made me realise the behaviour is flagrantly inconsistent between lambdas and anonymous classes though, so I fixed that. PiperOrigin-RevId: 775165446
1 parent bbb9e15 commit 150417a

File tree

2 files changed

+72
-22
lines changed

2 files changed

+72
-22
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/formatstring/AnnotateFormatMethod.java

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,17 @@
1818

1919
import static com.google.common.collect.Iterables.getLast;
2020
import static com.google.common.collect.MoreCollectors.toOptional;
21+
import static com.google.common.collect.Streams.stream;
2122
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
2223
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
2324
import static com.google.errorprone.matchers.Description.NO_MATCH;
2425
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
2526
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
26-
import static com.google.errorprone.util.ASTHelpers.findEnclosingNode;
2727
import static com.google.errorprone.util.ASTHelpers.getReceiver;
2828
import static com.google.errorprone.util.ASTHelpers.getSymbol;
2929
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
3030
import static com.google.errorprone.util.AnnotationNames.FORMAT_METHOD_ANNOTATION;
31+
import static java.util.stream.Stream.empty;
3132

3233
import com.google.errorprone.BugPattern;
3334
import com.google.errorprone.VisitorState;
@@ -42,6 +43,7 @@
4243
import com.sun.tools.javac.code.Symbol;
4344
import com.sun.tools.javac.code.Symbol.VarSymbol;
4445
import java.util.List;
46+
import java.util.stream.Stream;
4547
import org.jspecify.annotations.Nullable;
4648

4749
/** A BugPattern; see the summary. */
@@ -85,27 +87,38 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
8587
if (formatString == null || formatArgs == null) {
8688
return NO_MATCH;
8789
}
88-
MethodTree enclosingMethod = findEnclosingNode(state.getPath(), MethodTree.class);
89-
if (enclosingMethod == null
90-
|| !getSymbol(enclosingMethod).isVarArgs()
91-
|| hasAnnotation(enclosingMethod, FORMAT_METHOD_ANNOTATION, state)) {
92-
return NO_MATCH;
93-
}
94-
List<? extends VariableTree> enclosingParameters = enclosingMethod.getParameters();
95-
VariableTree formatParameter = findParameterWithSymbol(enclosingParameters, formatString);
96-
VariableTree argumentsParameter = findParameterWithSymbol(enclosingParameters, formatArgs);
97-
if (formatParameter == null || argumentsParameter == null) {
98-
return NO_MATCH;
99-
}
100-
if (!argumentsParameter.equals(getLast(enclosingParameters))) {
101-
return NO_MATCH;
102-
}
103-
// We can only generate a fix if the format string is the penultimate parameter.
104-
boolean fixable =
105-
formatParameter.equals(enclosingParameters.get(enclosingParameters.size() - 2));
106-
return buildDescription(enclosingMethod)
107-
.setMessage(fixable ? message() : (message() + REORDER))
108-
.build();
90+
91+
return stream(state.getPath())
92+
.flatMap(
93+
node -> {
94+
if (!(node instanceof MethodTree methodTree)) {
95+
return empty();
96+
}
97+
if (!getSymbol(methodTree).isVarArgs()
98+
|| hasAnnotation(methodTree, FORMAT_METHOD_ANNOTATION, state)) {
99+
return empty();
100+
}
101+
List<? extends VariableTree> enclosingParameters = methodTree.getParameters();
102+
VariableTree formatParameter =
103+
findParameterWithSymbol(enclosingParameters, formatString);
104+
VariableTree argumentsParameter =
105+
findParameterWithSymbol(enclosingParameters, formatArgs);
106+
if (formatParameter == null || argumentsParameter == null) {
107+
return empty();
108+
}
109+
if (!argumentsParameter.equals(getLast(enclosingParameters))) {
110+
return empty();
111+
}
112+
// We can only generate a fix if the format string is the penultimate parameter.
113+
boolean fixable =
114+
formatParameter.equals(enclosingParameters.get(enclosingParameters.size() - 2));
115+
return Stream.of(
116+
buildDescription(methodTree)
117+
.setMessage(fixable ? message() : (message() + REORDER))
118+
.build());
119+
})
120+
.findFirst()
121+
.orElse(NO_MATCH);
109122
}
110123

111124
private static VariableTree findParameterWithSymbol(

core/src/test/java/com/google/errorprone/bugpatterns/formatstring/AnnotateFormatMethodTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,43 @@ String formatMe(String formatString, Object... args) {
4242
.doTest();
4343
}
4444

45+
@Test
46+
public void passedThroughToLambda() {
47+
compilationHelper
48+
.addSourceLines(
49+
"AnnotateFormatMethodPositiveCases.java",
50+
"""
51+
class AnnotateFormatMethodPositiveCases {
52+
// BUG: Diagnostic contains: FormatMethod
53+
Runnable formatMe(String formatString, Object... args) {
54+
return () -> String.format(formatString, args);
55+
}
56+
}
57+
""")
58+
.doTest();
59+
}
60+
61+
@Test
62+
public void passedThroughToAnonymousClass() {
63+
compilationHelper
64+
.addSourceLines(
65+
"AnnotateFormatMethodPositiveCases.java",
66+
"""
67+
class AnnotateFormatMethodPositiveCases {
68+
// BUG: Diagnostic contains: FormatMethod
69+
Runnable formatMe(String formatString, Object... args) {
70+
return new Runnable() {
71+
@Override
72+
public void run() {
73+
String.format(formatString, args);
74+
}
75+
};
76+
}
77+
}
78+
""")
79+
.doTest();
80+
}
81+
4582
@Test
4683
public void formatted() {
4784
compilationHelper

0 commit comments

Comments
 (0)