Remove references to -XepOpt:Android:Java8Libs

PiperOrigin-RevId: 603821511
PicnicSupermarket · Feb 3, 2024 · 20562d0 · 20562d0
1 parent e5a6d0d
commit 20562d0
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 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
@@ -22,36 +22,31 @@
 
 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;
 import com.google.protobuf.ExtensionRegistry;
 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
  * devices. As of Android N, that includes all of JDK8 (which is only supported on Nougat) except
  * 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<ExpressionTree> 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
@@ -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 <long, TimeUnit> 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
@@ -16,23 +16,17 @@
 
 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;
 import org.junit.runners.JUnit4;
 
 /** {@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,37 +173,55 @@ 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);",
             "  }",
             "}")
         .doTest();
   }
 
   @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();
   }