Skip to content

Commit

Permalink
Add a suggested fix to TreeToString by searching for an available Vis…
Browse files Browse the repository at this point in the history
…itorState.

(And move it to TreeToString to match the check name.)

RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=227496488
  • Loading branch information
graememorgan authored and eaftan committed Jan 3, 2019
1 parent 11a1dfe commit 0a92a63
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 12 deletions.
Expand Up @@ -18,32 +18,39 @@

import static com.google.errorprone.BugPattern.ProvidesFix.NO_FIX;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.isSubtype;

import com.google.common.base.Optional;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.predicates.TypePredicate;
import com.google.errorprone.predicates.TypePredicates;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.FindIdentifiers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;

/**
* Flags com.sun.source.tree.Tree#toString usage in {@link BugChecker}s.
* Flags {@code com.sun.source.tree.Tree#toString} usage in {@link BugChecker}s.
*
* @author bhagwani@google.com (Sumit Bhagwani)
*/
@BugPattern(
name = "TreeToString",
summary = "Tree.toString shouldn't be used",
summary =
"Tree#toString shouldn't be used for Trees deriving from the code being "
+ "compiled, as it discards whitespace and comments.",
severity = WARNING,
providesFix = NO_FIX)
public class TreeToStringChecker extends AbstractToString {
public class TreeToString extends AbstractToString {

private static final Matcher<ClassTree> IS_BUGCHECKER =
Matchers.isSubtypeOf("com.google.errorprone.bugpatterns.BugChecker");
Expand All @@ -61,7 +68,7 @@ private static boolean treeToStringInBugChecker(Type type, VisitorState state) {

@Override
protected TypePredicate typePredicate() {
return TreeToStringChecker::treeToStringInBugChecker;
return TreeToString::treeToStringInBugChecker;
}

@Override
Expand All @@ -71,11 +78,36 @@ protected Optional<String> descriptionMessageForDefaultMatch(Type type, VisitorS

@Override
protected Optional<Fix> implicitToStringFix(ExpressionTree tree, VisitorState state) {
return Optional.absent();
return fix(tree, tree, state);
}

@Override
protected Optional<Fix> toStringFix(Tree parent, ExpressionTree tree, VisitorState state) {
return Optional.absent();
if (!(parent instanceof MethodInvocationTree)) {
return Optional.absent();
}
ExpressionTree receiver = getReceiver((ExpressionTree) parent);
if (receiver == null) {
return Optional.absent();
}
return fix(receiver, parent, state);
}

private static Optional<Fix> fix(Tree target, Tree replace, VisitorState state) {
return Optional.fromJavaUtil(
FindIdentifiers.findAllIdents(state).stream()
.filter(
s ->
isSubtype(
s.type,
state.getTypeFromString("com.google.errorprone.VisitorState"),
state))
.findFirst()
.map(
s ->
SuggestedFix.replace(
replace,
String.format(
"%s.getSourceForNode(%s)", s, state.getSourceForNode(target)))));
}
}
Expand Up @@ -252,7 +252,7 @@
import com.google.errorprone.bugpatterns.ThrowNull;
import com.google.errorprone.bugpatterns.ThrowsUncheckedException;
import com.google.errorprone.bugpatterns.ToStringReturnsNull;
import com.google.errorprone.bugpatterns.TreeToStringChecker;
import com.google.errorprone.bugpatterns.TreeToString;
import com.google.errorprone.bugpatterns.TruthAssertExpected;
import com.google.errorprone.bugpatterns.TruthConstantAsserts;
import com.google.errorprone.bugpatterns.TruthSelfEquals;
Expand Down Expand Up @@ -686,7 +686,7 @@ public static ScannerSupplier errorChecks() {
ThreeLetterTimeZoneID.class,
TimeUnitConversionChecker.class,
ToStringReturnsNull.class,
TreeToStringChecker.class,
TreeToString.class,
TruthAssertExpected.class,
TruthConstantAsserts.class,
TruthIncompatibleType.class,
Expand Down
Expand Up @@ -21,11 +21,11 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Unit tests for {@link TreeToStringChecker}. */
/** Unit tests for {@link TreeToString}. */
@RunWith(JUnit4.class)
public class TreeToStringCheckerTest {
public class TreeToStringTest {
private final CompilationTestHelper testHelper =
CompilationTestHelper.newInstance(TreeToStringChecker.class, getClass());
CompilationTestHelper.newInstance(TreeToString.class, getClass());

@Test
public void noMatch() {
Expand Down Expand Up @@ -70,7 +70,7 @@ public void matchInABugChecker() {
"@BugPattern(name = \"Example\", summary = \"\", severity = SeverityLevel.ERROR)",
"public class ExampleChecker extends BugChecker implements ClassTreeMatcher {",
" @Override public Description matchClass(ClassTree tree, VisitorState state) {",
" // BUG: Diagnostic contains: TreeToString",
" // BUG: Diagnostic contains: state.getSourceForNode(tree).contains",
" if (tree.toString().contains(\"match\")) {",
" return describeMatch(tree);",
" }",
Expand Down

0 comments on commit 0a92a63

Please sign in to comment.