Update TypeParameterShadowing replacement-finding logic to deal with

desugared lambda parameters MOE_MIGRATED_REVID=148801831
google · Feb 28, 2017 · 1f6cc84 · 1f6cc84
1 parent 2b5239b
commit 1f6cc84
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 12 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterShadowing.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterShadowing.java
@@ -60,19 +60,19 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
     if (tree.getTypeParameters().isEmpty()) {
       return Description.NO_MATCH;
     }
-    return findDuplicatesOf(tree, tree.getTypeParameters());
+    return findDuplicatesOf(tree, tree.getTypeParameters(), state);
   }
 
   @Override
   public Description matchClass(ClassTree tree, VisitorState state) {
     if (tree.getTypeParameters().isEmpty()) {
       return Description.NO_MATCH;
     }
-    return findDuplicatesOf(tree, tree.getTypeParameters());
+    return findDuplicatesOf(tree, tree.getTypeParameters(), state);
   }
 
   private Description findDuplicatesOf(
-      Tree tree, List<? extends TypeParameterTree> typeParameters) {
+      Tree tree, List<? extends TypeParameterTree> typeParameters, VisitorState state) {
 
     Symbol symbol = ASTHelpers.getSymbol(tree);
     if (symbol == null) {
@@ -118,7 +118,8 @@ private Description findDuplicatesOf(
                     tree,
                     typeParameters,
                     v.name,
-                    replacementTypeVarName(v.name, typeVarsInScope)))
+                    replacementTypeVarName(v.name, typeVarsInScope),
+                    state))
         .forEach(fixBuilder::merge);
 
     descriptionBuilder.addFix(fixBuilder.build());
@@ -151,7 +152,8 @@ private static SuggestedFix renameTypeVariable(
       Tree sourceTree,
       List<? extends TypeParameterTree> typeParameters,
       Name typeVariable,
-      String typeVarReplacement) {
+      String typeVarReplacement,
+      VisitorState state) {
 
     TypeParameterTree matchingTypeParam =
         typeParameters.stream().filter(t -> t.getName().contentEquals(typeVariable)).collect(
@@ -169,8 +171,22 @@ private static SuggestedFix renameTypeVariable(
             new TreeScanner() {
               @Override
               public void visitIdent(JCTree.JCIdent tree) {
-                if (Objects.equal(ASTHelpers.getSymbol(tree), typeVariableSymbol)) {
-                  fixBuilder.replace(tree, typeVarReplacement);
+                Symbol identSym = ASTHelpers.getSymbol(tree);
+                if (Objects.equal(identSym, typeVariableSymbol)) {
+                  // Lambda parameters can be desugared early, so we need to make sure the source
+                  // is there. In the example below, we would try to suggest replacing the node 't'
+                  // with T2, since the compiler desugars to g((T t) -> false). The extra condition
+                  // prevents us from doing that.
+
+                  // Foo<T> {
+                  //   <G> void g(Predicate<G> p) {},
+                  //   <T> void blah() {
+                  //     g(t -> false);
+                  //   }
+                  // }
+                  if (Objects.equal(state.getSourceForNode(tree), name)) {
+                    fixBuilder.replace(tree, typeVarReplacement);
+                  }
                 }
               }
             });

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterShadowingTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterShadowingTest.java
@@ -19,7 +19,6 @@
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import com.google.errorprone.CompilationTestHelper;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -314,11 +313,10 @@ public void symbolWithoutTypeParameters() {
         .doTest();
   }
 
-  @Ignore("b/35809355")
   @Test
-  public void b35809355() throws Exception {
-    compilationHelper
-        .addSourceLines(
+  public void lambdaParameterDesugaring() throws Exception {
+    refactoring
+        .addInputLines(
             "in/A.java",
             "import java.util.function.Consumer;",
             "class A<T> {",
@@ -329,6 +327,46 @@ public void b35809355() throws Exception {
             "    abstract void g(Consumer<T> c);",
             "  }",
             "}")
+        .addOutputLines(
+            "out/A.java",
+            "import java.util.function.Consumer;",
+            "class A<T> {",
+            "  abstract class B<T2> {",
+            "    void f() {",
+            "      g(t -> {});",
+            "    }",
+            "    abstract void g(Consumer<T2> c);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void typesWithBounds() throws Exception {
+    refactoring
+        .addInputLines(
+            "in/Test.java",
+            "import java.util.function.Predicate;",
+            "class Test<T> {",
+            "  <B extends Object & Comparable> void something(B b) {",
+            "    class Foo<B extends Object & Comparable> implements Predicate<B> {",
+            "        public boolean test(B b) { return false; }",
+            "    }",
+            "    new Foo<>();",
+            "  }",
+            "}")
+        .addOutputLines(
+            "out/Test.java",
+            "import java.util.function.Predicate;",
+            "class Test<T> {",
+            "  <B extends Object & Comparable> void something(B b) {",
+            "    class Foo<B2 extends Object & Comparable> implements Predicate<B2> {",
+            "      public boolean test(B2 b) { return false; }",
+            "    }",
+            "    new Foo<>();",
+            "  }",
+            "}")
         .doTest();
   }
+
 }