From cc91371d8128b8f01a7b041e70bc9ad2d2f43457 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 12 Oct 2022 10:43:34 -0700 Subject: [PATCH] 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. --- .github/workflows/continuous-integration.yml | 10 +++++----- build.gradle | 3 +-- gradle/dependencies.gradle | 2 +- .../java/com/uber/nullaway/CodeAnnotationInfo.java | 11 +++++++---- .../main/java/com/uber/nullaway/ErrorBuilder.java | 4 +++- .../src/main/java/com/uber/nullaway/NullAway.java | 12 +++++++++--- .../java/com/uber/nullaway/dataflow/AccessPath.java | 8 +++++--- .../dataflow/AccessPathNullnessPropagation.java | 1 + 8 files changed, 32 insertions(+), 19 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 9db6348018..51b3931540 100644 --- 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 index c46fbb94b7..f63d161545 100644 --- 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 index 5d6ffa9c5e..ea70f80391 100755 --- 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 index 55de1b66a5..764b905c5c 100644 --- 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 index b440bc31a2..1c5b0b28ff 100755 --- 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 index 132733bea7..50f8663ddc 100644 --- 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 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 index 1b88865b37..51c7219c68 100644 --- 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 index 19e7f3c2b1..aa003d7df1 100644 --- 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()) {