Skip to content

Commit

Permalink
Add the constaint that Matchers can only operate on Trees.
Browse files Browse the repository at this point in the history
Allowing things like Matcher<List<Statement>> made it impossible to provide the
matcher with a VisitorState with the correct path. This also de-duplicates the
MultiMatcher implementations and ensures that they use correct TreePaths.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=80664674
  • Loading branch information
cushon committed Jan 9, 2015
1 parent a45bc6a commit c299f92
Show file tree
Hide file tree
Showing 23 changed files with 293 additions and 261 deletions.
Expand Up @@ -23,7 +23,6 @@
import static com.google.errorprone.fixes.SuggestedFix.replace; import static com.google.errorprone.fixes.SuggestedFix.replace;
import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.matchers.Matchers.hasIdentifier; import static com.google.errorprone.matchers.Matchers.hasIdentifier;
import static com.google.errorprone.matchers.MultiMatcher.MatchType.ANY;
import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.getType; import static com.google.errorprone.util.ASTHelpers.getType;
import static com.sun.source.tree.Tree.Kind.IDENTIFIER; import static com.sun.source.tree.Tree.Kind.IDENTIFIER;
Expand Down Expand Up @@ -212,7 +211,7 @@ public boolean matches(IdentifierTree tree, VisitorState state) {
return isIdentifierWithName(tree, name); return isIdentifierWithName(tree, name);
} }
}; };
return hasIdentifier(ANY, identifierMatcher).matches(tree, state); return hasIdentifier(identifierMatcher).matches(tree, state);
} }


private static boolean isIdentifierWithName(ExpressionTree tree, String name) { private static boolean isIdentifierWithName(ExpressionTree tree, String name) {
Expand Down
Expand Up @@ -26,7 +26,6 @@
import static com.google.errorprone.matchers.Matchers.nestingKind; import static com.google.errorprone.matchers.Matchers.nestingKind;
import static com.google.errorprone.matchers.Matchers.not; import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.parentNode; import static com.google.errorprone.matchers.Matchers.parentNode;
import static com.google.errorprone.matchers.MultiMatcher.MatchType.ANY;


import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
Expand Down Expand Up @@ -80,7 +79,7 @@ public boolean matches(ClassTree classTree, VisitorState state) {
anyOf( anyOf(
parentNode(nestingKind(NestingKind.TOP_LEVEL)), parentNode(nestingKind(NestingKind.TOP_LEVEL)),
parentNode(Matchers.<ClassTree>hasModifier(Modifier.STATIC))), parentNode(Matchers.<ClassTree>hasModifier(Modifier.STATIC))),
not(hasIdentifier(ANY, referenceEnclosing(classTree, state.getTypes()))) not(hasIdentifier(referenceEnclosing(classTree, state.getTypes())))
).matches(classTree, state); ).matches(classTree, state);
} }
}; };
Expand Down
Expand Up @@ -25,7 +25,6 @@
import static com.google.errorprone.matchers.Matchers.enclosingMethod; import static com.google.errorprone.matchers.Matchers.enclosingMethod;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf; import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.kindIs; import static com.google.errorprone.matchers.Matchers.kindIs;
import static com.google.errorprone.matchers.Matchers.lastStatement;
import static com.google.errorprone.matchers.Matchers.not; import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.parentNode; import static com.google.errorprone.matchers.Matchers.parentNode;
import static com.google.errorprone.suppliers.Suppliers.EXCEPTION_TYPE; import static com.google.errorprone.suppliers.Suppliers.EXCEPTION_TYPE;
Expand All @@ -38,8 +37,9 @@
import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.ChildMultiMatcher;
import com.google.errorprone.matchers.ChildMultiMatcher.MatchType;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Enclosing;
import com.google.errorprone.matchers.JUnitMatchers; import com.google.errorprone.matchers.JUnitMatchers;
import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers; import com.google.errorprone.matchers.Matchers;
Expand All @@ -50,8 +50,6 @@
import com.sun.source.tree.StatementTree; import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree; import com.sun.source.tree.Tree;


import java.util.List;

/** /**
* @author alexeagle@google.com (Alex Eagle) * @author alexeagle@google.com (Alex Eagle)
*/ */
Expand All @@ -78,12 +76,9 @@ public Description matchNewClass(NewClassTree newClassTree, VisitorState state)


