Fix compatibility issue when building on JDK 17 but running on JDK 8 (#…

…779) Error Prone ran into a similar issue in google/error-prone#3895. We re-use the same fix, and also add a CI job to test this case. We could avoid this problem by only building releases on JDK 11, but might as well fix to avoid that requirement.
uber · Jul 15, 2023 · 61abe56 · 61abe56
1 parent 31004d3
commit 61abe56
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 3 deletions.
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -16,6 +16,9 @@ jobs:
           - os: ubuntu-latest
             java: 11
             epVersion: 2.4.0
+          - os: ubuntu-latest
+            java: 17
+            epVersion: 2.4.0
           - os: macos-latest
             java: 11
             epVersion: 2.20.0

diff --git a/nullaway/build.gradle b/nullaway/build.gradle
@@ -92,6 +92,13 @@ test {
             // Accessed by Lombok tests
             "--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED",
     ]
+    if (deps.versions.errorProneApi == "2.4.0" && JavaVersion.current() >= JavaVersion.VERSION_17) {
+        // This test does not pass on JDK 17 with Error Prone 2.4.0 due to a Mockito incompatibility.  Skip it (the
+        // test passes with more recent Error Prone versions on JDK 17)
+        filter {
+            excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.suggestNullableArgumentOnBytecodeNoFileInfo"
+        }
+    }
 }
 
 apply plugin: 'com.vanniktech.maven.publish'

diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
@@ -176,6 +176,18 @@ public static TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
     return findEnclosingMethodOrLambdaOrInitializer(path, ImmutableSet.of());
   }
 
+  /**
+   * A wrapper for {@link Symbol#getEnclosedElements} to avoid binary compatibility issues for
+   * covariant overrides in subtypes of {@link Symbol}.
+   *
+   * <p>Same as this ASTHelpers method in Error Prone:
+   * https://github.com/google/error-prone/blame/a1318e4b0da4347dff7508108835d77c470a7198/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java#L1148
+   * TODO: delete this method and switch to ASTHelpers once we can require Error Prone 2.20.0
+   */
+  public static List<Symbol> getEnclosedElements(Symbol symbol) {
+    return symbol.getEnclosedElements();
+  }
+
   /**
    * NOTE: this method does not work for getting all annotations of parameters of methods from class
    * files. For that case, use {@link #getAllAnnotationsForParameter(Symbol.MethodSymbol, int)}

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java
@@ -222,7 +222,7 @@ protected boolean validateAnnotationSyntax(
   public static @Nullable VariableElement getInstanceFieldOfClass(
       Symbol.ClassSymbol classSymbol, String name) {
     Preconditions.checkNotNull(classSymbol);
-    for (Element member : classSymbol.getEnclosedElements()) {
+    for (Element member : NullabilityUtil.getEnclosedElements(classSymbol)) {
       if (member.getKind().isField() && !member.getModifiers().contains(Modifier.STATIC)) {
         if (member.getSimpleName().toString().equals(name)) {
           return (VariableElement) member;

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java
@@ -30,6 +30,7 @@
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
 import com.uber.nullaway.NullAway;
+import com.uber.nullaway.NullabilityUtil;
 import com.uber.nullaway.Nullness;
 import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
@@ -142,7 +143,7 @@ private FieldAndGetterElements getFieldAndGetterForProperty(
     Element getter = null;
     String fieldName = decapitalize(capPropName);
     String getterName = "get" + capPropName;
-    for (Symbol elem : symbol.owner.getEnclosedElements()) {
+    for (Symbol elem : NullabilityUtil.getEnclosedElements(symbol.owner)) {
       if (elem.getKind().isField() && elem.getSimpleName().toString().equals(fieldName)) {
         if (field != null) {
           throw new RuntimeException("already found field " + fieldName);

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java
@@ -35,6 +35,7 @@
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
 import com.uber.nullaway.NullAway;
+import com.uber.nullaway.NullabilityUtil;
 import com.uber.nullaway.Nullness;
 import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
@@ -122,7 +123,7 @@ public ImmutableSet<String> onRegisterImmutableTypes() {
   private Symbol.MethodSymbol getGetterForMetadataSubtype(
       Symbol.ClassSymbol classSymbol, Types types) {
     // Is there a better way than iteration?
-    for (Symbol elem : classSymbol.getEnclosedElements()) {
+    for (Symbol elem : NullabilityUtil.getEnclosedElements(classSymbol)) {
       if (elem.getKind().equals(ElementKind.METHOD)) {
         Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) elem;
         if (grpcIsMetadataGetCall(methodSymbol, types)) {