Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.errorprone.bugpatterns;

import static com.google.errorprone.VisitorState.memoize;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
Expand All @@ -30,21 +31,27 @@
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.CompoundAssignmentTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MemberReferenceTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.predicates.TypePredicate;
import com.google.errorprone.suppliers.Supplier;
import com.sun.source.tree.BinaryTree;
import com.sun.source.tree.CompoundAssignmentTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.TreePath;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.ArrayType;
import com.sun.tools.javac.code.Type.MethodType;
import com.sun.tools.javac.util.Name;
import java.util.List;
import java.util.Optional;
import javax.lang.model.type.TypeKind;
Expand All @@ -56,7 +63,10 @@
* <p>See examples in {@link StreamToString} and {@link ArrayToString}.
*/
public abstract class AbstractToString extends BugChecker
implements BinaryTreeMatcher, MethodInvocationTreeMatcher, CompoundAssignmentTreeMatcher {
implements BinaryTreeMatcher,
MethodInvocationTreeMatcher,
MemberReferenceTreeMatcher,
CompoundAssignmentTreeMatcher {

/** The type to match on. */
protected abstract TypePredicate typePredicate();
Expand Down Expand Up @@ -93,6 +103,37 @@ protected boolean allowableToStringKind(ToStringKind toStringKind) {
protected abstract Optional<Fix> toStringFix(
Tree toStringCall, ExpressionTree stringifiedExpr, VisitorState state);

/**
* Constructs a fix for a method reference to {@code toString()}, e.g. {@code Object::toString}.
*
* @param receiverType the type of the object {@code toString()} will be called on
*/
protected Optional<Fix> memberReferenceFix(
MemberReferenceTree tree, Type receiverType, VisitorState state) {
return Optional.empty();
}

private static final Supplier<Name> TO_STRING_NAME = memoize(state -> state.getName("toString"));

private static final Supplier<Symbol> STRING_VALUE_OF_OBJECT =
memoize(
state -> {
var stringSymbol = state.getSymtab().stringType.tsym;
for (Symbol sym : stringSymbol.members().getSymbolsByName(state.getName("valueOf"))) {
if (sym instanceof MethodSymbol methodSym
&& methodSym.isStatic()
&& methodSym.getParameters().size() == 1
&& state
.getTypes()
.isSameType(
methodSym.getParameters().get(0).asType(),
state.getSymtab().objectType)) {
return methodSym;
}
}
return null;
});

private static final Matcher<ExpressionTree> TO_STRING =
instanceMethod().anyClass().named("toString").withNoParameters();

Expand Down Expand Up @@ -135,10 +176,12 @@ protected abstract Optional<Fix> toStringFix(

private final boolean handleJoinerVarargs;
private final boolean handleJoinerIterable;
private final boolean handleMemberReference;

protected AbstractToString(ErrorProneFlags flags) {
this.handleJoinerVarargs = flags.getBoolean("AbstractToString:Joiner").orElse(true);
this.handleJoinerIterable = flags.getBoolean("AbstractToString:JoinerIterable").orElse(true);
this.handleMemberReference = flags.getBoolean("AbstractToString:MemberReference").orElse(true);
}

private static boolean isInVarargsPosition(
Expand Down Expand Up @@ -231,6 +274,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
elementType.getTypeArguments().getFirst(),
argTree,
argTree,
// TODO(cpovirk): Suggest a fix.
ToStringKind.NONE,
state);
}
Expand All @@ -239,6 +283,41 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
return NO_MATCH;
}

@Override
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
if (!handleMemberReference) {
return NO_MATCH;
}

MethodSymbol symbol = getSymbol(tree);
if (symbol == null || !isToStringOrIsStringValueOf(symbol, state)) {
return NO_MATCH;
}

Type targetType = getType(tree);
if (targetType == null) {
return NO_MATCH;
}

Type descriptorType = state.getTypes().findDescriptorType(targetType);
if (descriptorType == null || descriptorType.getParameterTypes().isEmpty()) {
return NO_MATCH;
}

Type receiverType = descriptorType.getParameterTypes().get(0);
if (typePredicate().apply(receiverType, state)) {
handleStringifiedTree(receiverType, tree, tree, ToStringKind.MEMBER_REFERENCE, state);
}

return NO_MATCH;
}

private boolean isToStringOrIsStringValueOf(MethodSymbol symbol, VisitorState state) {
return (symbol.getSimpleName().equals(TO_STRING_NAME.get(state))
&& symbol.getParameters().isEmpty())
|| symbol.equals(STRING_VALUE_OF_OBJECT.get(state));
}

@Override
public Description matchBinary(BinaryTree tree, VisitorState state) {
if (!state.getTypes().isSameType(getType(tree), state.getSymtab().stringType)) {
Expand Down Expand Up @@ -293,7 +372,7 @@ private void handleStringifiedTree(
stringifiedExpr,
state,
type,
getFix(stringifiedExpr, state, toStringCall, toStringKind)));
getFix(type, stringifiedExpr, state, toStringCall, toStringKind)));
}

