All forEach on subtypes of Collection

`Collection` doesn't override the method from `Iterable`, but desugar special-cases it to allow `forEach` to be called on collection subtypes. PiperOrigin-RevId: 387858050
google · Jul 30, 2021 · 4c57125 · 4c57125
1 parent 6a63b57
commit 4c57125
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 3 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsChecker.java
@@ -17,13 +17,19 @@
 package com.google.errorprone.bugpatterns.apidiff;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
 
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.ErrorProneFlags;
+import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.apidiff.ApiDiff.ClassMemberKey;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ExpressionTree;
 import java.util.Map;
 import java.util.stream.Collectors;
 
@@ -40,6 +46,8 @@
 // TODO(b/32513850): Allow Android N+ APIs, e.g., by computing API diff using android.jar
 public class AndroidJdkLibsChecker extends ApiDiffChecker {
 
+  private final boolean allowJava8;
+
   public AndroidJdkLibsChecker(ErrorProneFlags flags) {
     this(flags.getBoolean("Android:Java8Libs").orElse(false));
   }
@@ -50,6 +58,25 @@ public AndroidJdkLibsChecker() {
 
   private AndroidJdkLibsChecker(boolean allowJava8) {
     super(deriveApiDiff(allowJava8));
+    this.allowJava8 = allowJava8;
+  }
+
+  private static final Matcher<ExpressionTree> FOREACH_ON_COLLECTION =
+      instanceMethod()
+          .onDescendantOf("java.util.Collection")
+          .named("forEach")
+          .withParameters("java.util.function.Consumer");
+
+  @Override
+  protected Description check(ExpressionTree tree, VisitorState state) {
+    Description description = super.check(tree, state);
+    if (description.equals(NO_MATCH)) {
+      return NO_MATCH;
+    }
+    if (allowJava8 && FOREACH_ON_COLLECTION.matches(tree, state)) {
+      return NO_MATCH;
+    }
+    return description;
   }
 
   private static ApiDiff deriveApiDiff(boolean allowJava8) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/ApiDiffChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/ApiDiffChecker.java
@@ -66,7 +66,7 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state)
     return check(tree, state);
   }
 
-  private Description check(ExpressionTree tree, VisitorState state) {
+  protected Description check(ExpressionTree tree, VisitorState state) {
     if (state.findEnclosing(ImportTree.class) != null) {
       return Description.NO_MATCH;
     }

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsCheckerTest.java
@@ -17,8 +17,8 @@
 package com.google.errorprone.bugpatterns.apidiff;
 
 
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.CompilationTestHelper;
-import java.util.Collections;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -30,7 +30,7 @@ public class AndroidJdkLibsCheckerTest extends Java7ApiCheckerTest {
 
   private final CompilationTestHelper allowJava8Helper =
       CompilationTestHelper.newInstance(AndroidJdkLibsChecker.class, getClass())
-          .setArgs(Collections.singletonList("-XepOpt:Android:Java8Libs"));
+          .setArgs(ImmutableList.of("-XepOpt:Android:Java8Libs"));
 
   public AndroidJdkLibsCheckerTest() {
     super(AndroidJdkLibsChecker.class);
@@ -190,4 +190,37 @@ public void allowJava8Flag_explicitNestedClass() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void forEach() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Collection;",
+            "class T {",
+            "  void f(Iterable<?> i, Collection<?> c) {",
+            "    // BUG: Diagnostic contains: java.lang.Iterable#forEach",
+            "    i.forEach(System.err::println);",
+            "    // BUG: Diagnostic contains: java.lang.Iterable#forEach",
+            "    c.forEach(System.err::println);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void allowJava8Flag_forEach() {
+    allowJava8Helper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Collection;",
+            "class T {",
+            "  void f(Iterable<?> i, Collection<?> c) {",
+            "    // BUG: Diagnostic contains: java.lang.Iterable#forEach",
+            "    i.forEach(System.err::println);",
+            "    c.forEach(System.err::println);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }