From 0bd743234ba82f42c3aaa6dce28c5acd76d3dc03 Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 6 Feb 2024 14:08:02 -0800 Subject: [PATCH] Rollforward of https://github.com/google/error-prone/commit/654d1dbf1e6dd652cd6e8ca003643ddf02266ec2: Handle Joiner.on(...) in AbstractToString. Not yet handling the Iterable overload; that's a bit more involved given the lack of prior art. Handily there was a 'isInVarargsPosition' _right there_. PiperOrigin-RevId: 604758974 --- .../bugpatterns/AbstractToString.java | 44 ++++++++++++++++++- .../bugpatterns/AnnotationMirrorToString.java | 7 +++ .../bugpatterns/AnnotationValueToString.java | 7 +++ .../errorprone/bugpatterns/ArrayToString.java | 7 +++ .../bugpatterns/LiteProtoToString.java | 7 +++ .../bugpatterns/ObjectToString.java | 7 +++ .../bugpatterns/StreamToString.java | 7 +++ .../bugpatterns/SymbolToString.java | 7 +++ .../errorprone/bugpatterns/TreeToString.java | 5 ++- .../errorprone/bugpatterns/TypeToString.java | 7 +++ .../bugpatterns/ArrayToStringTest.java | 14 ++++++ .../bugpatterns/StreamToStringTest.java | 18 ++++++++ 12 files changed, 135 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java index c87e48b91e6..94feba2dc59 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java @@ -25,6 +25,7 @@ import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.annotations.FormatMethod; import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; @@ -42,6 +43,7 @@ import com.sun.source.tree.Tree.Kind; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Type; +import com.sun.tools.javac.code.Type.ArrayType; import com.sun.tools.javac.code.Type.MethodType; import java.util.List; import java.util.Optional; @@ -117,6 +119,15 @@ protected abstract Optional toStringFix( .named("append") .withParameters("java.lang.Object")); + private static final Matcher JOINER = + instanceMethod().onDescendantOf("com.google.common.base.Joiner").named("join"); + + private final boolean handleJoiner; + + protected AbstractToString(ErrorProneFlags flags) { + this.handleJoiner = flags.getBoolean("AbstractToString:Joiner").orElse(true); + } + private static boolean isInVarargsPosition( ExpressionTree argTree, MethodInvocationTree methodInvocationTree, VisitorState state) { int parameterCount = getSymbol(methodInvocationTree).getParameters().size(); @@ -127,6 +138,17 @@ private static boolean isInVarargsPosition( && arguments.indexOf(argTree) >= parameterCount - 1; } + private static boolean isVarargsArray( + ExpressionTree argTree, MethodInvocationTree methodInvocationTree, VisitorState state) { + int parameterCount = getSymbol(methodInvocationTree).getParameters().size(); + List arguments = methodInvocationTree.getArguments(); + // Don't match if we're passing an array into a varargs parameter, but do match if there are + // other parameters along with it. + return arguments.size() == parameterCount + && state.getTypes().isArray(getType(argTree)) + && arguments.indexOf(argTree) == parameterCount - 1; + } + @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { if (PRINT_STRING.matches(tree, state)) { @@ -168,6 +190,22 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState handleStringifiedTree(argTree, ToStringKind.FLOGGER, state); } } + if (handleJoiner && JOINER.matches(tree, state)) { + var symbol = getSymbol(tree); + if (symbol.isVarArgs()) { + for (ExpressionTree argTree : tree.getArguments()) { + if (isVarargsArray(argTree, tree, state)) { + handleStringifiedTree( + ((ArrayType) getType(argTree)).getComponentType(), + argTree, + argTree, + ToStringKind.IMPLICIT, + state); + } + handleStringifiedTree(argTree, ToStringKind.IMPLICIT, state); + } + } + } return NO_MATCH; } @@ -202,7 +240,11 @@ private void handleStringifiedTree( private void handleStringifiedTree( Tree parent, ExpressionTree tree, ToStringKind toStringKind, VisitorState state) { - Type type = type(tree); + handleStringifiedTree(type(tree), parent, tree, toStringKind, state); + } + + private void handleStringifiedTree( + Type type, Tree parent, ExpressionTree tree, ToStringKind toStringKind, VisitorState state) { if (type.getKind() == TypeKind.NULL || !typePredicate().apply(type, state) || allowableToStringKind(toStringKind)) { diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationMirrorToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationMirrorToString.java index b7e01480775..f433ef5ce16 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationMirrorToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationMirrorToString.java @@ -20,6 +20,7 @@ import static com.google.errorprone.fixes.SuggestedFixes.qualifyType; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFix; @@ -28,6 +29,7 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.Tree; import java.util.Optional; +import javax.inject.Inject; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern( @@ -40,6 +42,11 @@ public class AnnotationMirrorToString extends AbstractToString { private static final TypePredicate TYPE_PREDICATE = TypePredicates.isExactType("javax.lang.model.element.AnnotationMirror"); + @Inject + AnnotationMirrorToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return TYPE_PREDICATE; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationValueToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationValueToString.java index c947fbab4ad..5343f243b03 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationValueToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AnnotationValueToString.java @@ -20,6 +20,7 @@ import static com.google.errorprone.fixes.SuggestedFixes.qualifyType; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFix; @@ -28,6 +29,7 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.Tree; import java.util.Optional; +import javax.inject.Inject; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern( @@ -40,6 +42,11 @@ public class AnnotationValueToString extends AbstractToString { private static final TypePredicate TYPE_PREDICATE = TypePredicates.isExactType("javax.lang.model.element.AnnotationValue"); + @Inject + AnnotationValueToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return TYPE_PREDICATE; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java index c868007ccd9..840d7d4d282 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java @@ -20,6 +20,7 @@ import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFix; @@ -32,6 +33,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import java.util.Optional; +import javax.inject.Inject; /** * @author adgar@google.com (Mike Edgar) @@ -47,6 +49,11 @@ public class ArrayToString extends AbstractToString { private static final TypePredicate IS_ARRAY = TypePredicates.isArray(); + @Inject + ArrayToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return IS_ARRAY; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java index 6bf2969052c..fa5908f3902 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.predicates.TypePredicate; @@ -35,6 +36,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Type; import java.util.Optional; +import javax.inject.Inject; /** Flags calls to {@code toString} on lite protos. */ @BugPattern( @@ -69,6 +71,11 @@ public final class LiteProtoToString extends AbstractToString { .add("v", "d", "i") .build(); + @Inject + LiteProtoToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return LiteProtoToString::matches; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java index 378580fa40e..543bbef59cf 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java @@ -20,6 +20,7 @@ import com.google.common.collect.Iterables; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFixes; @@ -32,6 +33,7 @@ import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Names; import java.util.Optional; +import javax.inject.Inject; /** * Warns against calling toString() on Objects which don't have toString() method overridden and @@ -72,6 +74,11 @@ private static boolean finalNoOverrides(Type type, VisitorState state) { && m.overrides(toString, type.tsym, types, /* checkResult= */ false))); } + @Inject + ObjectToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return ObjectToString::finalNoOverrides; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java index 42a6f92ece8..e19e57bf446 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java @@ -20,12 +20,14 @@ import static com.google.errorprone.predicates.TypePredicates.isDescendantOf; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.predicates.TypePredicate; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.Tree; import java.util.Optional; +import javax.inject.Inject; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @BugPattern( @@ -35,6 +37,11 @@ public class StreamToString extends AbstractToString { private static final TypePredicate STREAM = isDescendantOf("java.util.stream.Stream"); + @Inject + StreamToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return STREAM; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SymbolToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/SymbolToString.java index 9bbd0ff4a9a..8c24c74decc 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/SymbolToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/SymbolToString.java @@ -23,6 +23,7 @@ import static com.google.errorprone.util.ASTHelpers.isBugCheckerCode; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.matchers.Matcher; @@ -32,6 +33,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Type; import java.util.Optional; +import javax.inject.Inject; /** * Flags {@code com.sun.tools.javac.code.Symbol#toString} usage in {@link BugChecker}s. @@ -58,6 +60,11 @@ private static boolean symbolToStringInBugChecker(Type type, VisitorState state) return IS_SYMBOL.apply(type, state) && STRING_EQUALS.matches(parentTree, state); } + @Inject + SymbolToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return SymbolToString::symbolToStringInBugChecker; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java index 786e9d96754..3fda587d9a2 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java @@ -24,6 +24,7 @@ import static com.google.errorprone.util.ASTHelpers.isSubtype; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.SuggestedFix; @@ -67,7 +68,9 @@ public class TreeToString extends AbstractToString { .withParameters("java.lang.Object"); @Inject - TreeToString() {} + TreeToString(ErrorProneFlags flags) { + super(flags); + } @Override protected TypePredicate typePredicate() { diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypeToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypeToString.java index 212b5f6558a..a4196b13b46 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/TypeToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/TypeToString.java @@ -23,6 +23,7 @@ import static com.google.errorprone.util.ASTHelpers.isBugCheckerCode; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.fixes.Fix; import com.google.errorprone.matchers.Matcher; @@ -32,6 +33,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Type; import java.util.Optional; +import javax.inject.Inject; /** * Flags {@code com.sun.tools.javac.code.Type#toString} usage in {@link BugChecker}s. @@ -58,6 +60,11 @@ private static boolean typeToStringInBugChecker(Type type, VisitorState state) { return IS_TYPE.apply(type, state) && STRING_EQUALS.matches(parentTree, state); } + @Inject + TypeToString(ErrorProneFlags flags) { + super(flags); + } + @Override protected TypePredicate typePredicate() { return TypeToString::typeToStringInBugChecker; diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ArrayToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ArrayToStringTest.java index 972a9c852c8..1014d8798c2 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/ArrayToStringTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/ArrayToStringTest.java @@ -185,4 +185,18 @@ public void positiveConcat() { public void negativeConcat() { compilationHelper.addSourceFile("ArrayToStringConcatenationNegativeCases.java").doTest(); } + + @Test + public void arrayPassedToJoiner() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.base.Joiner;", + "class Test {", + " String test(Joiner j, Object[] a) {", + " return j.join(a);", + " }", + "}") + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StreamToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StreamToStringTest.java index b2250ac6c35..b5512269643 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StreamToStringTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/StreamToStringTest.java @@ -82,4 +82,22 @@ public void withinStreamClass() { "}") .doTest(); } + + @Test + public void viaJoiner() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.base.Joiner;", + "import java.util.stream.Stream;", + "class Test {", + " public void s(Stream xs) {", + " // BUG: Diagnostic contains:", + " var x = Joiner.on(\"foo\").join(xs, xs);", + " // BUG: Diagnostic contains:", + " var y = Joiner.on(\"foo\").join(null, null, new Stream[]{xs});", + " }", + "}") + .doTest(); + } }