private static Type type(ExpressionTree tree) {
Expand All @@ -305,13 +384,15 @@ private static Type type(ExpressionTree tree) {
}

private Optional<Fix> getFix(
Type type,
ExpressionTree stringifiedExpr,
VisitorState state,
Tree toStringCall,
ToStringKind toStringKind) {
return switch (toStringKind) {
case IMPLICIT, FLOGGER, FORMAT_METHOD -> implicitToStringFix(stringifiedExpr, state);
case EXPLICIT -> toStringFix(toStringCall, stringifiedExpr, state);
case MEMBER_REFERENCE -> memberReferenceFix((MemberReferenceTree) toStringCall, type, state);
case NONE -> Optional.empty();
};
}
Expand All @@ -331,6 +412,7 @@ enum ToStringKind {
EXPLICIT,
FORMAT_METHOD,
FLOGGER,
MEMBER_REFERENCE,
NONE,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.fixes.SuggestedFixes.qualifyType;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.getType;

import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
Expand All @@ -27,8 +30,8 @@
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.predicates.TypePredicate;
import com.google.errorprone.predicates.TypePredicates;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
Expand Down Expand Up @@ -77,14 +80,14 @@ protected Optional<Fix> toStringFix(
// If the array is the result of calling e.getStackTrace(), replace
// e.getStackTrace().toString() with Guava's Throwables.getStackTraceAsString(e).
if (GET_STACK_TRACE.matches(stringifiedExpr, state)) {
var fix = SuggestedFix.builder();
return Optional.of(
SuggestedFix.builder()
.addImport("com.google.common.base.Throwables")
.replace(
fix.replace(
toStringCall,
String.format(
"Throwables.getStackTraceAsString(%s)",
state.getSourceForNode(ASTHelpers.getReceiver(stringifiedExpr))))
"%s.getStackTraceAsString(%s)",
qualifyType(state, fix, "com.google.common.base.Throwables"),
state.getSourceForNode(getReceiver(stringifiedExpr))))
.build());
}
// e.g. String.valueOf(theArray) -> Arrays.toString(theArray)
Expand All @@ -93,21 +96,31 @@ protected Optional<Fix> toStringFix(
return fix(toStringCall, stringifiedExpr, state);
}

private static Optional<Fix> fix(Tree toStringCall, Tree stringifiedExpr, VisitorState state) {
String method = isNestedArray(stringifiedExpr, state) ? "deepToString" : "toString";
@Override
protected Optional<Fix> memberReferenceFix(
MemberReferenceTree tree, Type receiverType, VisitorState state) {
String method = isNestedArray(receiverType, state) ? "deepToString" : "toString";
var fix = SuggestedFix.builder();
return Optional.of(
fix.replace(tree, qualifyType(state, fix, "java.util.Arrays") + "::" + method).build());
}

private static Optional<Fix> fix(Tree toStringCall, Tree stringifiedExpr, VisitorState state) {
String method = isNestedArray(getType(stringifiedExpr), state) ? "deepToString" : "toString";
var fix = SuggestedFix.builder();
return Optional.of(
SuggestedFix.builder()
.addImport("java.util.Arrays")
.replace(
fix.replace(
toStringCall,
String.format("Arrays.%s(%s)", method, state.getSourceForNode(stringifiedExpr)))
String.format(
"%s.%s(%s)",
qualifyType(state, fix, "java.util.Arrays"),
method,
state.getSourceForNode(stringifiedExpr)))
.build());
}

private static boolean isNestedArray(Tree stringifiedExpr, VisitorState state) {
private static boolean isNestedArray(Type type, VisitorState state) {
Types types = state.getTypes();
Type withType = ASTHelpers.getType(stringifiedExpr);
return withType != null && types.isArray(withType) && types.isArray(types.elemtype(withType));
return type != null && types.isArray(type) && types.isArray(types.elemtype(type));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -487,4 +487,53 @@ String test(Joiner j, List<int[]> a) {
""")
.doTest();
}

@Test
public void methodReference() {
refactoringHelper
.addInputLines(
"Test.java",
"""
import static java.util.stream.Collectors.joining;

import java.util.stream.Stream;

class Test {
void f(Stream<int[]> s) {
s.map(Object::toString).collect(joining(", "));
}

void g(Stream<int[][]> s) {
s.map(Object::toString).collect(joining(", "));
}

void h(Stream<int[]> s) {
s.map(String::valueOf).collect(joining(", "));
}
}
""")
.addOutputLines(
"Test.java",
"""
import static java.util.stream.Collectors.joining;

import java.util.Arrays;
import java.util.stream.Stream;

class Test {
void f(Stream<int[]> s) {
s.map(Arrays::toString).collect(joining(", "));
}

void g(Stream<int[][]> s) {
s.map(Arrays::deepToString).collect(joining(", "));
}

void h(Stream<int[]> s) {
s.map(Arrays::toString).collect(joining(", "));
}
}
""")
.doTest();
}
}
Loading