Automated rollback of commit 654d1db.

*** Reason for rollback *** That did not handle varargs well: specifically array arguments in a varargs position. I'll fix and roll forward. *** Original change description *** 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: 591308763
PicnicSupermarket · Dec 15, 2023 · d272dfa · d272dfa
1 parent 654d1db
commit d272dfa
Show file tree

Hide file tree

Showing 11 changed files with 1 addition and 95 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
@@ -24,9 +24,7 @@
 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;
@@ -117,15 +115,6 @@ 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();
@@ -177,14 +166,6 @@ 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,7 +20,6 @@
 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;
@@ -29,7 +28,6 @@
 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(
@@ -42,11 +40,6 @@ 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,7 +20,6 @@
 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;
@@ -29,7 +28,6 @@
 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(
@@ -42,11 +40,6 @@ 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,7 +20,6 @@
 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;
@@ -33,7 +32,6 @@
 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)
@@ -49,11 +47,6 @@ 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,7 +27,6 @@
 
 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;
@@ -36,7 +35,6 @@
 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(
@@ -71,11 +69,6 @@ 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,7 +20,6 @@
 
 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;
@@ -33,7 +32,6 @@
 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
@@ -74,11 +72,6 @@ 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,14 +20,12 @@
 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(
@@ -37,11 +35,6 @@ 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,7 +23,6 @@
 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;
@@ -33,7 +32,6 @@
 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.
@@ -60,11 +58,6 @@ 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,7 +24,6 @@
 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;
@@ -68,9 +67,7 @@ public class TreeToString extends AbstractToString {
           .withParameters("java.lang.Object");
 
   @Inject
-  TreeToString(ErrorProneFlags flags) {
-    super(flags);
-  }
+  TreeToString() {}
 
   @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,7 +23,6 @@
 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;
@@ -33,7 +32,6 @@
 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.
@@ -60,11 +58,6 @@ 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,20 +82,4 @@ 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();
-  }
 }