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
PicnicSupermarket · Dec 15, 2023 · 654d1db · 654d1db
1 parent da7be27
commit 654d1db
Show file tree

Hide file tree

Showing 11 changed files with 95 additions and 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
@@ -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<Fix> toStringFix(
               .named("append")
               .withParameters("java.lang.Object"));
 
+  private static final Matcher<ExpressionTree> 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
@@ -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
@@ -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
@@ -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 [email protected] (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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<String> xs) {",
+            "    // BUG: Diagnostic contains:",
+            "    return Joiner.on(\"foo\").join(xs, xs);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }