Skip to content

Commit 2b78f85

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Consolidate more logic from format string checks in shared helpers
This makes FormatStringUtils more similar to LenientFormatStringUtils, it now handles annotated and well-known format method APIs, and returns the list of arguments starting with the format string. PiperOrigin-RevId: 830791614
1 parent 22c7428 commit 2b78f85

File tree

6 files changed

+217
-172
lines changed

6 files changed

+217
-172
lines changed

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

Lines changed: 106 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,31 @@
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;
2221
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
2322
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
2423
import static com.google.errorprone.matchers.Description.NO_MATCH;
25-
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
26-
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
27-
import static com.google.errorprone.util.ASTHelpers.getReceiver;
2824
import static com.google.errorprone.util.ASTHelpers.getSymbol;
2925
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
3026
import static com.google.errorprone.util.AnnotationNames.FORMAT_METHOD_ANNOTATION;
31-
import static java.util.stream.Stream.empty;
27+
import static com.google.errorprone.util.AnnotationNames.FORMAT_STRING_ANNOTATION;
28+
import static com.google.errorprone.util.AnnotationNames.LENIENT_FORMAT_STRING_ANNOTATION;
3229

30+
import com.google.common.collect.ImmutableList;
3331
import com.google.errorprone.BugPattern;
3432
import com.google.errorprone.VisitorState;
3533
import com.google.errorprone.bugpatterns.BugChecker;
3634
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
35+
import com.google.errorprone.fixes.SuggestedFix;
36+
import com.google.errorprone.fixes.SuggestedFixes;
3737
import com.google.errorprone.matchers.Description;
38-
import com.google.errorprone.matchers.Matcher;
3938
import com.sun.source.tree.ExpressionTree;
4039
import com.sun.source.tree.MethodInvocationTree;
4140
import com.sun.source.tree.MethodTree;
41+
import com.sun.source.tree.Tree;
4242
import com.sun.source.tree.VariableTree;
4343
import com.sun.tools.javac.code.Symbol;
4444
import com.sun.tools.javac.code.Symbol.VarSymbol;
4545
import java.util.List;
46-
import java.util.stream.Stream;
4746
import org.jspecify.annotations.Nullable;
4847

4948
/** A BugPattern; see the summary. */
@@ -60,68 +59,116 @@ public final class AnnotateFormatMethod extends BugChecker implements MethodInvo
6059
" (The parameters of this method would need to be reordered to make the format string and "
6160
+ "arguments the final parameters before the @FormatMethod annotation can be used.)";
6261

