Bump Error Prone and EP plugin (#675)

Bump Error Prone to 2.16 and the EP Gradle plugin to 3.0.1. Also fix some issues flagged by the new `ASTHelpersSuggestions` check. We should also update our docs to use the latest EP Gradle plugin. But I'd rather do that in a separate PR.
uber · Oct 12, 2022 · cc91371 · cc91371
1 parent 8b7c174
commit cc91371
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 19 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.15.0
+            epVersion: 2.16
           - os: ubuntu-latest
             java: 11
-            epVersion: 2.15.0
+            epVersion: 2.16
           - os: windows-latest
             java: 11
-            epVersion: 2.15.0
+            epVersion: 2.16
           - os: ubuntu-latest
             java: 17
-            epVersion: 2.15.0
+            epVersion: 2.16
       fail-fast: false
     runs-on: ${{ matrix.os }}
     steps:
@@ -70,7 +70,7 @@ jobs:
         with:
           arguments: coveralls
         continue-on-error: true
-        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.15.0' && github.repository == 'uber/NullAway'
+        if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.16' && github.repository == 'uber/NullAway'
       - name: Check that Git tree is clean after build and test
         run: ./.buildscript/check_git_clean.sh
   publish_snapshot:

diff --git a/build.gradle b/build.gradle
@@ -36,7 +36,7 @@ buildscript {
 }
 plugins {
   id "com.github.sherter.google-java-format" version "0.9"
-  id "net.ltgt.errorprone" version "2.0.2" apply false
+  id "net.ltgt.errorprone" version "3.0.1" apply false
   id "com.github.johnrengelman.shadow" version "6.1.0" apply false
   id "com.github.kt3k.coveralls" version "2.12.0" apply false
   id "me.champeau.jmh" version "0.6.7" apply false
@@ -54,7 +54,6 @@ subprojects { project ->
     project.apply plugin: "net.ltgt.errorprone"
     project.dependencies {
         errorprone deps.build.errorProneCore
-        errorproneJavac deps.build.errorProneJavac
     }
     project.tasks.withType(JavaCompile) {
         dependsOn(installGitHooks)

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.15.0"
+def latestErrorProneVersion = "2.16"
 // Default to using latest tested Error Prone version, except on Java 8, where 2.10.0 is the last version
 // that works
 def defaultErrorProneVersion =  JavaVersion.current() >= JavaVersion.VERSION_11 ? latestErrorProneVersion : "2.10.0"

diff --git a/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java b/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java
@@ -78,16 +78,19 @@ public static CodeAnnotationInfo instance(Context context) {
   private static boolean fromAnnotatedPackage(
       Symbol.ClassSymbol outermostClassSymbol, Config config) {
     final String className = outermostClassSymbol.getQualifiedName().toString();
+    Symbol.PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(outermostClassSymbol);
     if (!config.fromExplicitlyAnnotatedPackage(className)
-        && !ASTHelpers.hasDirectAnnotationWithSimpleName(
-            outermostClassSymbol.packge(), NullabilityUtil.NULLMARKED_SIMPLE_NAME)) {
+        && !(enclosingPackage != null
+            && ASTHelpers.hasDirectAnnotationWithSimpleName(
+                enclosingPackage, NullabilityUtil.NULLMARKED_SIMPLE_NAME))) {
       // By default, unknown code is unannotated unless @NullMarked or configured as annotated by
       // package name
       return false;
     }
     if (config.fromExplicitlyUnannotatedPackage(className)
-        || ASTHelpers.hasDirectAnnotationWithSimpleName(
-            outermostClassSymbol.packge(), NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
+        || (enclosingPackage != null
+            && ASTHelpers.hasDirectAnnotationWithSimpleName(
+                enclosingPackage, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME))) {
       // Any code explicitly marked as unannotated in our configuration is unannotated, no matter
       // what. Similarly, any package annotated as @NullUnmarked is unannotated, even if
       // explicitly passed to -XepOpt:NullAway::AnnotatedPackages

diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java
@@ -468,7 +468,9 @@ void reportInitErrorOnField(Symbol symbol, VisitorState state, Description.Build
       fieldName = flatName.substring(index) + "." + fieldName;
     }
 
-    if (symbol.isStatic()) {
+    @SuppressWarnings("ASTHelpersSuggestions") // remove once we require EP 2.16 or greater
+    boolean isStatic = symbol.isStatic();
+    if (isStatic) {
       state.reportMatch(
           createErrorDescription(
               new ErrorMessage(

diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -988,7 +988,9 @@ private Description checkForReadBeforeInit(ExpressionTree tree, VisitorState sta
     }
 
     // for static fields, make sure the enclosing init is a static method or block
-    if (symbol.isStatic()) {
+    @SuppressWarnings("ASTHelpersSuggestions") // remove once we require EP 2.16 or greater
+    boolean isStatic = symbol.isStatic();
+    if (isStatic) {
       Tree enclosing = enclosingBlockPath.getLeaf();
       if (enclosing instanceof MethodTree
           && !ASTHelpers.getSymbol((MethodTree) enclosing).isStatic()) {
@@ -1097,7 +1099,9 @@ private boolean fieldAlwaysInitializedBeforeRead(
       Symbol symbol, TreePath pathToRead, VisitorState state, TreePath enclosingBlockPath) {
     AccessPathNullnessAnalysis nullnessAnalysis = getNullnessAnalysis(state);
     Set<Element> nonnullFields;
-    if (symbol.isStatic()) {
+    @SuppressWarnings("ASTHelpersSuggestions") // remove once we require EP 2.16 or greater
+    boolean isStatic = symbol.isStatic();
+    if (isStatic) {
       nonnullFields = nullnessAnalysis.getNonnullStaticFieldsBefore(pathToRead, state.context);
     } else {
       nonnullFields = new LinkedHashSet<>();
@@ -2063,7 +2067,9 @@ private FieldInitEntities collectEntities(ClassTree tree, VisitorState state) {
             // matchVariable()
             continue;
           }
-          if (fieldSymbol.isStatic()) {
+          @SuppressWarnings("ASTHelpersSuggestions") // remove once we require EP 2.16 or greater
+          boolean fieldIsStatic = fieldSymbol.isStatic();
+          if (fieldIsStatic) {
             nonnullStaticFields.add(fieldSymbol);
           } else {
             nonnullInstanceFields.add(fieldSymbol);

diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java
@@ -365,9 +365,11 @@ public static AccessPath fromFieldElement(VariableElement element) {
   }
 
   private static boolean isBoxingMethod(Symbol.MethodSymbol methodSymbol) {
-    return methodSymbol.isStatic()
-        && methodSymbol.getSimpleName().contentEquals("valueOf")
-        && methodSymbol.enclClass().packge().fullname.contentEquals("java.lang");
+    if (methodSymbol.isStatic() && methodSymbol.getSimpleName().contentEquals("valueOf")) {
+      Symbol.PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(methodSymbol.enclClass());
+      return enclosingPackage != null && enclosingPackage.fullname.contentEquals("java.lang");
+    }
+    return false;
   }
 
   /**

diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java
@@ -761,6 +761,7 @@ private CodeAnnotationInfo getCodeAnnotationInfo(VisitorState state) {
     return codeAnnotationInfo;
   }
 
+  @SuppressWarnings("ASTHelpersSuggestions") // remove once we require EP 2.16 or greater
   private void setReceiverNonnull(
       AccessPathNullnessPropagation.ReadableUpdates updates, Node receiver, Symbol symbol) {
     if (symbol != null && !symbol.isStatic()) {