Skip to content

Commit 4c57125

Browse files
cushonError Prone Team
authored andcommitted
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
1 parent 6a63b57 commit 4c57125

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsChecker.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,19 @@
1717
package com.google.errorprone.bugpatterns.apidiff;
1818

1919
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
20+
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
2022

2123
import com.google.common.collect.ImmutableMultimap;
2224
import com.google.common.collect.ImmutableSet;
2325
import com.google.common.collect.ImmutableSetMultimap;
2426
import com.google.errorprone.BugPattern;
2527
import com.google.errorprone.ErrorProneFlags;
28+
import com.google.errorprone.VisitorState;
2629
import com.google.errorprone.bugpatterns.apidiff.ApiDiff.ClassMemberKey;
30+
import com.google.errorprone.matchers.Description;
31+
import com.google.errorprone.matchers.Matcher;
32+
import com.sun.source.tree.ExpressionTree;
2733
import java.util.Map;
2834
import java.util.stream.Collectors;
2935

@@ -40,6 +46,8 @@
4046
// TODO(b/32513850): Allow Android N+ APIs, e.g., by computing API diff using android.jar
4147
public class AndroidJdkLibsChecker extends ApiDiffChecker {
4248

49+
private final boolean allowJava8;
50+
4351
public AndroidJdkLibsChecker(ErrorProneFlags flags) {
4452
this(flags.getBoolean("Android:Java8Libs").orElse(false));
4553
}
@@ -50,6 +58,25 @@ public AndroidJdkLibsChecker() {
5058

5159
private AndroidJdkLibsChecker(boolean allowJava8) {
5260
super(deriveApiDiff(allowJava8));
61+
this.allowJava8 = allowJava8;
62+
}
63+
64+
private static final Matcher<ExpressionTree> FOREACH_ON_COLLECTION =
65+
instanceMethod()
66+
.onDescendantOf("java.util.Collection")
67+
.named("forEach")
68+
.withParameters("java.util.function.Consumer");
69+
70+
@Override
71+
protected Description check(ExpressionTree tree, VisitorState state) {
72+
Description description = super.check(tree, state);
73+
if (description.equals(NO_MATCH)) {
74+
return NO_MATCH;
75+
}
76+
if (allowJava8 && FOREACH_ON_COLLECTION.matches(tree, state)) {
77+
return NO_MATCH;
78+
}
79+
return description;
5380
}
5481

5582
private static ApiDiff deriveApiDiff(boolean allowJava8) {

core/src/main/java/com/google/errorprone/bugpatterns/apidiff/ApiDiffChecker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state)
6666
return check(tree, state);
6767
}
6868

69-
private Description check(ExpressionTree tree, VisitorState state) {
69+
protected Description check(ExpressionTree tree, VisitorState state) {
7070
if (state.findEnclosing(ImportTree.class) != null) {
7171
return Description.NO_MATCH;
7272
}

core/src/test/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsCheckerTest.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
package com.google.errorprone.bugpatterns.apidiff;
1818

1919

20+
import com.google.common.collect.ImmutableList;
2021
import com.google.errorprone.CompilationTestHelper;
21-
import java.util.Collections;
2222
import org.junit.Test;
2323
import org.junit.runner.RunWith;
2424
import org.junit.runners.JUnit4;
@@ -30,7 +30,7 @@ public class AndroidJdkLibsCheckerTest extends Java7ApiCheckerTest {
3030

3131
private final CompilationTestHelper allowJava8Helper =
3232
CompilationTestHelper.newInstance(AndroidJdkLibsChecker.class, getClass())
33-
.setArgs(Collections.singletonList("-XepOpt:Android:Java8Libs"));
33+
.setArgs(ImmutableList.of("-XepOpt:Android:Java8Libs"));
3434

3535
public AndroidJdkLibsCheckerTest() {
3636
super(AndroidJdkLibsChecker.class);
@@ -190,4 +190,37 @@ public void allowJava8Flag_explicitNestedClass() {
190190
"}")
191191
.doTest();
192192
}
193+
194+
@Test
195+
public void forEach() {
196+
compilationHelper
197+
.addSourceLines(
198+
"Test.java",
199+
"import java.util.Collection;",
200+
"class T {",
201+
" void f(Iterable<?> i, Collection<?> c) {",
202+
" // BUG: Diagnostic contains: java.lang.Iterable#forEach",
203+
" i.forEach(System.err::println);",
204+
" // BUG: Diagnostic contains: java.lang.Iterable#forEach",
205+
" c.forEach(System.err::println);",
206+
" }",
207+
"}")
208+
.doTest();
209+
}
210+
211+
@Test
212+
public void allowJava8Flag_forEach() {
213+
allowJava8Helper
214+
.addSourceLines(
215+
"Test.java",
216+
"import java.util.Collection;",
217+
"class T {",
218+
" void f(Iterable<?> i, Collection<?> c) {",
219+
" // BUG: Diagnostic contains: java.lang.Iterable#forEach",
220+
" i.forEach(System.err::println);",
221+
" c.forEach(System.err::println);",
222+
" }",
223+
"}")
224+
.doTest();
225+
}
193226
}

0 commit comments

Comments
 (0)