Skip to content

Commit

Permalink
Handle Joiner.on(...) in AbstractToString.
Browse files Browse the repository at this point in the history
Not yet handling the Iterable overload; that's a bit more involved given the lack of prior art.

PiperOrigin-RevId: 591205788
  • Loading branch information
graememorgan authored and Error Prone Team committed Dec 15, 2023
1 parent da7be27 commit 654d1db
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,7 +68,9 @@ public class TreeToString extends AbstractToString {
.withParameters("java.lang.Object");

@Inject
TreeToString() {}
TreeToString(ErrorProneFlags flags) {
super(flags);
}

@Override
protected TypePredicate typePredicate() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit 654d1db

Please sign in to comment.