Set VisitorState.getTreePath() correctly for Matchers that look at en…

…closing elements:

The existing code left the path to the original element in place. The new code updates the path to point to the enclosing element being matched against.
This brings the enclosing-element Matchers in line with the parentNode Matcher, which I believe is the desirable behavior.

There is one subtlety to this around Enclosing.BlockOrCase. See the comments I've inserted into DeadException.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=80496579

core/src/main/java/com/google/errorprone/VisitorState.java

@@ -197,34 +197,35 @@ public Type getType(Type baseType, boolean isArray, java.util.List<Type> typePar
   }
 
   /**
-   * Find the first enclosing tree node of one of the given types.
-   * @param classes
-   * @param <T>
-   * @return the node, or null if there is no enclosing tree node of this type
+   * Returns the {@link TreePath} to the nearest tree node of one of the given types. To instead
+   * retrieve the element directly, use {@link #findEnclosing(Class...)}.
+   *
+   * @return the path, or {@code null} if there is no match
    */
   @SafeVarargs
-  public final <T extends Tree> T findEnclosing(java.lang.Class<? extends T>... classes) {
+  public final TreePath findPathToEnclosing(Class<? extends Tree>... classes) {
     TreePath enclosingPath = getPath();
     while (enclosingPath != null) {
-      for (java.lang.Class<? extends T> aClass : classes) {
-        if (aClass.isInstance(enclosingPath.getLeaf())) {
-          return aClass.cast(enclosingPath.getLeaf());
+      for (Class<? extends Tree> clazz : classes) {
+        if (clazz.isInstance(enclosingPath.getLeaf())) {
+          return enclosingPath;
         }
       }
       enclosingPath = enclosingPath.getParentPath();
     }
     return null;
   }
-
-  public <T extends Tree> T findEnclosing(java.lang.Class<T> aClass) {
-    TreePath enclosingPath = getPath();
-    while (enclosingPath != null) {
-      if (aClass.isInstance(enclosingPath.getLeaf())) {
-        return aClass.cast(enclosingPath.getLeaf());
-      }
-      enclosingPath = enclosingPath.getParentPath();
-    }
-    return null;
+
+  /**
+   * Find the first enclosing tree node of one of the given types.
+   *
+   * @return the node, or {@code null} if there is no match
+   */
+  @SuppressWarnings("unchecked") // findPathToEnclosing guarantees that the type is from |classes|
+  @SafeVarargs
+  public final <T extends Tree> T findEnclosing(Class<? extends T>... classes) {
+    TreePath pathToEnclosing = findPathToEnclosing(classes);
+    return (pathToEnclosing == null) ? null : (T) pathToEnclosing.getLeaf();
   }
 
   /**

core/src/main/java/com/google/errorprone/bugpatterns/DeadException.java

@@ -32,6 +32,7 @@
 import static com.sun.source.tree.Tree.Kind.EXPRESSION_STATEMENT;
 import static com.sun.source.tree.Tree.Kind.IF;
 
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
@@ -43,10 +44,14 @@
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.matchers.Matchers;
 
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.CaseTree;
 import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.Tree;
 
+import java.util.List;
+
 /**
  * @author alexeagle@google.com (Alex Eagle)
  */
@@ -73,8 +78,12 @@ public Description matchNewClass(NewClassTree newClassTree, VisitorState state)
 
     StatementTree parent = (StatementTree) state.getPath().getParentPath().getLeaf();
 
+    Matcher<List<StatementTree>> isLastStatementOfList =
+        lastStatement(Matchers.<StatementTree>isSame(parent));
     boolean isLastStatement = anyOf(
-        new Enclosing.BlockOrCase<>(lastStatement(Matchers.<StatementTree>isSame(parent))),
+        new Enclosing.BlockOrCase<>(
+            inBlockWhereStatement(isLastStatementOfList),
+            inCaseWhereStatement(isLastStatementOfList)),
         // it could also be a bare if statement with no braces
         parentNode(parentNode(kindIs(IF))))
         .matches(newClassTree, state);
@@ -88,4 +97,37 @@ public Description matchNewClass(NewClassTree newClassTree, VisitorState state)
 
     return describeMatch(newClassTree, fix);
   }
+
+  /*
+   * state.getTreePath() is set incorrectly by the following classes in their call to
+   * statementsMatcher. The problem is that it's weird to have to choose a proper path for the
+   * multiple statements that statementsMatcher expects. The best approach might be to pick the path
+   * of an arbitrary statement, but of course that has its own problems. Plus, the block might be
+   * 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);
+      }
+    };
+  }
+
+  private static Matcher<CaseTree> inCaseWhereStatement(
+      final Matcher<List<StatementTree>> statementsMatcher) {
+    return new Matcher<CaseTree>() {
+      @Override
+      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);
+      }
+    };
+  }
 }

core/src/main/java/com/google/errorprone/matchers/Enclosing.java

@@ -16,17 +16,14 @@
 
 package com.google.errorprone.matchers;
 
-import com.google.common.collect.ImmutableList;
 import com.google.errorprone.VisitorState;
 
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.CaseTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.MethodTree;
-import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.Tree;
-
-import java.util.List;
+import com.sun.source.util.TreePath;
 
 /**
  * Adapt matchers to match against a parent node of a given type. For example, match a node if the
@@ -47,12 +44,13 @@ protected EnclosingMatcher(Matcher<T> matcher, java.lang.Class<T> clazz) {
 
     @Override
     public boolean matches(U unused, VisitorState state) {
-      T enclosing = state.findEnclosing(clazz);
+      TreePath pathToEnclosing = state.findPathToEnclosing(clazz);
       // No match if there is no enclosing element to match against
-      if (enclosing == null) {
+      if (pathToEnclosing == null) {
         return false;
       }
-      return matcher.matches(enclosing, state);
+      T enclosing = clazz.cast(pathToEnclosing.getLeaf());
+      return matcher.matches(enclosing, state.withPath(pathToEnclosing));
     }
   }
 
@@ -75,28 +73,30 @@ public Method(Matcher<MethodTree> matcher) {
   }
 
   public static class BlockOrCase<T extends Tree> implements Matcher<T> {
-    private final Matcher<List<StatementTree>> matcher;
+    private final Matcher<BlockTree> blockTreeMatcher;
+    private final Matcher<CaseTree> caseTreeMatcher;
 
-    public BlockOrCase(Matcher<List<StatementTree>> matcher) {
-      this.matcher = matcher;
+    public BlockOrCase(Matcher<BlockTree> blockTreeMatcher, Matcher<CaseTree> caseTreeMatcher) {
+      this.blockTreeMatcher = blockTreeMatcher;
+      this.caseTreeMatcher = caseTreeMatcher;
     }
 
     @Override
     public boolean matches(T unused, VisitorState state) {
-      Tree enclosing = state.findEnclosing(CaseTree.class, BlockTree.class);
-      if (enclosing == null) {
+      TreePath pathToEnclosing = state.findPathToEnclosing(CaseTree.class, BlockTree.class);
+      if (pathToEnclosing == null) {
         return false;
       }
-      final List<? extends StatementTree> statements;
+      Tree enclosing = pathToEnclosing.getLeaf();
+      state = state.withPath(pathToEnclosing);
       if (enclosing instanceof BlockTree) {
-        statements = ((BlockTree)enclosing).getStatements();
+        return blockTreeMatcher.matches((BlockTree) enclosing, state);
       } else if (enclosing instanceof CaseTree) {
-        statements = ((CaseTree)enclosing).getStatements();
+        return caseTreeMatcher.matches((CaseTree) enclosing, state);
       } else {
         // findEnclosing given two types must return something of one of those types
         throw new IllegalStateException("enclosing tree not a BlockTree or CaseTree");
       }
-      return matcher.matches(ImmutableList.copyOf(statements), state);
     }
   }
 }

core/src/main/java/com/google/errorprone/matchers/Matchers.java

@@ -472,6 +472,7 @@ public boolean matches(Tree t, VisitorState state) {
         TreePath path = state.getPath().getParentPath();
         while (path != null) {
           Tree node = path.getLeaf();
+          state = state.withPath(path);
           if (matcher.matches((T) node, state)) {
             return true;
           }