63-
private static final Matcher<ExpressionTree> STRING_FORMAT =
64-
staticMethod().onClass("java.lang.String").named("format");
65-
private static final Matcher<ExpressionTree> FORMATTED =
66-
instanceMethod().onExactClass("java.lang.String").named("formatted");
67-
6862
@Override
6963
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
70-
VarSymbol formatString;
71-
VarSymbol formatArgs;
72-
if (STRING_FORMAT.matches(tree, state)) {
73-
if (tree.getArguments().size() != 2) {
74-
return NO_MATCH;
75-
}
76-
formatString = asSymbol(tree.getArguments().get(0));
77-
formatArgs = asSymbol(tree.getArguments().get(1));
78-
} else if (FORMATTED.matches(tree, state)) {
79-
if (tree.getArguments().size() != 1) {
80-
return NO_MATCH;
64+
FormatMethodArguments args = getFormatMethodArguments(tree, state);
65+
if (args == null) {
66+
return NO_MATCH;
67+
}
68+
if (args.arguments().size() < 2) {
69+
return NO_MATCH;
70+
}
71+
VarSymbol formatString = asSymbol(args.arguments().get(0));
72+
if (formatString == null) {
73+
return NO_MATCH;
74+
}
75+
for (Tree enclosing : state.getPath()) {
76+
if (enclosing instanceof MethodTree methodTree) {
77+
Description description = matchEnclosingMethod(state, methodTree, formatString, args);
78+
if (description != NO_MATCH) {
79+
return description;
80+
}
8181
}
82-
formatString = asSymbol(getReceiver(tree));
83-
formatArgs = asSymbol(tree.getArguments().get(0));
84-
} else {
82+
}
83+
return NO_MATCH;
84+
}
85+
86+
private static @Nullable FormatMethodArguments getFormatMethodArguments(
87+
MethodInvocationTree tree, VisitorState state) {
88+
ImmutableList<ExpressionTree> args = FormatStringUtils.formatMethodArguments(tree, state);
89+
if (!args.isEmpty()) {
90+
return new FormatMethodArguments(false, args);
91+
}
92+
int index = LenientFormatStringUtils.getLenientFormatStringPosition(tree, state);
93+
if (index != -1) {
94+
return new FormatMethodArguments(
95+
true,
96+
ImmutableList.copyOf(tree.getArguments().subList(index, tree.getArguments().size())));
97+
}
98+
return null;
99+
}
100+
101+
private Description matchEnclosingMethod(
102+
VisitorState state, Tree node, VarSymbol formatString, FormatMethodArguments args) {
103+
if (!(node instanceof MethodTree methodTree)) {
85104
return NO_MATCH;
86105
}
87-
if (formatString == null || formatArgs == null) {
106+
if (hasAnnotation(methodTree, FORMAT_METHOD_ANNOTATION, state)) {
88107
return NO_MATCH;
89108
}
109+
List<? extends VariableTree> enclosingParameters = methodTree.getParameters();
110+
VariableTree formatParameter = findParameterWithSymbol(enclosingParameters, formatString);
111+
if (formatParameter == null) {
112+
return NO_MATCH;
113+
}
114+
if (hasAnnotation(formatParameter, FORMAT_STRING_ANNOTATION, state)
115+
|| hasAnnotation(formatParameter, LENIENT_FORMAT_STRING_ANNOTATION, state)) {
116+
return NO_MATCH;
117+
}
118+
if (args.lenient()) {
119+
return handleLenient(state, args.arguments(), methodTree, formatParameter);
120+
}
121+
if (!getSymbol(methodTree).isVarArgs()) {
122+
return NO_MATCH;
123+
}
124+
VarSymbol formatArgs = asSymbol(args.arguments().get(1));
125+
if (formatArgs == null) {
126+
return NO_MATCH;
127+
}
128+
VariableTree argumentsParameter = findParameterWithSymbol(enclosingParameters, formatArgs);
129+
if (argumentsParameter == null) {
130+
return NO_MATCH;
131+
}
132+
if (!argumentsParameter.equals(getLast(enclosingParameters))) {
133+
return NO_MATCH;
134+
}
135+
// We can only generate a fix if the format string is the penultimate parameter.
136+
boolean fixable =
137+
formatParameter.equals(enclosingParameters.get(enclosingParameters.size() - 2));
138+
return buildDescription(methodTree)
139+
.setMessage(fixable ? message() : (message() + REORDER))
140+
.build();
141+
}
90142

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);
143+
private Description handleLenient(
144+
VisitorState state,
145+
List<ExpressionTree> args,
146+
MethodTree methodTree,
147+
VariableTree formatParameter) {
148+
int formatParameterIndex = methodTree.getParameters().indexOf(formatParameter);
149+
if (args.size() != methodTree.getParameters().size() - formatParameterIndex) {
150+
return NO_MATCH;
151+
}
152+
if (args.size() == 1) {
153+
return NO_MATCH;
154+
}
155+
// Check that all the parameters after the format string are passed through in order.
156+
for (int i = 1; i < args.size(); i++) {
157+
if (!(getSymbol(args.get(i)) instanceof VarSymbol vs)
158+
|| !vs.equals(getSymbol(methodTree.getParameters().get(formatParameterIndex + i)))) {
159+
return NO_MATCH;
160+
}
161+
}
162+
SuggestedFix.Builder fix = SuggestedFix.builder();
163+
var lenientFormatString =
164+
SuggestedFixes.qualifyType(state, fix, LENIENT_FORMAT_STRING_ANNOTATION);
165+
fix.prefixWith(formatParameter, "@" + lenientFormatString + " ");
166+
return describeMatch(methodTree, fix.build());
122167
}
123168

124-
private static VariableTree findParameterWithSymbol(
169+
private record FormatMethodArguments(boolean lenient, ImmutableList<ExpressionTree> arguments) {}
170+
171+
private static @Nullable VariableTree findParameterWithSymbol(
125172
List<? extends VariableTree> parameters, Symbol symbol) {
126173
return parameters.stream()
127174
.filter(parameter -> symbol.equals(getSymbol(parameter)))

‎core/src/main/java/com/google/errorprone/bugpatterns/formatstring/FormatString.java‎

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@
1717
package com.google.errorprone.bugpatterns.formatstring;
1818

1919
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
20-
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
2120

2221
import com.google.common.collect.ImmutableList;
2322
import com.google.errorprone.BugPattern;
2423
import com.google.errorprone.VisitorState;
2524
import com.google.errorprone.bugpatterns.BugChecker;
2625
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
2726
import com.google.errorprone.matchers.Description;
28-
import com.google.errorprone.matchers.Matcher;
2927
import com.google.errorprone.util.ASTHelpers;
3028
import com.sun.source.tree.ExpressionTree;
3129
import com.sun.source.tree.MethodInvocationTree;
@@ -35,35 +33,13 @@
3533
@BugPattern(summary = "Invalid printf-style format string", severity = ERROR)
3634
public class FormatString extends BugChecker implements MethodInvocationTreeMatcher {
3735

38-
private static final Matcher<ExpressionTree> FORMATTED_METHOD =
39-
instanceMethod().onExactClass("java.lang.String").named("formatted");
40-
4136
@Override
4237
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
43-
ImmutableList<ExpressionTree> args;
44-
MethodSymbol sym = ASTHelpers.getSymbol(tree);
45-
46-
if (FORMATTED_METHOD.matches(tree, state)) {
47-
/*
48-
Java 15 and greater supports the formatted method on an instance of string. If found
49-
then use the string value as the pattern and all-of-the arguments and send directly to
50-
the validate method.
51-
*/
52-
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
53-
if (receiver == null) {
54-
// an unqualified call to 'formatted', possibly inside the definition
55-
// of java.lang.String
56-
return Description.NO_MATCH;
57-
}
58-
args =
59-
ImmutableList.<ExpressionTree>builder().add(receiver).addAll(tree.getArguments()).build();
60-
61-
} else {
62-
args = FormatStringUtils.formatMethodArguments(tree, state);
63-
}
38+
ImmutableList<ExpressionTree> args = FormatStringUtils.formatMethodArguments(tree, state);
6439
if (args.isEmpty()) {
6540
return Description.NO_MATCH;
6641
}
42+
MethodSymbol sym = ASTHelpers.getSymbol(tree);
6743
FormatStringValidation.ValidationResult result =
6844
FormatStringValidation.validate(sym, args, state);
6945
if (result == null) {

‎core/src/main/java/com/google/errorprone/bugpatterns/formatstring/FormatStringAnnotationChecker.java‎

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static com.google.errorprone.util.AnnotationNames.FORMAT_METHOD_ANNOTATION;
2525
import static com.google.errorprone.util.AnnotationNames.FORMAT_STRING_ANNOTATION;
2626

27+
import com.google.common.collect.ImmutableList;
2728
import com.google.errorprone.BugPattern;
2829
import com.google.errorprone.VisitorState;
2930
import com.google.errorprone.bugpatterns.BugChecker;
@@ -58,43 +59,21 @@ private Description matchInvocation(
5859
MethodSymbol symbol,
5960
List<? extends ExpressionTree> args,
6061
VisitorState state) {
61-
if (!hasAnnotation(symbol, FORMAT_METHOD_ANNOTATION, state)) {
62-
return Description.NO_MATCH;
63-
}
64-
65-
int formatString = formatStringIndex(symbol, state);
66-
if (formatString == -1) {
67-
// will be an error at call site
62+
ImmutableList<ExpressionTree> formatArgs =
63+
FormatStringUtils.formatMethodAnnotationArguments(tree, symbol, args, state);
64+
if (formatArgs.isEmpty()) {
6865
return NO_MATCH;
6966
}
70-
7167
FormatStringValidation.ValidationResult result =
7268
StrictFormatStringValidation.validate(
73-
args.get(formatString), args.subList(formatString + 1, args.size()), state);
74-
69+
formatArgs.get(0), formatArgs.subList(1, formatArgs.size()), state);
7570
if (result != null) {
7671
return buildDescription(tree).setMessage(result.message()).build();
7772
} else {
7873
return Description.NO_MATCH;
7974
}
8075
}
8176

82-
private static int formatStringIndex(MethodSymbol symbol, VisitorState state) {
83-
Type stringType = state.getSymtab().stringType;
84-
List<VarSymbol> params = symbol.getParameters();
85-
int firstStringIndex = -1;
86-
for (int i = 0; i < params.size(); i++) {
87-
VarSymbol param = params.get(i);
88-
if (hasAnnotation(param, FORMAT_STRING_ANNOTATION, state)) {
89-
return i;
90-
}
91-
if (firstStringIndex < 0 && isSameType(param.type, stringType, state)) {
92-
firstStringIndex = i;
93-
}
94-
}
95-
return firstStringIndex;
96-
}
97-
9877
@Override
9978
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
10079
return matchInvocation(tree, getSymbol(tree), tree.getArguments(), state);

0 commit comments

Comments
 (0)