Skip to content

Commit

Permalink
Delete type matchers that take Types or Trees
Browse files Browse the repository at this point in the history 
Matches should not hold on to state from the current compilation.

MOE_MIGRATED_REVID=127225038
  • Loading branch information
@cushon
cushon committed Jul 13, 2016
1 parent 6fa3d89 commit 749671e
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 128 deletions.
39 changes: 7 additions & 32 deletions core/src/main/java/com/google/errorprone/bugpatterns/ComparisonOutOfRange.java
View file
Expand Up @@ -29,12 +29,13 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;

import com.sun.source.tree.BinaryTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCLiteral;
Expand Down Expand Up @@ -67,54 +68,28 @@ public class ComparisonOutOfRange extends BugChecker implements BinaryTreeMatche
* type of comparison (byte or char).
*/
private static class BadComparisonMatcher implements Matcher<BinaryTree> {
/**
* The type of bad comparison matcher to create. Must be either Byte.TYPE or Character.TYPE.
*/
private final Class<?> type;

private boolean initialized = false;
private Type comparisonType;
private int maxValue;
private int minValue;
private final Supplier<Type> comparisonType;
private final int maxValue;
private final int minValue;

public BadComparisonMatcher(Class<?> type) {
if (type != Byte.TYPE && type != Character.TYPE) {
throw new IllegalArgumentException("type must be either byte or char, but was "
+ type.getName());
}
this.type = type;
}

/**
* Initialize matcher for the specific type. We can't do this in the constructor because
* we need the symbol table, which isn't available at that time.
*
* @param symbolTable The compiler's symbol table
*/
private void init(Symtab symbolTable) {
if (initialized) {
throw new IllegalStateException("Do not try to initialize twice!");
}

// Specialize matcher based on type.
if (type == Byte.TYPE) {
comparisonType = symbolTable.byteType;
comparisonType = Suppliers.BYTE_TYPE;
maxValue = Byte.MAX_VALUE;
minValue = Byte.MIN_VALUE;
} else {
comparisonType = symbolTable.charType;
comparisonType = Suppliers.CHAR_TYPE;
maxValue = Character.MAX_VALUE;
minValue = Character.MIN_VALUE;
}
initialized = true;
}

@Override
public boolean matches(BinaryTree tree, VisitorState state) {
if (!initialized) {
init(state.getSymtab());
}

// Must be an == or != comparison.
if (tree.getKind() != Kind.EQUAL_TO && tree.getKind() != Kind.NOT_EQUAL_TO) {
return false;
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/com/google/errorprone/bugpatterns/StringBuilderInitWithChar.java
View file
Expand Up @@ -23,13 +23,12 @@
import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.util.ASTHelpers;

import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LiteralTree;
import com.sun.source.tree.NewClassTree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.util.Convert;
Expand All @@ -47,8 +46,9 @@
public class StringBuilderInitWithChar extends BugChecker implements NewClassTreeMatcher {
@Override
public Description matchNewClass(NewClassTree tree, VisitorState state) {
if (Matchers.isSameType(Symtab.instance(state.context).stringBuilderType).matches(
tree.getIdentifier(), state) && tree.getArguments().size() == 1) {
if (ASTHelpers.isSameType(
state.getSymtab().stringBuilderType, ASTHelpers.getType(tree.getIdentifier()), state)
&& tree.getArguments().size() == 1) {
ExpressionTree argument = tree.getArguments().get(0);
Type type = ((JCTree) argument).type;
if (type.getKind() == TypeKind.CHAR) {
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/com/google/errorprone/bugpatterns/inject/guice/AssistedParameters.java
View file
Expand Up @@ -34,6 +34,7 @@
import com.sun.tools.javac.code.Attribute.Compound;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Types;

import javax.lang.model.element.TypeElement;

Expand Down Expand Up @@ -82,7 +83,8 @@ public final Description matchVariable(VariableTree variableTree, VisitorState s
// will be iterating through all parameters including the one we're matching.
int numIdentical = 0;
for (VariableTree parameter : enclosingMethod.getParameters()) {
if (Matchers.<VariableTree>isSameType(variableTree).matches(parameter, state)) {
Types types = state.getTypes();
if (types.isSameType(ASTHelpers.getType(variableTree), ASTHelpers.getType(parameter))) {
Compound otherParamsAssisted = null;
for (Compound c : ASTHelpers.getSymbol(parameter).getAnnotationMirrors()) {
if (((TypeElement) c.getAnnotationType().asElement())
Expand Down
10 changes: 0 additions & 10 deletions core/src/main/java/com/google/errorprone/matchers/AbstractTypeMatcher.java
View file
Expand Up @@ -16,15 +16,13 @@

package com.google.errorprone.matchers;

import static com.google.errorprone.suppliers.Suppliers.identitySupplier;
import static com.google.errorprone.suppliers.Suppliers.typeFromString;

import com.google.errorprone.VisitorState;
import com.google.errorprone.suppliers.Supplier;

import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;

/**
* Base class for type matchers.
Expand All @@ -37,14 +35,6 @@ public AbstractTypeMatcher(Supplier<Type> typeToCompareSupplier) {
this.typeToCompareSupplier = typeToCompareSupplier;
}

public AbstractTypeMatcher(Type typeToCompare) {
this(identitySupplier(typeToCompare));
}

public AbstractTypeMatcher(Tree tree) {
this(((JCTree) tree).type);
}

public AbstractTypeMatcher(String typeString) {
this(typeFromString(typeString));
}
Expand Down
8 changes: 0 additions & 8 deletions core/src/main/java/com/google/errorprone/matchers/IsCastableTo.java
View file
Expand Up @@ -37,14 +37,6 @@ public IsCastableTo(String typeString) {
super(typeString);
}

public IsCastableTo(Tree tree) {
super(tree);
}

public IsCastableTo(Type typeToCompare) {
super(typeToCompare);
}

@Override
public boolean matches(T tree, VisitorState state) {
Types types = state.getTypes();
Expand Down
8 changes: 0 additions & 8 deletions core/src/main/java/com/google/errorprone/matchers/IsSameType.java
View file
Expand Up @@ -37,14 +37,6 @@ public IsSameType(String typeString) {
super(typeString);
}

public IsSameType(Tree tree) {
super(tree);
}

public IsSameType(Type typeToCompare) {
super(typeToCompare);
}

@Override
public boolean matches(T tree, VisitorState state) {
Types types = state.getTypes();
Expand Down
8 changes: 0 additions & 8 deletions core/src/main/java/com/google/errorprone/matchers/IsSubtypeOf.java
View file
Expand Up @@ -37,14 +37,6 @@ public IsSubtypeOf(String typeString) {
super(typeString);
}

public IsSubtypeOf(Tree tree) {
super(tree);
}

public IsSubtypeOf(Type typeToCompare) {
super(typeToCompare);
}

@Override
public boolean matches(T tree, VisitorState state) {
Types types = state.getTypes();
Expand Down
33 changes: 1 addition & 32 deletions core/src/main/java/com/google/errorprone/matchers/Matchers.java
View file
Expand Up @@ -439,15 +439,6 @@ public static <T extends Tree> Matcher<T> isSubtypeOf(String typeStr) {
return new IsSubtypeOf<>(typeStr);
}

/**
* Matches an AST node if its type is a subtype of the given type.
*
* @param type the type to check against
*/
public static <T extends Tree> Matcher<T> isSubtypeOf(Type type) {
return new IsSubtypeOf<>(type);
}

/**
* Matches an AST node if its type is a subtype of the given type.
*
Expand All @@ -465,7 +456,7 @@ public static <T extends Tree> Matcher<T> isSubtypeOf(Supplier<Type> type) {
public static <T extends Tree> Matcher<T> isSubtypeOf(Class<?> clazz) {
return new IsSubtypeOf<>(typeFromClass(clazz));
}

/**
* Matches an AST node if its type is castable to the given type.
*
Expand All @@ -484,15 +475,6 @@ public static <T extends Tree> Matcher<T> isCastableTo(Supplier<Type> typeSuppli
return new IsCastableTo<>(typeSupplier);
}

/**
* Matches an AST node if its type is the same as the given type.
*
* @param type the type to check against
*/
public static <T extends Tree> Matcher<T> isSameType(Type type) {
return new IsSameType<>(type);
}

/**
* Matches an AST node if its type is the same as the given type.
*
Expand All @@ -511,15 +493,6 @@ public static <T extends Tree> Matcher<T> isSameType(String typeString) {
return new IsSameType<>(typeString);
}

/**
* Matches an AST node if its type is the same as the type of the given tree.
*
* @param tree an AST node whose type to check against
*/
public static <T extends Tree> Matcher<T> isSameType(Tree tree) {
return new IsSameType<>(tree);
}

/**
* Matches an AST node if its type is an array type.
*/
Expand Down Expand Up @@ -849,10 +822,6 @@ public boolean matches(MethodTree methodTree, VisitorState state) {
};
}

public static Matcher<MethodTree> methodReturns(final Type returnType) {
return methodReturns(isSameType(returnType));
}

public static Matcher<MethodTree> methodReturns(final Supplier<Type> returnType) {
return methodReturns(isSameType(returnType));
}
Expand Down
13 changes: 7 additions & 6 deletions core/src/main/java/com/google/errorprone/suppliers/Suppliers.java
View file
Expand Up @@ -225,12 +225,13 @@ public T get(VisitorState state) {
};
}

public static Supplier<Type> ENCLOSING_CLASS = new Supplier<Type>() {
@Override
public Type get(VisitorState state) {
return ((JCTree) state.findEnclosing(ClassTree.class)).type;
}
};
public static final Supplier<Type> ENCLOSING_CLASS =
new Supplier<Type>() {
@Override
public Type get(VisitorState state) {
return ((JCTree) state.findEnclosing(ClassTree.class)).type;
}
};

public static Supplier<Type> arrayOf(final Supplier<Type> elementType) {
return new Supplier<Type>() {
Expand Down
36 changes: 17 additions & 19 deletions core/src/test/java/com/google/errorprone/matchers/MethodReturnsTest.java
View file
Expand Up @@ -16,15 +16,14 @@

package com.google.errorprone.matchers;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.errorprone.matchers.Matchers.methodReturns;
import static org.junit.Assert.assertEquals;

import com.google.errorprone.VisitorState;
import com.google.errorprone.scanner.Scanner;
import com.google.errorprone.suppliers.Suppliers;

import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Type;

import org.junit.After;
import org.junit.Test;
Expand Down Expand Up @@ -75,29 +74,28 @@ public void returnsInt() {
assertCompiles(fooReturnsType(true, "int"));
}

private abstract class ScannerTest extends Scanner {
private abstract static class ScannerTest extends Scanner {
abstract void assertDone();
}

private Scanner fooReturnsType(final boolean shouldMatch, final String typeString) {
ScannerTest test = new ScannerTest() {
private boolean matched = false;
ScannerTest test =
new ScannerTest() {
private boolean matched = false;

@Override
public Void visitMethod(MethodTree node, VisitorState visitorState) {
Type type = visitorState.getTypeFromString(typeString);
checkNotNull(type, "Unrecognized type %s", typeString);
if (methodReturns(type).matches(node, visitorState)) {
matched = true;
}
return super.visitMethod(node, visitorState);
}
@Override
public Void visitMethod(MethodTree node, VisitorState visitorState) {
if (methodReturns(Suppliers.typeFromString(typeString)).matches(node, visitorState)) {
matched = true;
}
return super.visitMethod(node, visitorState);
}

@Override
void assertDone() {
assertEquals(matched, shouldMatch);
}
};
@Override
void assertDone() {
assertEquals(matched, shouldMatch);
}
};
tests.add(test);
return test;
}
Expand Down

0 comments on commit 749671e

Please sign in to comment.