StatementTree parent = (StatementTree) state.getPath().getParentPath().getLeaf(); StatementTree parent = (StatementTree) state.getPath().getParentPath().getLeaf();


Matcher<List<StatementTree>> isLastStatementOfList =
lastStatement(Matchers.<StatementTree>isSame(parent));
boolean isLastStatement = anyOf( boolean isLastStatement = anyOf(
new Enclosing.BlockOrCase<>( new ChildOfBlockOrCase<>(ChildMultiMatcher.MatchType.LAST,
inBlockWhereStatement(isLastStatementOfList), Matchers.<StatementTree>isSame(parent)),
inCaseWhereStatement(isLastStatementOfList)),
// it could also be a bare if statement with no braces // it could also be a bare if statement with no braces
parentNode(parentNode(kindIs(IF)))) parentNode(parentNode(kindIs(IF))))
.matches(newClassTree, state); .matches(newClassTree, state);
Expand All @@ -97,37 +92,27 @@ public Description matchNewClass(NewClassTree newClassTree, VisitorState state)


return describeMatch(newClassTree, fix); return describeMatch(newClassTree, fix);
} }

private static class ChildOfBlockOrCase<T extends Tree>
extends ChildMultiMatcher<T, StatementTree> {
public ChildOfBlockOrCase(MatchType matchType, Matcher<StatementTree> nodeMatcher) {
super(matchType, nodeMatcher);
}


/* @Override
* state.getTreePath() is set incorrectly by the following classes in their call to protected Iterable<? extends StatementTree> getChildNodes(T tree, VisitorState state) {
* statementsMatcher. The problem is that it's weird to have to choose a proper path for the Tree enclosing = state.findEnclosing(CaseTree.class, BlockTree.class);
* multiple statements that statementsMatcher expects. The best approach might be to pick the path if (enclosing == null) {
* of an arbitrary statement, but of course that has its own problems. Plus, the block might be return ImmutableList.of();
* empty. Fortunately, in this case, we use only lastStatement, which doesn't use the TreePath.
* Thus, it doesn't matter if the TreePath is wrong. But this problem casts suspicion on the use
* of any Matcher<NotATree>, since without a unique tree, we can't have a unique TreePath in the
* VisitorState passed to its match() method.
*/

private static Matcher<BlockTree> inBlockWhereStatement(
final Matcher<List<StatementTree>> statementsMatcher) {
return new Matcher<BlockTree>() {
@Override
public boolean matches(BlockTree t, VisitorState state) {
// state.getTreePath() will be wrong for statementsMatcher. See doc above.
return statementsMatcher.matches(ImmutableList.copyOf(t.getStatements()), state);
} }
}; if (enclosing instanceof BlockTree) {
} return ((BlockTree) enclosing).getStatements();

} else if (enclosing instanceof CaseTree) {
private static Matcher<CaseTree> inCaseWhereStatement( return ((CaseTree) enclosing).getStatements();
final Matcher<List<StatementTree>> statementsMatcher) { } else {
return new Matcher<CaseTree>() { // findEnclosing given two types must return something of one of those types
@Override throw new IllegalStateException("enclosing tree not a BlockTree or CaseTree");
public boolean matches(CaseTree t, VisitorState state) {
// state.getTreePath() will be wrong for statementsMatcher. See doc above.
return statementsMatcher.matches(ImmutableList.copyOf(t.getStatements()), state);
} }
}; }
} }
} }
Expand Up @@ -19,12 +19,12 @@
import static com.google.errorprone.BugPattern.Category.GUICE; import static com.google.errorprone.BugPattern.Category.GUICE;
import static com.google.errorprone.BugPattern.MaturityLevel.MATURE; import static com.google.errorprone.BugPattern.MaturityLevel.MATURE;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.ANY;
import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.annotations; import static com.google.errorprone.matchers.Matchers.annotations;
import static com.google.errorprone.matchers.Matchers.constructor; import static com.google.errorprone.matchers.Matchers.constructor;
import static com.google.errorprone.matchers.Matchers.hasAnnotation; import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.Matchers.methodHasParameters; import static com.google.errorprone.matchers.Matchers.methodHasParameters;
import static com.google.errorprone.matchers.MultiMatcher.MatchType.ANY;


