From 654d1dbf1e6dd652cd6e8ca003643ddf02266ec2 Mon Sep 17 00:00:00 2001 From: ghm Date: Fri, 15 Dec 2023 03:52:49 -0800 Subject: [PATCH] Handle Joiner.on(...) in AbstractToString. Not yet handling the Iterable overload; that's a bit more involved given the lack of prior art. PiperOrigin-RevId: 591205788 --- .../bugpatterns/AbstractToString.java | 19 +++++++++++++++++++ .../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/StreamToStringTest.java | 16 ++++++++++++++++ 11 files changed, 95 insertions(+), 1 deletion(-) 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 3be2fb60fe9..1096115ce73 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java @@ -24,7 +24,9 @@ import static com.google.errorprone.util.ASTHelpers.getReceiver; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; +import static com.google.errorprone.util.ASTHelpers.isSubtype; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.annotations.FormatMethod; import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; @@ -115,6 +117,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(); @@ -166,6 +177,14 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState handleStringifiedTree(argTree, ToStringKind.FLOGGER, state); } } + if (handleJoiner && JOINER.matches(tree, state)) { + var symbol = getSymbol(tree); + if (!isSubtype(symbol.params().get(0).type, state.getSymtab().iterableType, state)) { + for (ExpressionTree argTree : tree.getArguments()) { + handleStringifiedTree(argTree, ToStringKind.IMPLICIT, state); + } + } + } return NO_MATCH; } 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/StreamToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StreamToStringTest.java index b2250ac6c35..d726635dc76 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,20 @@ 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 String s(Stream xs) {", + " // BUG: Diagnostic contains:", + " return Joiner.on(\"foo\").join(xs, xs);", + " }", + "}") + .doTest(); + } }