Update to Error Prone 2.21.1 (#797)

Mostly straightforward. Error Prone has a new [NullableOptional](https://errorprone.info/bugpattern/NullableOptional) check that warned on a couple instances in our code base. I changed these cases to use an `@Initializer` method which I think better captures what is going on. Will update the required CI checks before landing.
uber · Aug 11, 2023 · 9ee04c2 · 9ee04c2
1 parent bbc696c
commit 9ee04c2
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 16 deletions.
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -21,16 +21,16 @@ jobs:
             epVersion: 2.4.0
           - os: macos-latest
             java: 11
-            epVersion: 2.20.0
+            epVersion: 2.21.1
           - os: ubuntu-latest
             java: 11
-            epVersion: 2.20.0
+            epVersion: 2.21.1
           - os: windows-latest
             java: 11
-            epVersion: 2.20.0
+            epVersion: 2.21.1
           - os: ubuntu-latest
             java: 17
-            epVersion: 2.20.0
+            epVersion: 2.21.1
       fail-fast: false
     runs-on: ${{ matrix.os }}
     steps:
@@ -61,7 +61,7 @@ jobs:
         with:
           arguments: coveralls
         continue-on-error: true
-        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.20.0' && github.repository == 'uber/NullAway'
+        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.21.1' && github.repository == 'uber/NullAway'
       - name: Test publishToMavenLocal flow
         env:
           ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }}

diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -19,7 +19,7 @@ import org.gradle.util.VersionNumber
 // The oldest version of Error Prone that we support running on
 def oldestErrorProneVersion = "2.4.0"
 // Latest released Error Prone version that we've tested with
-def latestErrorProneVersion = "2.20.0"
+def latestErrorProneVersion = "2.21.1"
 // Default to using latest tested Error Prone version
 def defaultErrorProneVersion =  latestErrorProneVersion
 def errorProneVersionToCompileAgainst = defaultErrorProneVersion

diff --git a/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java b/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java
@@ -32,6 +32,7 @@ public static boolean isStatic(Symbol symbol) {
    * 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
    */
+  @SuppressWarnings("ASTHelpersSuggestions")
   public static List<Symbol> getEnclosedElements(Symbol symbol) {
     return symbol.getEnclosedElements();
   }

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java
@@ -23,7 +23,6 @@
 
 import static com.uber.nullaway.ASTHelpersBackports.getEnclosedElements;
 
-import com.google.common.base.Preconditions;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.suppliers.Supplier;
 import com.google.errorprone.suppliers.Suppliers;
@@ -33,6 +32,7 @@
 import com.sun.tools.javac.code.Types;
 import com.uber.nullaway.NullAway;
 import com.uber.nullaway.Nullness;
+import com.uber.nullaway.annotations.Initializer;
 import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import java.util.Objects;
@@ -54,8 +54,13 @@ public class ApacheThriftIsSetHandler extends BaseNoOpHandler {
 
   private static final Supplier<Type> TBASE_TYPE_SUPPLIER = Suppliers.typeFromString(TBASE_NAME);
 
-  @Nullable private Optional<Type> tbaseType;
+  private Optional<Type> tbaseType;
 
+  /**
+   * This method is annotated {@code @Initializer} since it will be invoked when the first class is
+   * processed, before any other handler methods
+   */
+  @Initializer
   @Override
   public void onMatchTopLevelClass(
       NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) {
@@ -173,7 +178,6 @@ private static String decapitalize(String str) {
   }
 
   private boolean thriftIsSetCall(Symbol.MethodSymbol symbol, Types types) {
-    Preconditions.checkNotNull(tbaseType);
     // noinspection ConstantConditions
     return tbaseType.isPresent()
         && symbol.getSimpleName().toString().startsWith("isSet")

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java
@@ -24,7 +24,6 @@
 import static com.uber.nullaway.ASTHelpersBackports.getEnclosedElements;
 import static com.uber.nullaway.NullabilityUtil.castToNonNull;
 
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.suppliers.Supplier;
@@ -37,6 +36,7 @@
 import com.sun.tools.javac.code.Types;
 import com.uber.nullaway.NullAway;
 import com.uber.nullaway.Nullness;
+import com.uber.nullaway.annotations.Initializer;
 import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import java.util.ArrayList;
@@ -61,9 +61,14 @@ public class GrpcHandler extends BaseNoOpHandler {
   private static final Supplier<Type> GRPC_METADATA_KEY_TYPE_SUPPLIER =
       Suppliers.typeFromString(GRPC_METADATA_KEY_TNAME);
 
-  @Nullable private Optional<Type> grpcMetadataType;
-  @Nullable private Optional<Type> grpcKeyType;
+  private Optional<Type> grpcMetadataType;
+  private Optional<Type> grpcKeyType;
 
+  /**
+   * This method is annotated {@code @Initializer} since it will be invoked when the first class is
+   * processed, before any other handler methods
+   */
+  @Initializer
   @Override
   public void onMatchTopLevelClass(
       NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) {
@@ -135,8 +140,6 @@ private Symbol.MethodSymbol getGetterForMetadataSubtype(
   }
 
   private boolean grpcIsMetadataContainsKeyCall(Symbol.MethodSymbol symbol, Types types) {
-    Preconditions.checkNotNull(grpcMetadataType);
-    Preconditions.checkNotNull(grpcKeyType);
     // noinspection ConstantConditions
     return grpcMetadataType.isPresent()
         && grpcKeyType.isPresent()
@@ -149,8 +152,6 @@ private boolean grpcIsMetadataContainsKeyCall(Symbol.MethodSymbol symbol, Types
   }
 
   private boolean grpcIsMetadataGetCall(Symbol.MethodSymbol symbol, Types types) {
-    Preconditions.checkNotNull(grpcMetadataType);
-    Preconditions.checkNotNull(grpcKeyType);
     // noinspection ConstantConditions
     return grpcMetadataType.isPresent()
         && grpcKeyType.isPresent()