import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
Expand Down
Expand Up @@ -26,10 +26,10 @@
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.ChildMultiMatcher.MatchType;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.MultiMatcher; import com.google.errorprone.matchers.MultiMatcher;
import com.google.errorprone.matchers.MultiMatcher.MatchType;
import com.google.errorprone.util.ASTHelpers; import com.google.errorprone.util.ASTHelpers;


import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AnnotationTree;
Expand Down
Expand Up @@ -19,13 +19,13 @@
import static com.google.errorprone.BugPattern.Category.INJECT; import static com.google.errorprone.BugPattern.Category.INJECT;
import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL; import static com.google.errorprone.BugPattern.MaturityLevel.EXPERIMENTAL;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher; import static com.google.errorprone.matchers.ChildMultiMatcher.MatchType.ANY;
import static com.google.errorprone.matchers.Matchers.constructor; import static com.google.errorprone.matchers.Matchers.constructor;
import static com.google.errorprone.matchers.Matchers.hasAnnotation; import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.MultiMatcher.MatchType.ANY;


import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher; import com.google.errorprone.matchers.Matcher;
Expand Down
34 changes: 8 additions & 26 deletions core/src/main/java/com/google/errorprone/matchers/Annotation.java
Expand Up @@ -16,9 +16,6 @@


package com.google.errorprone.matchers; package com.google.errorprone.matchers;


import static com.google.errorprone.matchers.MultiMatcher.MatchType.ALL;
import static com.google.errorprone.matchers.MultiMatcher.MatchType.ANY;

import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;


import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AnnotationTree;
Expand All @@ -28,46 +25,31 @@
import com.sun.source.tree.Tree; import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree; import com.sun.source.tree.VariableTree;


import java.util.List;

/** /**
* Matches if the given annotation matcher matches all of or any of the annotations on the tree * Matches if the given annotation matcher matches all of or any of the annotations on the tree
* node. * node.
* *
* @author eaftan@google.com (Eddie Aftandilian) * @author eaftan@google.com (Eddie Aftandilian)
*/ */
public class Annotation<T extends Tree> extends MultiMatcher<T, AnnotationTree> { public class Annotation<T extends Tree> extends ChildMultiMatcher<T, AnnotationTree> {


public Annotation(MatchType matchType, Matcher<AnnotationTree> nodeMatcher) { public Annotation(MatchType matchType, Matcher<AnnotationTree> nodeMatcher) {
super(matchType, nodeMatcher); super(matchType, nodeMatcher);
} }


@Override @Override
public boolean matches(T tree, VisitorState state) { protected Iterable<? extends AnnotationTree> getChildNodes(T tree, VisitorState state) {
List<? extends AnnotationTree> annotations;
if (tree instanceof ClassTree) { if (tree instanceof ClassTree) {
annotations = ((ClassTree) tree).getModifiers().getAnnotations(); return ((ClassTree) tree).getModifiers().getAnnotations();
} else if (tree instanceof VariableTree) { } else if (tree instanceof VariableTree) {
annotations = ((VariableTree) tree).getModifiers().getAnnotations(); return ((VariableTree) tree).getModifiers().getAnnotations();
} else if (tree instanceof MethodTree) { } else if (tree instanceof MethodTree) {
annotations = ((MethodTree) tree).getModifiers().getAnnotations(); return ((MethodTree) tree).getModifiers().getAnnotations();
} else if (tree instanceof CompilationUnitTree) { } else if (tree instanceof CompilationUnitTree) {
annotations = ((CompilationUnitTree) tree).getPackageAnnotations(); return ((CompilationUnitTree) tree).getPackageAnnotations();
} else { } else {
throw new IllegalArgumentException("Cannot access annotations from tree of type " throw new IllegalArgumentException(
+ tree.getClass()); "Cannot access annotations from tree of type " + tree.getClass());
}

for (AnnotationTree annotation : annotations) {
boolean matches = nodeMatcher.matches(annotation, state);
if (matchType == ANY && matches) {
matchingNode = annotation;
return true;
}
if (matchType == ALL && !matches) {
return false;
}
} }
return matchType == ALL && annotations.size() >= 1;
} }
} }

0 comments on commit c299f92

Please sign in to comment.