From 20562d09a69dffe6537f9d33e844ba59e3a66451 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Fri, 2 Feb 2024 16:50:04 -0800 Subject: [PATCH] Remove references to `-XepOpt:Android:Java8Libs` PiperOrigin-RevId: 603821511 --- .../apidiff/AndroidJdkLibsChecker.java | 27 ++---- .../time/PreferJavaTimeOverload.java | 13 --- .../apidiff/AndroidJdkLibsCheckerTest.java | 84 +++++++++---------- 3 files changed, 44 insertions(+), 80 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 index 5124e5996ab..ea8096e745e 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsChecker.java @@ -22,7 +22,6 @@ import com.google.common.io.Resources; import com.google.errorprone.BugPattern; -import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Matcher; @@ -30,7 +29,6 @@ import com.sun.source.tree.ExpressionTree; import java.io.IOException; import java.io.UncheckedIOException; -import javax.inject.Inject; /** * Checks for uses of classes, fields, or methods that are not compatible with legacy Android @@ -38,20 +36,17 @@ * type and repeated annotations, which are compiled in a backwards compatible way. */ @BugPattern( - name = "AndroidJdkLibsChecker", altNames = "AndroidApiChecker", summary = "Use of class, field, or method that is not compatible with legacy Android devices", severity = ERROR) // TODO(b/32513850): Allow Android N+ APIs, e.g., by computing API diff using android.jar public class AndroidJdkLibsChecker extends ApiDiffChecker { - private static ApiDiff loadApiDiff(boolean allowJava8) { + private static ApiDiff loadApiDiff() { try { byte[] diffData = Resources.toByteArray( - Resources.getResource( - AndroidJdkLibsChecker.class, - allowJava8 ? "android_java8.binarypb" : "android.binarypb")); + Resources.getResource(AndroidJdkLibsChecker.class, "android_java8.binarypb")); ApiDiffProto.Diff diff = ApiDiffProto.Diff.newBuilder() .mergeFrom(diffData, ExtensionRegistry.getEmptyRegistry()) @@ -62,20 +57,8 @@ private static ApiDiff loadApiDiff(boolean allowJava8) { } } - private final boolean allowJava8; - - @Inject - AndroidJdkLibsChecker(ErrorProneFlags flags) { - this(flags.getBoolean("Android:Java8Libs").orElse(false)); - } - - public AndroidJdkLibsChecker() { - this(false); - } - - private AndroidJdkLibsChecker(boolean allowJava8) { - super(loadApiDiff(allowJava8)); - this.allowJava8 = allowJava8; + private AndroidJdkLibsChecker() { + super(loadApiDiff()); } private static final Matcher FOREACH_ON_COLLECTION = @@ -90,7 +73,7 @@ protected Description check(ExpressionTree tree, VisitorState state) { if (description.equals(NO_MATCH)) { return NO_MATCH; } - if (allowJava8 && FOREACH_ON_COLLECTION.matches(tree, state)) { + if (FOREACH_ON_COLLECTION.matches(tree, state)) { return NO_MATCH; } return description; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/PreferJavaTimeOverload.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/PreferJavaTimeOverload.java index 8b7d1b00714..a02e6030b66 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/time/PreferJavaTimeOverload.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/PreferJavaTimeOverload.java @@ -35,7 +35,6 @@ import com.google.common.collect.ImmutableMap; import com.google.errorprone.BugPattern; -import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; @@ -55,7 +54,6 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; -import javax.inject.Inject; /** This check suggests the use of {@code java.time}-based APIs, when available. */ @BugPattern( @@ -125,21 +123,10 @@ public final class PreferJavaTimeOverload extends BugChecker .onExactClass(JAVA_DURATION) .namedAnyOf("toNanos", "toMillis", "getSeconds", "toMinutes", "toHours", "toDays"); - private final boolean hasJava8LibSupport; - - @Inject - PreferJavaTimeOverload(ErrorProneFlags flags) { - this.hasJava8LibSupport = flags.getBoolean("Android:Java8Libs").orElse(false); - } - // TODO(kak): Add support for constructors that accept a or JodaTime Duration @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - // don't fire for Android code that doesn't have Java8 library support (b/138965731) - if (state.isAndroidCompatible() && !hasJava8LibSupport) { - return Description.NO_MATCH; - } // we return no match for a set of explicitly ignored APIs if (IGNORED_APIS.matches(tree, state)) { 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 index 3e79fe4ce23..a473f1eb9f5 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/apidiff/AndroidJdkLibsCheckerTest.java @@ -16,7 +16,6 @@ package com.google.errorprone.bugpatterns.apidiff; -import com.google.common.collect.ImmutableList; import com.google.errorprone.CompilationTestHelper; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,15 +23,10 @@ /** {@link AndroidJdkLibsChecker}Test. */ @RunWith(JUnit4.class) -public class AndroidJdkLibsCheckerTest extends Java7ApiCheckerTest { +public class AndroidJdkLibsCheckerTest { - private final CompilationTestHelper allowJava8Helper = - CompilationTestHelper.newInstance(AndroidJdkLibsChecker.class, getClass()) - .setArgs(ImmutableList.of("-XepOpt:Android:Java8Libs")); - - public AndroidJdkLibsCheckerTest() { - super(AndroidJdkLibsChecker.class); - } + protected final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(AndroidJdkLibsChecker.class, getClass()); @Test public void repeatedAnnotationAllowed() { @@ -77,8 +71,6 @@ public void defaultMethod() { " }", " }", " void f(A a, B b) {", - " // BUG: Diagnostic contains: java.util.Map#getOrDefault(java.lang.Object,V)" - + " is not available in Test.A", " a.getOrDefault(null, null);", " b.getOrDefault(null, null); // OK: overrides getOrDefault", " }", @@ -107,7 +99,6 @@ public void stopwatchElapsed() { "import java.util.concurrent.TimeUnit;", "public class Test {", " void o() {", - " // BUG: Diagnostic contains:", " Stopwatch.createStarted().elapsed();", " Stopwatch.createStarted().elapsed(TimeUnit.MILLISECONDS);", " }", @@ -117,7 +108,7 @@ public void stopwatchElapsed() { @Test public void allowJava8Flag_packageAllowed() { - allowJava8Helper + compilationHelper .addSourceLines( "Test.java", "import java.time.Duration;", @@ -135,7 +126,7 @@ public void allowJava8Flag_packageAllowed() { @Test public void allowJava8Flag_memberAllowed() { - allowJava8Helper + compilationHelper .addSourceLines( "Test.java", "import java.util.Arrays;", @@ -147,24 +138,9 @@ public void allowJava8Flag_memberAllowed() { .doTest(); } - @Test - public void allowJava8Flag_memberBanned() { - allowJava8Helper - .addSourceLines( - "Test.java", - "import java.util.stream.Stream;", - "public class Test {", - " public static void test(Stream s) {", - " // BUG: Diagnostic contains: parallel", - " s.parallel();", - " }", - "}") - .doTest(); - } - @Test public void allowJava8Flag_getTimeZone() { - allowJava8Helper + compilationHelper .addSourceLines( "Test.java", "import java.time.ZoneId;", @@ -180,7 +156,7 @@ public void allowJava8Flag_getTimeZone() { @Test public void allowJava8Flag_explicitNestedClass() { - allowJava8Helper + compilationHelper .addSourceLines( "Test.java", "import java.util.Spliterator;", @@ -197,9 +173,7 @@ public void forEach() { "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);", " }", "}") @@ -207,27 +181,47 @@ public void forEach() { } @Test - public void allowJava8Flag_forEach() { - allowJava8Helper + public void moduleInfo() { + compilationHelper .addSourceLines( - "Test.java", - "import java.util.Collection;", - "class T {", - " void f(Iterable i, Collection c) {", - " i.forEach(System.err::println);", - " c.forEach(System.err::println);", + "module-info.java", // + "module testmodule {", + " requires java.base;", + "}") + .doTest(); + } + + // A JDK API that is not available on Android < 26 + @Test + public void methodHandle() { + compilationHelper + .addSourceLines( + "Test.java", // + "import java.lang.invoke.MethodHandles;", + "public class Test {", + " void f() {", + " Object o = MethodHandles.lookup();", " }", "}") .doTest(); } + // an Android API that was added in 24 @Test - public void moduleInfo() { + public void newAndroidApi() { compilationHelper .addSourceLines( - "module-info.java", // - "module testmodule {", - " requires java.base;", + "Tile.java", // + "package android.service.quicksettings;", + "public class Tile {", + "}") + .addSourceLines( + "Test.java", // + "import static java.util.Objects.requireNonNull;", + "public class Test {", + " void f() {", + " requireNonNull(android.service.quicksettings.Tile.class);", + " }", "}") .doTest(); }