aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSorin Basca <sorinbasca@google.com>2024-02-02 13:23:18 +0000
committerSorin Basca <sorinbasca@google.com>2024-02-02 13:23:18 +0000
commit01752c0520b50b1a042178e6c21e8eb4090cfe01 (patch)
treeccc85488637ab1f4842a3c579a083e52d193a0cd
parent3486bbdf92d21d6b3ae1f7f7eb35ff4e7126a8f0 (diff)
parent986d060b570c629cb00b0b77fd6ec4e019983fbc (diff)
downloadnullaway-01752c0520b50b1a042178e6c21e8eb4090cfe01.tar.gz
Merge commit '0.10.22' into main
Bug: 313924276 Test: RUN_ERROR_PRONE=true m Change-Id: Iec85dbae53ab406d88fff2d3d195e482d4c32895
-rw-r--r--.github/workflows/continuous-integration.yml10
-rwxr-xr-xCHANGELOG.md98
-rw-r--r--README.md10
-rw-r--r--build.gradle6
-rw-r--r--buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle36
-rw-r--r--gradle.properties2
-rwxr-xr-xgradle/dependencies.gradle8
-rw-r--r--gradle/wrapper/gradle-wrapper.jarbin63721 -> 43462 bytes
-rw-r--r--gradle/wrapper/gradle-wrapper.properties4
-rw-r--r--jar-infer/jar-infer-lib/build.gradle6
-rw-r--r--jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java20
-rw-r--r--nullaway/build.gradle33
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java20
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java25
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/LibraryModels.java21
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/NullAway.java58
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java15
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java28
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java151
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java42
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java6
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java12
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java10
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java68
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java21
-rw-r--r--nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java65
-rw-r--r--nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java41
-rw-r--r--nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java37
-rw-r--r--nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java80
-rw-r--r--sample-app/build.gradle33
-rw-r--r--sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java5
-rw-r--r--test-java-lib/src/main/java/com/uber/lib/unannotated/UnannotatedWithModels.java4
-rw-r--r--test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java10
33 files changed, 811 insertions, 174 deletions
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
index 62adc2f..00c76ab 100644
--- a/.github/workflows/continuous-integration.yml
+++ b/.github/workflows/continuous-integration.yml
@@ -21,16 +21,16 @@ jobs:
epVersion: 2.10.0
- os: macos-latest
java: 11
- epVersion: 2.23.0
+ epVersion: 2.24.1
- os: ubuntu-latest
java: 11
- epVersion: 2.23.0
+ epVersion: 2.24.1
- os: windows-latest
java: 11
- epVersion: 2.23.0
+ epVersion: 2.24.1
- os: ubuntu-latest
java: 17
- epVersion: 2.23.0
+ epVersion: 2.24.1
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
@@ -63,7 +63,7 @@ jobs:
with:
arguments: codeCoverageReport
continue-on-error: true
- if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.23.0' && github.repository == 'uber/NullAway'
+ if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.24.1' && github.repository == 'uber/NullAway'
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
with:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eda8659..1854e1f 100755
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,45 @@
Changelog
=========
+Version 0.10.22
+---------------
+IMPORTANT: The support for JDK 8 is deprecated in this release and will be removed in
+ an upcoming release.
+
+* Fix bug with implicit equals() methods in interfaces (#898)
+* Fix crash with raw types in overrides in JSpecify mode (#899)
+* Docs fix: Update instructions for Android and our sample app (#900)
+
+Version 0.10.21
+---------------
+IMPORTANT: This release fixes a crash when running against <2.24.0 release of
+ Error Prone (see #894) introduced in NullAway v0.10.20 and another crash related to
+ Checker Framework (see #895) introduced in NullAway v0.10.19.
+
+* Fix backwards-incompatible calls to ASTHelpers.hasDirectAnnotationWithSimpleName (#894)
+* Downgrade to Checker Framework 3.40.0 (#895)
+
+Version 0.10.20
+---------------
+* Fix JSpecify support on JDK 21 (#869)
+* Build / CI tooling upgrades for NullAway itself:
+ - Update to WALA 1.6.3 (#887)
+ - Update to Error Prone 2.24.1 (#888)
+
+Version 0.10.19
+---------------
+* Update to Checker Framework 3.41.0 (#873)
+* Extend library models to mark fields as nullable (#878)
+ - Main use case is NullAwayAnnotator
+* Fix jarinfer cli output determinism (#884)
+* Add support for AssertJ as() and describedAs() in AssertionHandler (#885)
+* Support for JSpecify's 0.3.0 annotation [experimental]
+ - JSpecify: In generics code, get rid of checks for ClassType (#863)
+* Update some dependencies (#883)
+
+Version 0.10.18
+---------------
+* Fix assertion check for structure of enhanced-for loop over a Map keySet (#868)
+
Version 0.10.17
---------------
* Fix bug with computing direct type use annotations on parameters (#864)
@@ -95,7 +135,7 @@ Note: This is the first release built with Java 11. In particular, running
- Update to Error Prone 2.20.0 (#772)
- Add tasks to run JDK 8 tests on JDK 11+ (#778)
- Switch to Spotless for formatting Java code (#780)
- - Added GCP JMH Benchmark Workflow (#770)
+ - Added GCP JMH Benchmark Workflow (#770)
- Set concurrency for JMH benchmarking workflow (#784)
- Disable daemon when running benchmarks (#786)
- Update to Gradle 8.2.1 (#781)
@@ -179,7 +219,7 @@ Version 0.10.6
* Fix logic for @Nullable annotation on type parameter (#702)
* Preserve nullness checks in final fields when propagating nullness into inner contexts (#703)
* NullAwayInfer/Annotator data serialization support [experimental]
- - Add source offset and path to reported errors in error serialization. (#704)
+ - Add source offset and path to reported errors in error serialization. (#704)
* Build / CI tooling for NullAway itself:
- [Jspecify] Update test dep to final JSpecify 0.3.0 release (#700)
= Intermediate PRs: 0.3.0-alpha-3 (#692), 0.3-alpha2 (#691)
@@ -211,10 +251,10 @@ Version 0.10.3
--------------
* Report an error when casting @Nullable expression to primitive type (#663)
* Fix an NPE in the optional emptiness handler (#678)
-* Add support for boolean constraints (about nullness) in Contract annotations (#669)
+* Add support for boolean constraints (about nullness) in Contract annotations (#669)
* Support for specific libraries/APIs:
- PreconditionsHandler reflects Guava Preconditions exception types (#668)
- - Handle Guava Verify functions (#682)
+ - Handle Guava Verify functions (#682)
* Dependency Updates:
- checkerframework 3.26.0 (#671)
* Build / CI tooling for NullAway itself:
@@ -223,7 +263,7 @@ Version 0.10.3
Version 0.10.2
--------------
-* Make AbstractConfig collection fields explicity Immutable (#601)
+* Make AbstractConfig collection fields explicity Immutable (#601)
* NullAwayInfer/Annotator data serialization support [experimental]
- Fix crash in fixserialization when ClassSymbol.sourcefile is null (#656)
@@ -287,7 +327,7 @@ Version 0.9.8
- Model for com.google.api.client.util.Strings.isNullOrEmpty (#605)
* Refactoring:
- Cleanups to AccessPath representation and implementation (#603)
- - Clean-up: Remove unused fix suggestion code. (#615)
+ - Clean-up: Remove unused fix suggestion code. (#615)
* Dependency Updates:
- Update to Checker Framework 3.22.2 (#610)
* Build / CI tooling for NullAway itself:
@@ -339,7 +379,7 @@ Version 0.9.6
* Build / CI tooling for NullAway itself:
- Enable parallel builds (#549) (#555)
- Add dependence from coveralls task to codeCoverageReport (#552)
- - Switch to temurin on CI (#553)
+ - Switch to temurin on CI (#553)
- Separating NullAwayTests into smaller files (#550)
- Require braces for all conditionals and loops (#556)
- Enable build cache (#562)
@@ -349,7 +389,7 @@ Version 0.9.6
- Update CI jobs (#565)
- Set epApiVersion for jacoco coverage reporting (#566)
- Compile and test against Error Prone 2.11.0 (#567)
- - Fix EP version for jacoco coverage step (#571)
+ - Fix EP version for jacoco coverage step (#571)
- Update to latest Google Java Format (#572)
Version 0.9.5
@@ -371,7 +411,7 @@ Version 0.9.3
-------------
IMPORTANT: This version introduces EXPERIMENTAL JDK17 support.
There is a known crash on lambdas with switch expressions as body
- (see #524). Best current workaround is to
+ (see #524). Best current workaround is to
`@SuppressWarnings("NullAway")` on the enclosing method
* Improve reporting of multiple parameter errors on a single method call (#503)
* Support compile-time constant field args in method Access Paths (#504)
@@ -463,13 +503,13 @@ Version 0.8.0
Version 0.7.10
--------------
-* Add Java 8 streams nullness-propagation support (#371)
+* Add Java 8 streams nullness-propagation support (#371)
* Give line numbers for uninitialized fields when reporting error on an initializer (#380)
-* Include outer$inner class name when reporting field init errors (#375)
+* Include outer$inner class name when reporting field init errors (#375)
* Update to Gradle 6.1.1 (#381)
* Add @MonotonicNonNull as lazy initialization annotation. (#383)
* Add default library model for CompilationUnitTree.getPackageName() (#384)
-* Improve matching of native Map methods (#390)
+* Improve matching of native Map methods (#390)
- Fixes an IndexOutOfBoundsException checker crash
Version 0.7.9
@@ -479,14 +519,14 @@ Version 0.7.9
- WALA to 1.5.4 (#337)
- Checker Dataflow to 3.0.0 (#369)
* Added OPTIONAL_CONTENT synthetic field to track Optional emptiness (#364)
- - With this, `-XepOpt:NullAway:CheckOptionalEmptiness` should be
+ - With this, `-XepOpt:NullAway:CheckOptionalEmptiness` should be
ready for use.
* Handle Nullchk operator (#368)
Version 0.7.8
-------------
-* Added NullAway.Optional suppression (#359)
-* [JarInfer] Ignore non-public classes when inferring annotations. (#360)
+* Added NullAway.Optional suppression (#359)
+* [JarInfer] Ignore non-public classes when inferring annotations. (#360)
Version 0.7.7
-------------
@@ -522,7 +562,7 @@ Version 0.7.4
* Refactor the driver and annotation summary type in JarInfer (#317)
* Minor refactor and cleanup in JarInfer-lib (#319)
* Different approach for param analysis (#320)
-* Fix @NullableDecl support (#324)
+* Fix @NullableDecl support (#324)
* Treat methods of final classes as final for initialization. (#325)
Version 0.7.3
@@ -536,7 +576,7 @@ Version 0.7.3
Version 0.7.2
-------------
-* Install GJF hook using a gradle task, rather than a gradlew hack (#298).
+* Install GJF hook using a gradle task, rather than a gradlew hack (#298).
* Nullable switch expression support (#300).
* Upgrade to Error Prone 2.3.3 (#295).
Update Gradle, Error Prone plugin, and Android Gradle Plugin (#294).
@@ -557,7 +597,7 @@ Version 0.7.0
Version 0.6.6
---------------
-This only adds a minor library fix supporting Guava's Preconditions.checkNotNull with an error message
+This only adds a minor library fix supporting Guava's Preconditions.checkNotNull with an error message
argument (#283)
Version 0.6.5
@@ -587,16 +627,16 @@ Version 0.6.2
Version 0.6.1
-------------
* Enable excluded class annotations to (mostly) work on inner classes (#239)
-* Assertion of not equal to null updates the access path (#240)
+* Assertion of not equal to null updates the access path (#240)
* Update Gradle examples in README (#244)
* Change how jarinfer finds astubx model jars. (#243)
* Update to Error Prone 2.3.2 (#242)
* Update net.ltgt.errorprone to 0.6, and build updates ((#248)
-* Restrictive annotated method overriding (#249)
- Note: This can require significant annotation changes if
+* Restrictive annotated method overriding (#249)
+ Note: This can require significant annotation changes if
`-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true` is set.
Not a new minor version, since that option is false by default.
-* Fix error on checking the initTree2PrevFieldInit cache. (#252)
+* Fix error on checking the initTree2PrevFieldInit cache. (#252)
* Add support for renamed android.support packages in models. (#253)
Version 0.6.0
@@ -604,12 +644,12 @@ Version 0.6.0
* Add support for marking library parameters as explicitly @Nullable (#228)
* De-genericize NullnessStore (#231)
* Bump Checker Framework to 2.5.5 (#233)
-* Pass nullability info on enclosing locals into dataflow analysis for
+* Pass nullability info on enclosing locals into dataflow analysis for
lambdas and anonymous / local classes (#235)
Version 0.5.6
-------------
-* Add coverage measurement through coveralls. (#224)
+* Add coverage measurement through coveralls. (#224)
* Fix empty comment added when AutoFixSuppressionComment is not set. (#225)
* Make JarInfer generated jars fully deterministic by removing timestamps. (#227)
@@ -642,7 +682,7 @@ android-jarinfer-models-sdk28 artifacts
Version 0.5.2
-------------
* Fix NPE in Thrift handler on complex receiver expressions (#195)
-* Add ExcludedFieldAnnotations unit tests. (#192)
+* Add ExcludedFieldAnnotations unit tests. (#192)
* Various crash fixes (#196)
* Fix @NonNull argument detection in RestrictiveAnnotationHandler. (#198)
@@ -668,7 +708,7 @@ Version 0.4.7
Version 0.4.6
-------------
* Fix a couple of Thrift issues (#164)
-* Don't report initialization warnings on fields for @ExternalInit classes with
+* Don't report initialization warnings on fields for @ExternalInit classes with
no initializer methods (#166)
Version 0.4.5
@@ -717,7 +757,7 @@ Version 0.3.6
Version 0.3.5
-------------
-* Support for treating `@Generated`-annotated classes as unannotated (#127)
+* Support for treating `@Generated`-annotated classes as unannotated (#127)
Version 0.3.4
-------------
@@ -789,8 +829,8 @@ Version 0.1.5
-------------
* Add finer grained suppressions and auto-fixes (#31). You can
suppress initialization errors specifically now with
- `@SuppressWarnings("NullAway.Init")`
-* Fix performance issue with lambdas (#29)
+ `@SuppressWarnings("NullAway.Init")`
+* Fix performance issue with lambdas (#29)
* Add lambda support to the RxNullabilityPropagator handler. (#12)
Version 0.1.4
diff --git a/README.md b/README.md
index 1376997..e968cdd 100644
--- a/README.md
+++ b/README.md
@@ -58,15 +58,9 @@ Snapshots of the development version are available in [Sonatype's snapshots repo
#### Android
-The configuration for an Android project is very similar to the Java case, with one key difference: The `com.google.code.findbugs:jsr305:3.0.2` dependency can be removed; you can use the `android.support.annotation.Nullable` annotation from the Android Support library.
+Versions 3.0.0 and later of the Gradle Error Prone Plugin [no longer support Android](https://github.com/tbroyer/gradle-errorprone-plugin/releases/tag/v3.0.0). So if you're using a recent version of this plugin, you'll need to add some further configuration to run Error Prone and NullAway. Our [sample app `build.gradle` file](https://github.com/uber/NullAway/blob/master/sample-app/build.gradle) shows one way to do this, but your Android project may require tweaks. Alternately, 2.x versions of the Gradle Error Prone Plugin still support Android and may still work with your project.
-```gradle
-dependencies {
- errorprone "com.uber.nullaway:nullaway:<NullAway version>"
- errorprone "com.google.errorprone:error_prone_core:<Error Prone version>"
-}
-```
-For a more complete example see our [sample app](https://github.com/uber/NullAway/blob/master/sample-app/). (The sample app's [`build.gradle`](https://github.com/uber/NullAway/blob/master/sample-app/) is not suitable for direct copy-pasting, as some configuration is inherited from the top-level `build.gradle`.)
+Beyond that, compared to the Java configuration, the `com.google.code.findbugs:jsr305:3.0.2` dependency can be removed; you can use the `android.support.annotation.Nullable` annotation from the Android Support library instead.
#### Annotation Processors / Generated Code
diff --git a/build.gradle b/build.gradle
index 729ffc2..5ccd8b0 100644
--- a/build.gradle
+++ b/build.gradle
@@ -27,11 +27,11 @@ buildscript {
}
}
plugins {
- id "com.diffplug.spotless" version "6.21.0"
+ id "com.diffplug.spotless" version "6.23.3"
id "net.ltgt.errorprone" version "3.1.0" apply false
id "com.github.johnrengelman.shadow" version "8.1.1" apply false
id "me.champeau.jmh" version "0.7.1" apply false
- id "com.github.ben-manes.versions" version "0.48.0"
+ id "com.github.ben-manes.versions" version "0.50.0"
id "com.felipefzdz.gradle.shellcheck" version "1.4.6"
}
@@ -118,7 +118,7 @@ spotless {
}
}
spotlessPredeclare {
- java { googleJavaFormat('1.17.0') }
+ java { googleJavaFormat('1.18.1') }
groovyGradle {
greclipse()
}
diff --git a/buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle b/buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
index 07f8f26..6a0e641 100644
--- a/buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
+++ b/buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
@@ -44,22 +44,6 @@ configurations.create('transitiveSourcesElements') {
}
}
-// Share the coverage data to be aggregated for the whole product
-configurations.create('coverageDataElements') {
- visible = false
- canBeResolved = false
- canBeConsumed = true
- extendsFrom(configurations.implementation)
- attributes {
- attribute(Usage.USAGE_ATTRIBUTE, objects.named(Usage, Usage.JAVA_RUNTIME))
- attribute(Category.CATEGORY_ATTRIBUTE, objects.named(Category, Category.DOCUMENTATION))
- attribute(DocsType.DOCS_TYPE_ATTRIBUTE, objects.named(DocsType, 'jacoco-coverage-data'))
- }
- // This will cause the test task to run if the coverage data is requested by the aggregation task
- outgoing.artifact(tasks.named("test").map { task ->
- task.extensions.getByType(JacocoTaskExtension).destinationFile
- })
-}
test {
maxHeapSize = "1024m"
@@ -118,3 +102,23 @@ def testJdk21 = tasks.register("testJdk21", Test) {
tasks.named('check').configure {
dependsOn testJdk21
}
+
+
+// Share the coverage data to be aggregated for the whole product
+configurations.create('coverageDataElements') {
+ visible = false
+ canBeResolved = false
+ canBeConsumed = true
+ extendsFrom(configurations.implementation)
+ attributes {
+ attribute(Usage.USAGE_ATTRIBUTE, objects.named(Usage, Usage.JAVA_RUNTIME))
+ attribute(Category.CATEGORY_ATTRIBUTE, objects.named(Category, Category.DOCUMENTATION))
+ attribute(DocsType.DOCS_TYPE_ATTRIBUTE, objects.named(DocsType, 'jacoco-coverage-data'))
+ }
+ // This will cause the test tasks to run if the coverage data is requested by the aggregation task
+ tasks.withType(Test).forEach {task ->
+ // tasks.named(task.name) is a hack to introduce the right task dependence in Gradle. We will fix this
+ // correctly when addressing https://github.com/uber/NullAway/issues/871
+ outgoing.artifact(tasks.named(task.name).map { t -> t.extensions.getByType(JacocoTaskExtension).destinationFile })
+ }
+}
diff --git a/gradle.properties b/gradle.properties
index fe3f54f..3c140cc 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -12,7 +12,7 @@ org.gradle.caching=true
org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m
GROUP=com.uber.nullaway
-VERSION_NAME=0.10.18-SNAPSHOT
+VERSION_NAME=0.10.22
POM_DESCRIPTION=A fast annotation-based null checker for Java
diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
index 566b5cf..bada5ee 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.10.0"
// Latest released Error Prone version that we've tested with
-def latestErrorProneVersion = "2.23.0"
+def latestErrorProneVersion = "2.24.1"
// Default to using latest tested Error Prone version
def defaultErrorProneVersion = latestErrorProneVersion
def errorProneVersionToCompileAgainst = defaultErrorProneVersion
@@ -40,7 +40,7 @@ if (project.hasProperty("epApiVersion")) {
def versions = [
asm : "9.3",
- checkerFramework : "3.39.0",
+ checkerFramework : "3.40.0",
// for comparisons in other parts of the build
errorProneLatest : latestErrorProneVersion,
// The version of Error Prone used to check NullAway's code.
@@ -48,7 +48,7 @@ def versions = [
// The version of Error Prone that NullAway is compiled and tested against
errorProneApi : errorProneVersionToCompileAgainst,
support : "27.1.1",
- wala : "1.6.2",
+ wala : "1.6.3",
commonscli : "1.4",
autoValue : "1.10.2",
autoService : "1.1.1",
@@ -67,10 +67,12 @@ def build = [
asm : "org.ow2.asm:asm:${versions.asm}",
asmTree : "org.ow2.asm:asm-tree:${versions.asm}",
errorProneCheckApi : "com.google.errorprone:error_prone_check_api:${versions.errorProneApi}",
+ errorProneCheckApiOld : "com.google.errorprone:error_prone_check_api:${oldestErrorProneVersion}",
errorProneCore : "com.google.errorprone:error_prone_core:${versions.errorProne}",
errorProneCoreForApi : "com.google.errorprone:error_prone_core:${versions.errorProneApi}",
errorProneJavac : "com.google.errorprone:javac:9+181-r4173-1",
errorProneTestHelpers : "com.google.errorprone:error_prone_test_helpers:${versions.errorProneApi}",
+ errorProneTestHelpersOld: "com.google.errorprone:error_prone_test_helpers:${oldestErrorProneVersion}",
checkerDataflow : "org.checkerframework:dataflow-nullaway:${versions.checkerFramework}",
guava : "com.google.guava:guava:30.1-jre",
javaxValidation : "javax.validation:validation-api:2.0.1.Final",
diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar
index 7f93135..d64cd49 100644
--- a/gradle/wrapper/gradle-wrapper.jar
+++ b/gradle/wrapper/gradle-wrapper.jar
Binary files differ
diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
index 46671ac..db8c3ba 100644
--- a/gradle/wrapper/gradle-wrapper.properties
+++ b/gradle/wrapper/gradle-wrapper.properties
@@ -1,7 +1,7 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
-distributionSha256Sum=3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip
+distributionSha256Sum=9d926787066a081739e8200858338b4a69e837c3a821a33aca9db09dd4a41026
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.5-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
diff --git a/jar-infer/jar-infer-lib/build.gradle b/jar-infer/jar-infer-lib/build.gradle
index 2ae8bea..8ea4f9b 100644
--- a/jar-infer/jar-infer-lib/build.gradle
+++ b/jar-infer/jar-infer-lib/build.gradle
@@ -51,12 +51,6 @@ test {
dependsOn ':jar-infer:test-android-lib-jarinfer:bundleReleaseAar'
}
-tasks.named('testJdk21', Test).configure {
- // Tests fail since WALA does not yet support JDK 21; see https://github.com/uber/NullAway/issues/829
- // So, disable them
- onlyIf { false }
-}
-
tasks.withType(JavaCompile).configureEach {
options.compilerArgs += "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED"
}
diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
index 94e128c..52498fd 100644
--- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
+++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java
@@ -212,6 +212,18 @@ public final class BytecodeAnnotator {
annotateBytecode(is, os, nonnullParams, nullableReturns, javaxNullableDesc, javaxNonnullDesc);
}
+ /**
+ * Create a zip entry with creation time of 0 to ensure that jars always have the same checksum.
+ *
+ * @param name of the zip entry.
+ * @return the zip entry.
+ */
+ private static ZipEntry createZipEntry(String name) {
+ ZipEntry entry = new ZipEntry(name);
+ entry.setTime(0);
+ return entry;
+ }
+
private static void copyAndAnnotateJarEntry(
JarEntry jarEntry,
InputStream is,
@@ -224,7 +236,7 @@ public final class BytecodeAnnotator {
throws IOException {
String entryName = jarEntry.getName();
if (entryName.endsWith(".class")) {
- jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ jarOS.putNextEntry(createZipEntry(jarEntry.getName()));
annotateBytecode(is, jarOS, nonnullParams, nullableReturns, nullableDesc, nonnullDesc);
} else if (entryName.equals("META-INF/MANIFEST.MF")) {
// Read full file
@@ -241,7 +253,7 @@ public final class BytecodeAnnotator {
if (!manifestText.equals(manifestMinusDigests) && !stripJarSignatures) {
throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
}
- jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ jarOS.putNextEntry(createZipEntry(jarEntry.getName()));
jarOS.write(manifestMinusDigests.getBytes(UTF_8));
} else if (entryName.startsWith("META-INF/")
&& (entryName.endsWith(".DSA")
@@ -251,7 +263,7 @@ public final class BytecodeAnnotator {
throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
} // the case where stripJarSignatures==true is handled by default by skipping these files
} else {
- jarOS.putNextEntry(new ZipEntry(jarEntry.getName()));
+ jarOS.putNextEntry(createZipEntry(jarEntry.getName()));
jarOS.write(IOUtils.toByteArray(is));
}
jarOS.closeEntry();
@@ -329,7 +341,7 @@ public final class BytecodeAnnotator {
while (zipIterator.hasNext()) {
ZipEntry zipEntry = zipIterator.next();
InputStream is = inputZip.getInputStream(zipEntry);
- zipOS.putNextEntry(new ZipEntry(zipEntry.getName()));
+ zipOS.putNextEntry(createZipEntry(zipEntry.getName()));
if (zipEntry.getName().equals("classes.jar")) {
JarInputStream jarIS = new JarInputStream(is);
JarEntry inputJarEntry = jarIS.getNextJarEntry();
diff --git a/nullaway/build.gradle b/nullaway/build.gradle
index 90ac062..15327ae 100644
--- a/nullaway/build.gradle
+++ b/nullaway/build.gradle
@@ -21,7 +21,8 @@ plugins {
}
configurations {
- nullawayJar
+ // A configuration holding the jars for the oldest supported version of Error Prone, to use with tests
+ errorProneOldest
}
dependencies {
@@ -64,6 +65,14 @@ dependencies {
testImplementation deps.test.mockito
testImplementation deps.test.javaxAnnotationApi
testImplementation deps.test.assertJ
+ // This is for a test exposing a CFG construction failure in the Checker Framework. We can probably remove it once
+ // the issue is fixed upstream and we update. See https://github.com/typetools/checker-framework/issues/6396.
+ testImplementation 'org.apache.spark:spark-sql_2.12:3.3.2'
+
+ errorProneOldest deps.build.errorProneCheckApiOld
+ errorProneOldest(deps.build.errorProneTestHelpersOld) {
+ exclude group: "junit", module: "junit"
+ }
}
javadoc {
@@ -93,12 +102,10 @@ apply plugin: 'com.vanniktech.maven.publish'
// }
// Create a task to test on JDK 8
+// NOTE: even when we drop JDK 8 support, we will still need a test task similar to this one for testing building
+// against a recent JDK and Error Prone version but then running on the oldest supported JDK and Error Prone version,
+// to check for binary compatibility issues.
def jdk8Test = tasks.register("testJdk8", Test) {
- onlyIf {
- // Only if we are using a version of Error Prone compatible with JDK 8
- deps.versions.errorProneApi == "2.10.0"
- }
-
javaLauncher = javaToolchains.launcherFor {
languageVersion = JavaLanguageVersion.of(8)
}
@@ -108,7 +115,11 @@ def jdk8Test = tasks.register("testJdk8", Test) {
// Copy inputs from normal Test task.
def testTask = tasks.getByName("test")
- classpath = testTask.classpath
+ // A bit of a hack: we add the dependencies of the oldest supported Error Prone version to the _beginning_ of the
+ // classpath, so that they are used instead of the latest version. This exercises the scenario of building
+ // NullAway against the latest supported Error Prone version but then running on the oldest supported version.
+ classpath = configurations.errorProneOldest + testTask.classpath
+
testClassesDirs = testTask.testClassesDirs
jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}"
filter {
@@ -124,14 +135,6 @@ tasks.named('check').configure {
dependsOn(jdk8Test)
}
-tasks.named('testJdk21', Test).configure {
- filter {
- // JSpecify Generics tests do not yet pass on JDK 21
- // See https://github.com/uber/NullAway/issues/827
- excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests"
- }
-}
-
// Create a task to build NullAway with NullAway checking enabled
tasks.register('buildWithNullAway', JavaCompile) {
onlyIf {
diff --git a/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java b/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java
index 06a91f9..47dc99f 100644
--- a/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java
+++ b/nullaway/src/main/java/com/uber/nullaway/ASTHelpersBackports.java
@@ -1,12 +1,15 @@
package com.uber.nullaway;
+import com.google.errorprone.util.ASTHelpers;
import com.sun.tools.javac.code.Symbol;
import java.util.List;
/**
* Methods backported from {@link com.google.errorprone.util.ASTHelpers} since we do not yet require
* a recent-enough Error Prone version. The methods should be removed once we bump our minimum Error
- * Prone version accordingly.
+ * Prone version accordingly. We also include methods where new overloads have been added in recent
+ * Error Prone versions, to ensure binary compatibility when compiling against a new version but
+ * running on an old version.
*/
public class ASTHelpersBackports {
@@ -36,4 +39,19 @@ public class ASTHelpersBackports {
public static List<Symbol> getEnclosedElements(Symbol symbol) {
return symbol.getEnclosedElements();
}
+
+ /**
+ * A wrapper for {@link ASTHelpers#hasDirectAnnotationWithSimpleName(Symbol, String)} to avoid
+ * binary compatibility issues with new overloads in recent Error Prone versions. NullAway code
+ * should only use this method and not call the corresponding ASTHelpers methods directly.
+ *
+ * <p>TODO: delete this method and switch to ASTHelpers once we can require Error Prone 2.24.0
+ *
+ * @param sym the symbol
+ * @param simpleName the simple name
+ * @return {@code true} iff the symbol has a direct annotation with the given simple name
+ */
+ public static boolean hasDirectAnnotationWithSimpleName(Symbol sym, String simpleName) {
+ return ASTHelpers.hasDirectAnnotationWithSimpleName(sym, simpleName);
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java b/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java
index 013e8f4..37fcc03 100644
--- a/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java
+++ b/nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java
@@ -22,6 +22,7 @@
package com.uber.nullaway;
+import static com.uber.nullaway.ASTHelpersBackports.hasDirectAnnotationWithSimpleName;
import static com.uber.nullaway.NullabilityUtil.castToNonNull;
import com.google.common.base.Preconditions;
@@ -81,7 +82,7 @@ public final class CodeAnnotationInfo {
Symbol.PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(outermostClassSymbol);
if (!config.fromExplicitlyAnnotatedPackage(className)
&& !(enclosingPackage != null
- && ASTHelpers.hasDirectAnnotationWithSimpleName(
+ && hasDirectAnnotationWithSimpleName(
enclosingPackage, NullabilityUtil.NULLMARKED_SIMPLE_NAME))) {
// By default, unknown code is unannotated unless @NullMarked or configured as annotated by
// package name
@@ -89,7 +90,7 @@ public final class CodeAnnotationInfo {
}
if (config.fromExplicitlyUnannotatedPackage(className)
|| (enclosingPackage != null
- && ASTHelpers.hasDirectAnnotationWithSimpleName(
+ && 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
@@ -120,7 +121,7 @@ public final class CodeAnnotationInfo {
return false;
}
Symbol.ClassSymbol outermostClassSymbol = get(classSymbol, config).outermostClassSymbol;
- return ASTHelpers.hasDirectAnnotationWithSimpleName(outermostClassSymbol, "Generated");
+ return hasDirectAnnotationWithSimpleName(outermostClassSymbol, "Generated");
}
/**
@@ -216,10 +217,10 @@ public final class CodeAnnotationInfo {
if (enclosingMethod != null) {
isAnnotated = recordForEnclosing.isMethodNullnessAnnotated(enclosingMethod);
}
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
+ if (hasDirectAnnotationWithSimpleName(
classSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
isAnnotated = false;
- } else if (ASTHelpers.hasDirectAnnotationWithSimpleName(
+ } else if (hasDirectAnnotationWithSimpleName(
classSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) {
isAnnotated = true;
}
@@ -244,7 +245,7 @@ public final class CodeAnnotationInfo {
// Generated code is or isn't excluded, depending on configuration
// Note: In the future, we might want finer grain controls to distinguish code that is
// generated with nullability info and without.
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(classSymbol, "Generated")) {
+ if (hasDirectAnnotationWithSimpleName(classSymbol, "Generated")) {
return true;
}
ImmutableSet<String> generatedCodeAnnotations = config.getGeneratedCodeAnnotations();
@@ -259,13 +260,11 @@ public final class CodeAnnotationInfo {
private boolean isAnnotatedTopLevelClass(Symbol.ClassSymbol classSymbol, Config config) {
// First, check for an explicitly @NullUnmarked top level class
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
- classSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
+ if (hasDirectAnnotationWithSimpleName(classSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
return false;
}
// Then, check if the class has a @NullMarked annotation or comes from an annotated package
- if ((ASTHelpers.hasDirectAnnotationWithSimpleName(
- classSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)
+ if ((hasDirectAnnotationWithSimpleName(classSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)
|| fromAnnotatedPackage(classSymbol, config))) {
// make sure it's not explicitly configured as unannotated
return !shouldTreatAsUnannotated(classSymbol, config);
@@ -295,14 +294,12 @@ public final class CodeAnnotationInfo {
return methodNullnessCache.computeIfAbsent(
methodSymbol,
m -> {
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
- m, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
+ if (hasDirectAnnotationWithSimpleName(m, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
return false;
} else if (this.isNullnessAnnotated) {
return true;
} else {
- return ASTHelpers.hasDirectAnnotationWithSimpleName(
- m, NullabilityUtil.NULLMARKED_SIMPLE_NAME);
+ return hasDirectAnnotationWithSimpleName(m, NullabilityUtil.NULLMARKED_SIMPLE_NAME);
}
});
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java
index a0816b3..1eeb219 100644
--- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java
+++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java
@@ -22,6 +22,7 @@
package com.uber.nullaway;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
@@ -132,6 +133,13 @@ public interface LibraryModels {
ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods();
/**
+ * Get the set of library fields that may be {@code null}.
+ *
+ * @return set of library fields that may be {@code null}.
+ */
+ ImmutableSet<FieldRef> nullableFields();
+
+ /**
* Get a list of custom stream library specifications.
*
* <p>This allows users to define filter/map/other methods for APIs which behave similarly to Java
@@ -244,4 +252,17 @@ public interface LibraryModels {
+ '}';
}
}
+
+ /** Representation of a field as a qualified class name + a field name */
+ @AutoValue
+ abstract class FieldRef {
+
+ public abstract String getEnclosingClassName();
+
+ public abstract String getFieldName();
+
+ public static FieldRef fieldRef(String enclosingClass, String fieldName) {
+ return new AutoValue_LibraryModels_FieldRef(enclosingClass, fieldName);
+ }
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
index 744bddd..a23438a 100644
--- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java
+++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -28,6 +28,7 @@ import static com.sun.source.tree.Tree.Kind.IDENTIFIER;
import static com.sun.source.tree.Tree.Kind.OTHER;
import static com.sun.source.tree.Tree.Kind.PARENTHESIZED;
import static com.sun.source.tree.Tree.Kind.TYPE_CAST;
+import static com.uber.nullaway.ASTHelpersBackports.hasDirectAnnotationWithSimpleName;
import static com.uber.nullaway.ASTHelpersBackports.isStatic;
import static com.uber.nullaway.ErrorBuilder.errMsgForInitializer;
import static com.uber.nullaway.NullabilityUtil.castToNonNull;
@@ -35,6 +36,7 @@ import static com.uber.nullaway.NullabilityUtil.castToNonNull;
import com.google.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
@@ -398,16 +400,31 @@ public class NullAway extends BugChecker
if (!withinAnnotatedCode(state)) {
return Description.NO_MATCH;
}
- final Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
- if (methodSymbol == null) {
- throw new RuntimeException("not expecting unresolved method here");
- }
+ Symbol.MethodSymbol methodSymbol = getSymbolForMethodInvocation(tree, state);
handler.onMatchMethodInvocation(this, tree, state, methodSymbol);
// assuming this list does not include the receiver
List<? extends ExpressionTree> actualParams = tree.getArguments();
return handleInvocation(tree, state, methodSymbol, actualParams);
}
+ private static Symbol.MethodSymbol getSymbolForMethodInvocation(
+ MethodInvocationTree tree, VisitorState state) {
+ Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
+ Verify.verify(methodSymbol != null, "not expecting unresolved method here");
+ // For interface methods, if the method is an implicit method corresponding to a method from
+ // java.lang.Object, use the symbol for the java.lang.Object method instead. We do this to
+ // properly treat the method as unannotated, which is particularly important for equals()
+ // methods. This is an adaptation to a change in JDK 18; see
+ // https://bugs.openjdk.org/browse/JDK-8272564
+ if (methodSymbol.owner.isInterface()) {
+ Symbol.MethodSymbol baseSymbol = (Symbol.MethodSymbol) methodSymbol.baseSymbol();
+ if (baseSymbol != methodSymbol && baseSymbol.owner == state.getSymtab().objectType.tsym) {
+ methodSymbol = baseSymbol;
+ }
+ }
+ return methodSymbol;
+ }
+
@Override
public Description matchNewClass(NewClassTree tree, VisitorState state) {
if (!withinAnnotatedCode(state)) {
@@ -476,7 +493,7 @@ public class NullAway extends BugChecker
doUnboxingCheck(state, tree.getExpression());
}
// generics check
- if (lhsType != null && lhsType.getTypeArguments().length() > 0) {
+ if (lhsType != null && lhsType.getTypeArguments().length() > 0 && config.isJSpecifyMode()) {
GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state);
}
@@ -486,7 +503,8 @@ public class NullAway extends BugChecker
return Description.NO_MATCH;
}
- if (Nullness.hasNullableAnnotation(assigned, config)) {
+ if (Nullness.hasNullableAnnotation(assigned, config)
+ || handler.onOverrideFieldNullability(assigned)) {
// field already annotated
return Description.NO_MATCH;
}
@@ -577,20 +595,20 @@ public class NullAway extends BugChecker
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree);
switch (nullMarkingForTopLevelClass) {
case FULLY_MARKED:
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
+ if (hasDirectAnnotationWithSimpleName(
methodSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME)) {
nullMarkingForTopLevelClass = NullMarking.PARTIALLY_MARKED;
}
break;
case FULLY_UNMARKED:
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
+ if (hasDirectAnnotationWithSimpleName(
methodSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) {
nullMarkingForTopLevelClass = NullMarking.PARTIALLY_MARKED;
markedMethodInUnmarkedContext = true;
}
break;
case PARTIALLY_MARKED:
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(
+ if (hasDirectAnnotationWithSimpleName(
methodSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) {
// We still care here if this is a transition between @NullUnmarked and @NullMarked code,
// within partially marked code, see checks below for markedMethodInUnmarkedContext.
@@ -694,7 +712,9 @@ public class NullAway extends BugChecker
if (!withinAnnotatedCode(state)) {
return Description.NO_MATCH;
}
- GenericsChecks.checkInstantiationForParameterizedTypedTree(tree, state, this, config);
+ if (config.isJSpecifyMode()) {
+ GenericsChecks.checkInstantiationForParameterizedTypedTree(tree, state, this, config);
+ }
return Description.NO_MATCH;
}
@@ -1423,7 +1443,7 @@ public class NullAway extends BugChecker
return Description.NO_MATCH;
}
VarSymbol symbol = ASTHelpers.getSymbol(tree);
- if (tree.getInitializer() != null) {
+ if (tree.getInitializer() != null && config.isJSpecifyMode()) {
GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state);
}
@@ -1464,10 +1484,10 @@ public class NullAway extends BugChecker
*/
private boolean classAnnotationIntroducesPartialMarking(Symbol.ClassSymbol classSymbol) {
return (nullMarkingForTopLevelClass == NullMarking.FULLY_UNMARKED
- && ASTHelpers.hasDirectAnnotationWithSimpleName(
+ && hasDirectAnnotationWithSimpleName(
classSymbol, NullabilityUtil.NULLMARKED_SIMPLE_NAME))
|| (nullMarkingForTopLevelClass == NullMarking.FULLY_MARKED
- && ASTHelpers.hasDirectAnnotationWithSimpleName(
+ && hasDirectAnnotationWithSimpleName(
classSymbol, NullabilityUtil.NULLUNMARKED_SIMPLE_NAME));
}
@@ -1576,7 +1596,9 @@ public class NullAway extends BugChecker
public Description matchConditionalExpression(
ConditionalExpressionTree tree, VisitorState state) {
if (withinAnnotatedCode(state)) {
- GenericsChecks.checkTypeParameterNullnessForConditionalExpression(tree, this, state);
+ if (config.isJSpecifyMode()) {
+ GenericsChecks.checkTypeParameterNullnessForConditionalExpression(tree, this, state);
+ }
doUnboxingCheck(state, tree.getCondition());
}
return Description.NO_MATCH;
@@ -1707,8 +1729,10 @@ public class NullAway extends BugChecker
: Nullness.NONNULL);
}
}
- GenericsChecks.compareGenericTypeParameterNullabilityForCall(
- formalParams, actualParams, methodSymbol.isVarArgs(), this, state);
+ if (config.isJSpecifyMode()) {
+ GenericsChecks.compareGenericTypeParameterNullabilityForCall(
+ formalParams, actualParams, methodSymbol.isVarArgs(), this, state);
+ }
}
// Allow handlers to override the list of non-null argument positions
@@ -2233,7 +2257,7 @@ public class NullAway extends BugChecker
}
private boolean isInitializerMethod(VisitorState state, Symbol.MethodSymbol symbol) {
- if (ASTHelpers.hasDirectAnnotationWithSimpleName(symbol, "Initializer")
+ if (hasDirectAnnotationWithSimpleName(symbol, "Initializer")
|| config.isKnownInitializerMethod(symbol)) {
return true;
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
index 025c3ee..de86547 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
@@ -6,6 +6,7 @@ import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import java.util.List;
+import javax.lang.model.type.NullType;
/**
* Visitor that checks equality of nullability annotations for all nested generic type arguments
@@ -21,22 +22,25 @@ public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor<Boolean,
@Override
public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) {
+ if (rhsType instanceof NullType) {
+ return true;
+ }
Types types = state.getTypes();
// The base type of rhsType may be a subtype of lhsType's base type. In such cases, we must
// compare lhsType against the supertype of rhsType with a matching base type.
- rhsType = types.asSuper(rhsType, lhsType.tsym);
+ Type rhsTypeAsSuper = types.asSuper(rhsType, lhsType.tsym);
// This is impossible, considering the fact that standard Java subtyping succeeds before
// running NullAway
- if (rhsType == null) {
+ if (rhsTypeAsSuper == null) {
throw new RuntimeException("Did not find supertype of " + rhsType + " matching " + lhsType);
}
List<Type> lhsTypeArguments = lhsType.getTypeArguments();
- List<Type> rhsTypeArguments = rhsType.getTypeArguments();
+ List<Type> rhsTypeArguments = rhsTypeAsSuper.getTypeArguments();
// This is impossible, considering the fact that standard Java subtyping succeeds before
// running NullAway
if (lhsTypeArguments.size() != rhsTypeArguments.size()) {
throw new RuntimeException(
- "Number of types arguments in " + rhsType + " does not match " + lhsType);
+ "Number of types arguments in " + rhsTypeAsSuper + " does not match " + lhsType);
}
for (int i = 0; i < lhsTypeArguments.size(); i++) {
Type lhsTypeArgument = lhsTypeArguments.get(i);
@@ -77,6 +81,9 @@ public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor<Boolean,
@Override
public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) {
+ if (rhsType instanceof NullType) {
+ return true;
+ }
Type.ArrayType arrRhsType = (Type.ArrayType) rhsType;
return lhsType.getComponentType().accept(this, arrRhsType.getComponentType());
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
index 23affc4..4754ed9 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -3,7 +3,6 @@ package com.uber.nullaway.generics;
import static com.google.common.base.Verify.verify;
import static com.uber.nullaway.NullabilityUtil.castToNonNull;
-import com.google.common.base.Preconditions;
import com.google.errorprone.VisitorState;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
@@ -301,7 +300,7 @@ public final class GenericsChecks {
Type lhsType = getTreeType(lhsTree, state);
Type rhsType = getTreeType(rhsTree, state);
- if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) {
+ if (lhsType != null && rhsType != null) {
boolean isAssignmentValid = compareNullabilityAnnotations(lhsType, rhsType, state);
if (!isAssignmentValid) {
reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis);
@@ -333,8 +332,7 @@ public final class GenericsChecks {
return;
}
Type returnExpressionType = getTreeType(retExpr, state);
- if (formalReturnType instanceof Type.ClassType
- && returnExpressionType instanceof Type.ClassType) {
+ if (formalReturnType != null && returnExpressionType != null) {
boolean isReturnTypeValid =
compareNullabilityAnnotations(formalReturnType, returnExpressionType, state);
if (!isReturnTypeValid) {
@@ -407,14 +405,14 @@ public final class GenericsChecks {
// The condExpr type should be the least-upper bound of the true and false part types. To check
// the nullability annotations, we check that the true and false parts are assignable to the
// type of the whole expression
- if (condExprType instanceof Type.ClassType) {
- if (truePartType instanceof Type.ClassType) {
+ if (condExprType != null) {
+ if (truePartType != null) {
if (!compareNullabilityAnnotations(condExprType, truePartType, state)) {
reportMismatchedTypeForTernaryOperator(
truePartTree, condExprType, truePartType, state, analysis);
}
}
- if (falsePartType instanceof Type.ClassType) {
+ if (falsePartType != null) {
if (!compareNullabilityAnnotations(condExprType, falsePartType, state)) {
reportMismatchedTypeForTernaryOperator(
falsePartTree, condExprType, falsePartType, state, analysis);
@@ -452,8 +450,7 @@ public final class GenericsChecks {
Type formalParameter = formalParams.get(i).type;
if (!formalParameter.getTypeArguments().isEmpty()) {
Type actualParameter = getTreeType(actualParams.get(i), state);
- if (formalParameter instanceof Type.ClassType
- && actualParameter instanceof Type.ClassType) {
+ if (actualParameter != null) {
if (!compareNullabilityAnnotations(formalParameter, actualParameter, state)) {
reportInvalidParametersNullabilityError(
formalParameter, actualParameter, actualParams.get(i), state, analysis);
@@ -468,8 +465,7 @@ public final class GenericsChecks {
if (!varargsElementType.getTypeArguments().isEmpty()) {
for (int i = formalParams.size() - 1; i < actualParams.size(); i++) {
Type actualParameter = getTreeType(actualParams.get(i), state);
- if (varargsElementType instanceof Type.ClassType
- && actualParameter instanceof Type.ClassType) {
+ if (actualParameter != null) {
if (!compareNullabilityAnnotations(varargsElementType, actualParameter, state)) {
reportInvalidParametersNullabilityError(
varargsElementType, actualParameter, actualParams.get(i), state, analysis);
@@ -775,10 +771,9 @@ public final class GenericsChecks {
List<Type> overriddenMethodParameterTypes = overriddenMethodType.getParameterTypes();
// TODO handle varargs; they are not handled for now
for (int i = 0; i < methodParameters.size(); i++) {
- Type overridingMethodParameterType = ASTHelpers.getType(methodParameters.get(i));
+ Type overridingMethodParameterType = getTreeType(methodParameters.get(i), state);
Type overriddenMethodParameterType = overriddenMethodParameterTypes.get(i);
- if (overriddenMethodParameterType instanceof Type.ClassType
- && overridingMethodParameterType instanceof Type.ClassType) {
+ if (overriddenMethodParameterType != null && overridingMethodParameterType != null) {
if (!compareNullabilityAnnotations(
overriddenMethodParameterType, overridingMethodParameterType, state)) {
reportInvalidOverridingMethodParamTypeError(
@@ -805,11 +800,10 @@ public final class GenericsChecks {
private static void checkTypeParameterNullnessForOverridingMethodReturnType(
MethodTree tree, Type overriddenMethodType, NullAway analysis, VisitorState state) {
Type overriddenMethodReturnType = overriddenMethodType.getReturnType();
- Type overridingMethodReturnType = ASTHelpers.getType(tree.getReturnType());
- if (!(overriddenMethodReturnType instanceof Type.ClassType)) {
+ Type overridingMethodReturnType = getTreeType(tree.getReturnType(), state);
+ if (overriddenMethodReturnType == null || overridingMethodReturnType == null) {
return;
}
- Preconditions.checkArgument(overridingMethodReturnType instanceof Type.ClassType);
if (!compareNullabilityAnnotations(
overriddenMethodReturnType, overridingMethodReturnType, state)) {
reportInvalidOverridingMethodReturnTypeError(
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
index 2cde64f..822fcf1 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
@@ -14,6 +14,10 @@ import com.sun.source.util.SimpleTreeVisitor;
import com.sun.tools.javac.code.Attribute;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.TypeMetadata;
+import com.sun.tools.javac.util.ListBuffer;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -74,10 +78,10 @@ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor<Type, Void
new Attribute.TypeCompound(
nullableType, com.sun.tools.javac.util.List.nil(), null)))
: com.sun.tools.javac.util.List.nil();
- TypeMetadata typeMetadata =
- new TypeMetadata(new TypeMetadata.Annotations(nullableAnnotationCompound));
+ TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(nullableAnnotationCompound);
Type currentTypeArgType = curTypeArg.accept(this, null);
- Type newTypeArgType = currentTypeArgType.cloneWithMetadata(typeMetadata);
+ Type newTypeArgType =
+ TYPE_METADATA_BUILDER.cloneTypeWithMetadata(currentTypeArgType, typeMetadata);
newTypeArgs.add(newTypeArgType);
}
Type.ClassType finalType =
@@ -91,4 +95,145 @@ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor<Type, Void
protected Type defaultAction(Tree node, Void unused) {
return castToNonNull(ASTHelpers.getType(node));
}
+
+ /**
+ * Abstracts over the different APIs for building {@link TypeMetadata} objects in different JDK
+ * versions.
+ */
+ private interface TypeMetadataBuilder {
+ TypeMetadata create(com.sun.tools.javac.util.List<Attribute.TypeCompound> attrs);
+
+ Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metaData);
+ }
+
+ /**
+ * Provides implementations for methods under TypeMetadataBuilder compatible with JDK 17 and
+ * earlier versions.
+ */
+ private static class JDK17AndEarlierTypeMetadataBuilder implements TypeMetadataBuilder {
+
+ @Override
+ public TypeMetadata create(com.sun.tools.javac.util.List<Attribute.TypeCompound> attrs) {
+ return new TypeMetadata(new TypeMetadata.Annotations(attrs));
+ }
+
+ /**
+ * Clones the given type with the specified Metadata for getting the right nullability
+ * annotations.
+ *
+ * @param typeToBeCloned The Type we want to clone with the required Nullability Metadata
+ * @param metadata The required Nullability metadata which is lost from the type
+ * @return Type after it has been cloned by applying the required Nullability metadata
+ */
+ @Override
+ public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) {
+ return typeToBeCloned.cloneWithMetadata(metadata);
+ }
+ }
+
+ /**
+ * Provides implementations for methods under TypeMetadataBuilder compatible with the updates made
+ * to the library methods for Jdk 21. The implementation calls the logic specific to JDK 21
+ * indirectly using MethodHandles since we still need the code to compile on earlier versions.
+ */
+ private static class JDK21TypeMetadataBuilder implements TypeMetadataBuilder {
+
+ private static final MethodHandle typeMetadataHandle = createHandle();
+ private static final MethodHandle addMetadataHandle =
+ createVirtualMethodHandle(Type.class, TypeMetadata.class, Type.class, "addMetadata");
+ private static final MethodHandle dropMetadataHandle =
+ createVirtualMethodHandle(Type.class, Class.class, Type.class, "dropMetadata");
+
+ private static MethodHandle createHandle() {
+ MethodHandles.Lookup lookup = MethodHandles.lookup();
+ MethodType mt = MethodType.methodType(void.class, com.sun.tools.javac.util.ListBuffer.class);
+ try {
+ return lookup.findConstructor(TypeMetadata.Annotations.class, mt);
+ } catch (NoSuchMethodException e) {
+ throw new RuntimeException(e);
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Used to get a MethodHandle for a virtual method from the specified class
+ *
+ * @param retTypeClass Class to indicate the return type of the desired method
+ * @param paramTypeClass Class to indicate the parameter type of the desired method
+ * @param refClass Class within which the desired method is contained
+ * @param methodName Name of the desired method
+ * @return The appropriate MethodHandle for the virtual method
+ */
+ private static MethodHandle createVirtualMethodHandle(
+ Class<?> retTypeClass, Class<?> paramTypeClass, Class<?> refClass, String methodName) {
+ MethodHandles.Lookup lookup = MethodHandles.lookup();
+ MethodType mt = MethodType.methodType(retTypeClass, paramTypeClass);
+ try {
+ return lookup.findVirtual(refClass, methodName, mt);
+ } catch (NoSuchMethodException e) {
+ throw new RuntimeException(e);
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Override
+ public TypeMetadata create(com.sun.tools.javac.util.List<Attribute.TypeCompound> attrs) {
+ ListBuffer<Attribute.TypeCompound> b = new ListBuffer<>();
+ b.appendList(attrs);
+ try {
+ return (TypeMetadata) typeMetadataHandle.invoke(b);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Calls dropMetadata and addMetadata using MethodHandles for JDK 21, which removed the previous
+ * cloneWithMetadata method.
+ *
+ * @param typeToBeCloned The Type we want to clone with the required Nullability metadata
+ * @param metadata The required Nullability metadata
+ * @return Cloned Type with the necessary Nullability metadata
+ */
+ @Override
+ public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) {
+ try {
+ // In JDK 21 addMetadata works if there is no metadata associated with the type, so we
+ // create a copy without the existing metadata first and then add it
+ Type clonedTypeWithoutMetadata =
+ (Type) dropMetadataHandle.invoke(typeToBeCloned, metadata.getClass());
+ return (Type) addMetadataHandle.invoke(clonedTypeWithoutMetadata, metadata);
+ } catch (Throwable e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /** The TypeMetadataBuilder to be used for the current JDK version. */
+ private static final TypeMetadataBuilder TYPE_METADATA_BUILDER =
+ getVersion() >= 21
+ ? new JDK21TypeMetadataBuilder()
+ : new JDK17AndEarlierTypeMetadataBuilder();
+
+ /**
+ * Utility method to get the current JDK version, that works on Java 8 and above.
+ *
+ * <p>TODO remove this method once we drop support for Java 8
+ *
+ * @return the current JDK version
+ */
+ private static int getVersion() {
+ String version = System.getProperty("java.version");
+ if (version.startsWith("1.")) {
+ version = version.substring(2, 3);
+ } else {
+ int dot = version.indexOf(".");
+ if (dot != -1) {
+ version = version.substring(0, dot);
+ }
+ }
+ return Integer.parseInt(version);
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java
index 2a24923..676d3b4 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java
@@ -30,6 +30,7 @@ import com.sun.tools.javac.code.Symbol;
import com.uber.nullaway.dataflow.AccessPath;
import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
import java.util.List;
+import javax.annotation.Nullable;
import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.nullaway.dataflow.cfg.node.Node;
@@ -60,17 +61,9 @@ public class AssertionHandler extends BaseNoOpHandler {
// assertThat(A).isInstanceOf(Foo.class)
// A will not be NULL after this statement.
if (methodNameUtil.isMethodIsNotNull(callee) || methodNameUtil.isMethodIsInstanceOf(callee)) {
- Node receiver = node.getTarget().getReceiver();
- if (receiver instanceof MethodInvocationNode) {
- MethodInvocationNode receiver_method = (MethodInvocationNode) receiver;
- Symbol.MethodSymbol receiver_symbol = ASTHelpers.getSymbol(receiver_method.getTree());
- if (methodNameUtil.isMethodAssertThat(receiver_symbol)) {
- Node arg = receiver_method.getArgument(0);
- AccessPath ap = AccessPath.getAccessPathForNode(arg, state, apContext);
- if (ap != null) {
- bothUpdates.set(ap, NONNULL);
- }
- }
+ AccessPath ap = getAccessPathForNotNullAssertThatExpr(node, state, apContext);
+ if (ap != null) {
+ bothUpdates.set(ap, NONNULL);
}
}
@@ -94,4 +87,31 @@ public class AssertionHandler extends BaseNoOpHandler {
return NullnessHint.UNKNOWN;
}
+
+ /**
+ * Returns the AccessPath for the argument of an assertThat() call, if present as a valid nested
+ * receiver expression of a method invocation
+ *
+ * @param node the method invocation node
+ * @param state the visitor state
+ * @param apContext the access path context
+ * @return the AccessPath for the argument of the assertThat() call, if present, otherwise {@code
+ * null}
+ */
+ private @Nullable AccessPath getAccessPathForNotNullAssertThatExpr(
+ MethodInvocationNode node, VisitorState state, AccessPath.AccessPathContext apContext) {
+ Node receiver = node.getTarget().getReceiver();
+ if (receiver instanceof MethodInvocationNode) {
+ MethodInvocationNode receiver_method = (MethodInvocationNode) receiver;
+ Symbol.MethodSymbol receiver_symbol = ASTHelpers.getSymbol(receiver_method.getTree());
+ if (methodNameUtil.isMethodAssertThat(receiver_symbol)) {
+ Node arg = receiver_method.getArgument(0);
+ return AccessPath.getAccessPathForNode(arg, state, apContext);
+ } else if (methodNameUtil.isMethodAssertJDescribedAs(receiver_symbol)) {
+ // For calls to as() or describedAs(), we recursively search for the assertThat() call
+ return getAccessPathForNotNullAssertThatExpr(receiver_method, state, apContext);
+ }
+ }
+ return null;
+ }
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java
index 242a96a..815b46d 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java
@@ -119,6 +119,12 @@ public abstract class BaseNoOpHandler implements Handler {
}
@Override
+ public boolean onOverrideFieldNullability(Symbol field) {
+ // NoOp
+ return false;
+ }
+
+ @Override
public Nullness[] onOverrideMethodInvocationParametersNullability(
Context context,
Symbol.MethodSymbol methodSymbol,
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java
index 31617da..2001269 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java
@@ -136,6 +136,18 @@ class CompositeHandler implements Handler {
}
@Override
+ public boolean onOverrideFieldNullability(Symbol field) {
+ for (Handler h : handlers) {
+ if (h.onOverrideFieldNullability(field)) {
+ // If any handler determines that the field is @Nullable, we should acknowledge that and
+ // treat it as such.
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
public Nullness[] onOverrideMethodInvocationParametersNullability(
Context context,
Symbol.MethodSymbol methodSymbol,
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java
index 835c01f..b2018ee 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java
@@ -174,6 +174,16 @@ public interface Handler {
Nullness returnNullness);
/**
+ * Called to potentially override the nullability of a field which is not annotated as @Nullable.
+ * If the field is decided to be @Nullable by this handler, the field should be treated
+ * as @Nullable anyway.
+ *
+ * @param field The symbol for the field in question.
+ * @return true if the field should be treated as @Nullable, false otherwise.
+ */
+ boolean onOverrideFieldNullability(Symbol field);
+
+ /**
* Called after the analysis determines the nullability of a method's arguments, allowing handlers
* to override.
*
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
index e000c4f..fe87278 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
@@ -22,6 +22,7 @@
package com.uber.nullaway.handlers;
+import static com.uber.nullaway.LibraryModels.FieldRef.fieldRef;
import static com.uber.nullaway.LibraryModels.MethodRef.methodRef;
import static com.uber.nullaway.Nullness.NONNULL;
import static com.uber.nullaway.Nullness.NULLABLE;
@@ -57,6 +58,7 @@ import java.util.ServiceLoader;
import java.util.Set;
import java.util.function.Function;
import javax.annotation.Nullable;
+import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode;
import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.nullaway.dataflow.cfg.node.Node;
@@ -81,6 +83,25 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
}
@Override
+ public boolean onOverrideFieldNullability(Symbol field) {
+ return isNullableFieldInLibraryModels(field);
+ }
+
+ @Override
+ public NullnessHint onDataflowVisitFieldAccess(
+ FieldAccessNode node,
+ Symbol symbol,
+ Types types,
+ Context context,
+ AccessPath.AccessPathContext apContext,
+ AccessPathNullnessPropagation.SubNodeValues inputs,
+ AccessPathNullnessPropagation.Updates updates) {
+ return isNullableFieldInLibraryModels(symbol)
+ ? NullnessHint.HINT_NULLABLE
+ : NullnessHint.UNKNOWN;
+ }
+
+ @Override
public Nullness[] onOverrideMethodInvocationParametersNullability(
Context context,
Symbol.MethodSymbol methodSymbol,
@@ -132,6 +153,9 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
+ if (isNullableFieldInLibraryModels(exprSymbol)) {
+ return true;
+ }
if (!(expr.getKind() == Tree.Kind.METHOD_INVOCATION
&& exprSymbol instanceof Symbol.MethodSymbol)) {
return exprMayBeNull;
@@ -233,6 +257,32 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
}
}
+ /**
+ * Check if the given symbol is a field that is marked as nullable in any of our library models.
+ *
+ * @param symbol The symbol to check.
+ * @return True if the symbol is a field that is marked as nullable in any of our library models.
+ */
+ private boolean isNullableFieldInLibraryModels(@Nullable Symbol symbol) {
+ if (libraryModels.nullableFields().isEmpty()) {
+ // no need to do any work if there are no nullable fields.
+ return false;
+ }
+ if (symbol instanceof Symbol.VarSymbol && symbol.getKind().isField()) {
+ Symbol.VarSymbol varSymbol = (Symbol.VarSymbol) symbol;
+ Symbol.ClassSymbol classSymbol = varSymbol.enclClass();
+ if (classSymbol == null) {
+ // e.g. .class expressions
+ return false;
+ }
+ String fieldName = varSymbol.getSimpleName().toString();
+ String enclosingClassName = classSymbol.flatName().toString();
+ // This check could be optimized further in the future if needed
+ return libraryModels.nullableFields().contains(fieldRef(enclosingClassName, fieldName));
+ }
+ return false;
+ }
+
private void setConditionalArgumentNullness(
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
@@ -824,6 +874,12 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
public ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods() {
return CAST_TO_NONNULL_METHODS;
}
+
+ @Override
+ public ImmutableSet<FieldRef> nullableFields() {
+ // No nullable fields by default.
+ return ImmutableSet.of();
+ }
}
private static class CombinedLibraryModels implements LibraryModels {
@@ -846,6 +902,8 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
private final ImmutableSet<MethodRef> nonNullReturns;
+ private final ImmutableSet<FieldRef> nullableFields;
+
private final ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods;
private final ImmutableList<StreamTypeRecord> customStreamNullabilitySpecs;
@@ -870,6 +928,7 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
new ImmutableSetMultimap.Builder<>();
ImmutableList.Builder<StreamTypeRecord> customStreamNullabilitySpecsBuilder =
new ImmutableList.Builder<>();
+ ImmutableSet.Builder<FieldRef> nullableFieldsBuilder = new ImmutableSet.Builder<>();
for (LibraryModels libraryModels : models) {
for (Map.Entry<MethodRef, Integer> entry : libraryModels.failIfNullParameters().entries()) {
if (shouldSkipModel(entry.getKey())) {
@@ -932,6 +991,9 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
for (StreamTypeRecord streamTypeRecord : libraryModels.customStreamNullabilitySpecs()) {
customStreamNullabilitySpecsBuilder.add(streamTypeRecord);
}
+ for (FieldRef fieldRef : libraryModels.nullableFields()) {
+ nullableFieldsBuilder.add(fieldRef);
+ }
}
failIfNullParameters = failIfNullParametersBuilder.build();
explicitlyNullableParameters = explicitlyNullableParametersBuilder.build();
@@ -943,6 +1005,7 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
nonNullReturns = nonNullReturnsBuilder.build();
castToNonNullMethods = castToNonNullMethodsBuilder.build();
customStreamNullabilitySpecs = customStreamNullabilitySpecsBuilder.build();
+ nullableFields = nullableFieldsBuilder.build();
}
private boolean shouldSkipModel(MethodRef key) {
@@ -990,6 +1053,11 @@ public class LibraryModelsHandler extends BaseNoOpHandler {
}
@Override
+ public ImmutableSet<FieldRef> nullableFields() {
+ return nullableFields;
+ }
+
+ @Override
public ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods() {
return castToNonNullMethods;
}
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java b/nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java
index 51e9cd9..1a276bb 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/MethodNameUtil.java
@@ -56,6 +56,9 @@ class MethodNameUtil {
private static final String IS_PRESENT_OWNER_ASSERTJ =
"org.assertj.core.api.AbstractOptionalAssert";
private static final String ASSERT_THAT_METHOD = "assertThat";
+ private static final String AS_METHOD = "as";
+ private static final String DESCRIBED_AS_METHOD = "describedAs";
+
private static final String ASSERT_THAT_OWNER_TRUTH = "com.google.common.truth.Truth";
private static final String ASSERT_THAT_OWNER_ASSERTJ = "org.assertj.core.api.Assertions";
@@ -101,6 +104,9 @@ class MethodNameUtil {
private Name assertThatOwnerTruth;
private Name assertThatOwnerAssertJ;
+ private Name as;
+ private Name describedAs;
+
// Names for junit assertion libraries.
private Name hamcrestAssertClass;
private Name junitAssertClass;
@@ -141,6 +147,9 @@ class MethodNameUtil {
assertThatOwnerTruth = table.fromString(ASSERT_THAT_OWNER_TRUTH);
assertThatOwnerAssertJ = table.fromString(ASSERT_THAT_OWNER_ASSERTJ);
+ as = table.fromString(AS_METHOD);
+ describedAs = table.fromString(DESCRIBED_AS_METHOD);
+
isPresent = table.fromString(IS_PRESENT_METHOD);
isNotEmpty = table.fromString(IS_NOT_EMPTY_METHOD);
isPresentOwnerAssertJ = table.fromString(IS_PRESENT_OWNER_ASSERTJ);
@@ -211,6 +220,18 @@ class MethodNameUtil {
|| matchesMethod(methodSymbol, assertThat, assertThatOwnerAssertJ);
}
+ /**
+ * Returns true if the method is describedAs() or as() from AssertJ. Note that this implementation
+ * does not check the ower, as there are many possible implementations. This method should only be
+ * used in a caller content where it is clear that the operation is related to use of AssertJ.
+ *
+ * @param methodSymbol symbol for the method
+ * @return {@code true} iff the method is describedAs() or as() from AssertJ
+ */
+ public boolean isMethodAssertJDescribedAs(Symbol.MethodSymbol methodSymbol) {
+ return methodSymbol.name.equals(as) || methodSymbol.name.equals(describedAs);
+ }
+
boolean isMethodHamcrestAssertThat(Symbol.MethodSymbol methodSymbol) {
return matchesMethod(methodSymbol, assertThat, hamcrestAssertClass);
}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java
index 3a6838c..2967422 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java
@@ -366,6 +366,71 @@ public class NullAwayAssertionLibsTests extends NullAwayTestsBase {
}
@Test
+ public void supportAssertJAssertThatIsNotNullWithDescription_Object() {
+ makeTestHelperWithArgs(
+ Arrays.asList(
+ "-d",
+ temporaryFolder.getRoot().getAbsolutePath(),
+ "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+ "-XepOpt:NullAway:HandleTestAssertionLibraries=true"))
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import java.lang.Object;",
+ "import java.util.Objects;",
+ "import javax.annotation.Nullable;",
+ "import static org.assertj.core.api.Assertions.assertThat;",
+ "class Test {",
+ " private void foo(@Nullable Object o) {",
+ " assertThat(o).as(\"test\").isNotNull();",
+ " o.toString();",
+ " }",
+ " private void foo2(@Nullable Object o) {",
+ " assertThat(o).describedAs(\"test\").isNotNull();",
+ " o.toString();",
+ " }",
+ " private void foo3(@Nullable Object o) {",
+ " assertThat(o).describedAs(\"test1\").as(\"test2\").isNotNull();",
+ " o.toString();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void assertJAssertThatIsNotNullUnhandled() {
+ makeTestHelperWithArgs(
+ Arrays.asList(
+ "-d",
+ temporaryFolder.getRoot().getAbsolutePath(),
+ "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+ "-XepOpt:NullAway:HandleTestAssertionLibraries=true"))
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import java.lang.Object;",
+ "import java.util.Objects;",
+ "import javax.annotation.Nullable;",
+ "import static org.assertj.core.api.Assertions.assertThat;",
+ "class Test {",
+ " private void foo(@Nullable Object o) {",
+ " org.assertj.core.api.ObjectAssert t = assertThat(o);",
+ " t.isNotNull();",
+ " // False positive",
+ " // BUG: Diagnostic contains: dereferenced expression",
+ " o.toString();",
+ " }",
+ " private void foo2(@Nullable Object o) {",
+ " assertThat(o).isEqualToIgnoringNullFields(o).describedAs(\"test\").isNotNull();",
+ " // False positive",
+ " // BUG: Diagnostic contains: dereferenced expression",
+ " o.toString();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
public void supportAssertJAssertThatIsNotNull_String() {
makeTestHelperWithArgs(
Arrays.asList(
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java
index f8f3ed6..3649de9 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java
@@ -960,4 +960,45 @@ public class NullAwayCoreTests extends NullAwayTestsBase {
"}")
.doTest();
}
+
+ /**
+ * This test exposes a failure in CFG construction in Checker Framework 3.41.0 and above. Once a
+ * fix for this issue makes it to a Checker Framework release, we can probably remove this test.
+ * See https://github.com/typetools/checker-framework/issues/6396.
+ */
+ @Test
+ public void cfgConstructionSymbolCompletionFailure() {
+ defaultCompilationHelper
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.apache.spark.sql.SparkSession;",
+ "class Test {",
+ " static class X {",
+ " X(SparkSession session) {}",
+ " }",
+ " X run() {",
+ " try (SparkSession session = SparkSession.builder().getOrCreate()) {",
+ " return new X(session);",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void testDefaultEqualsInInterfaceTakesNullable() {
+ defaultCompilationHelper
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import javax.annotation.Nullable;",
+ "class Test {",
+ " public interface AnInterface {}",
+ " public static boolean foo(AnInterface a, @Nullable AnInterface b) {",
+ " return a.equals(b);",
+ " }",
+ "}")
+ .doTest();
+ }
}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java
index 937752c..098b98b 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java
@@ -227,4 +227,41 @@ public class NullAwayCustomLibraryModelsTests extends NullAwayTestsBase {
"}")
.doTest();
}
+
+ @Test
+ public void libraryModelsAndOverridingFieldNullability() {
+ makeLibraryModelsTestHelperWithArgs(
+ Arrays.asList(
+ "-d",
+ temporaryFolder.getRoot().getAbsolutePath(),
+ "-XepOpt:NullAway:AnnotatedPackages=com.uber"))
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import com.uber.lib.unannotated.UnannotatedWithModels;",
+ "public class Test {",
+ " UnannotatedWithModels uwm = new UnannotatedWithModels();",
+ " Object nonnullField = new Object();",
+ " void assignNullableFromLibraryModelField() {",
+ " // BUG: Diagnostic contains: assigning @Nullable",
+ " this.nonnullField = uwm.nullableFieldUnannotated1;",
+ " // BUG: Diagnostic contains: assigning @Nullable",
+ " this.nonnullField = uwm.nullableFieldUnannotated2;",
+ " }",
+ " void flowTest() {",
+ " if(uwm.nullableFieldUnannotated1 != null) {",
+ " // no error here, to check that library models only initialize flow store",
+ " this.nonnullField = uwm.nullableFieldUnannotated1;",
+ " }",
+ " }",
+ " String dereferenceTest() {",
+ " // BUG: Diagnostic contains: dereferenced expression uwm.nullableFieldUnannotated1 is @Nullable",
+ " return uwm.nullableFieldUnannotated1.toString();",
+ " }",
+ " void assignmentTest() {",
+ " uwm.nullableFieldUnannotated1 = null;",
+ " }",
+ "}")
+ .doTest();
+ }
}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
index 68f1d11..d146442 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -1021,6 +1021,23 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
}
@Test
+ public void nestedGenericTypeAssignment2() {
+ makeHelper()
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.jspecify.annotations.Nullable;",
+ "class Test {",
+ " static class A<T extends @Nullable Object> { }",
+ " static void testPositive() {",
+ " // BUG: Diagnostic contains: Cannot assign from type",
+ " A<A<String>[]> var2 = new A<A<@Nullable String>[]>();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
public void genericPrimitiveArrayTypeAssignment() {
makeHelper()
.addSourceLines(
@@ -1560,6 +1577,69 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
.doTest();
}
+ @Test
+ public void testForNullRhsTypeWhenReturnedForGenericType() {
+ makeHelper()
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.jspecify.annotations.Nullable;",
+ "class Test {",
+ " static class A<T extends @Nullable Object> { }",
+ " static A<String> testPositive() {",
+ " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type",
+ " return null;",
+ " }",
+ " static @Nullable A<String> testNegative() {",
+ " return null;",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void testForNullTypeRhsTypeForArrayType() {
+ makeHelper()
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.jspecify.annotations.Nullable;",
+ "import java.util.List;",
+ "import java.util.ArrayList;",
+ "class Test {",
+ " static void testNegative() {",
+ " List<String> a = new ArrayList<String>();",
+ " Object[] o = a != null ? a.toArray() : null;",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void overrideWithRawType() {
+ makeHelper()
+ .addSourceLines(
+ "Test.java",
+ "package com.uber;",
+ "import org.jspecify.annotations.Nullable;",
+ "class Test {",
+ " interface Foo<T> {}",
+ " interface Bar<T> {",
+ " void add(Foo<T> foo);",
+ " @Nullable Foo<T> get();",
+ " }",
+ " static class Baz<T> implements Bar<T> {",
+ " @SuppressWarnings(\"rawtypes\")",
+ " @Override",
+ " public void add(Foo foo) {}",
+ " @SuppressWarnings(\"rawtypes\")",
+ " @Override",
+ " public @Nullable Foo get() { return null; }",
+ " }",
+ "}")
+ .doTest();
+ }
+
private CompilationTestHelper makeHelper() {
return makeTestHelperWithArgs(
Arrays.asList(
diff --git a/sample-app/build.gradle b/sample-app/build.gradle
index 6c4deb2..0ab1e6f 100644
--- a/sample-app/build.gradle
+++ b/sample-app/build.gradle
@@ -45,27 +45,30 @@ android {
variants.addAll(getUnitTestVariants())
variants.configureEach { variant ->
variant.getJavaCompileProvider().configure {
- options.errorprone {
- check("NullAway", CheckSeverity.ERROR)
- option("NullAway:AnnotatedPackages", "com.uber")
- }
+ options.compilerArgs += [
+ "-XDcompilePolicy=simple",
+ "-Xplugin:ErrorProne -XepOpt:NullAway:AnnotatedPackages=com.uber",
+ ]
+ options.fork = true
+ options.forkOptions.jvmArgs = [
+ "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+ "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+ "--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+ "--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED"
+ ]
}
}
-
- // If you want to disable NullAway in just tests, you can do the below
- // DomainObjectSet<BaseVariant> testVariants = getTestVariants()
- // testVariants.addAll(getUnitTestVariants())
- // testVariants.configureEach { variant ->
- // variant.getJavaCompileProvider().configure {
- // options.errorprone {
- // check("NullAway", CheckSeverity.OFF)
- // }
- // }
- // }
}
dependencies {
implementation deps.support.appcompat
+ annotationProcessor deps.build.errorProneCore
annotationProcessor project(":nullaway")
annotationProcessor project(path: ":sample-library-model")
diff --git a/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java b/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java
index a1aa99d..5c17aca 100644
--- a/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java
+++ b/sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java
@@ -71,4 +71,9 @@ public class ExampleLibraryModels implements LibraryModels {
public ImmutableSetMultimap<MethodRef, Integer> castToNonNullMethods() {
return ImmutableSetMultimap.of();
}
+
+ @Override
+ public ImmutableSet<FieldRef> nullableFields() {
+ return ImmutableSet.of();
+ }
}
diff --git a/test-java-lib/src/main/java/com/uber/lib/unannotated/UnannotatedWithModels.java b/test-java-lib/src/main/java/com/uber/lib/unannotated/UnannotatedWithModels.java
index 412042d..a978333 100644
--- a/test-java-lib/src/main/java/com/uber/lib/unannotated/UnannotatedWithModels.java
+++ b/test-java-lib/src/main/java/com/uber/lib/unannotated/UnannotatedWithModels.java
@@ -2,6 +2,10 @@ package com.uber.lib.unannotated;
public class UnannotatedWithModels {
+ public Object nullableFieldUnannotated1;
+
+ public Object nullableFieldUnannotated2;
+
public Object returnsNullUnannotated() {
return null;
}
diff --git a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
index c94dfef..01304a9 100644
--- a/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
+++ b/test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
@@ -15,6 +15,7 @@
*/
package com.uber.nullaway.testlibrarymodels;
+import static com.uber.nullaway.LibraryModels.FieldRef.fieldRef;
import static com.uber.nullaway.LibraryModels.MethodRef.methodRef;
import com.google.auto.service.AutoService;
@@ -122,4 +123,13 @@ public class TestLibraryModels implements LibraryModels {
.withPassthroughMethodFromSignature("distinct()")
.end();
}
+
+ @Override
+ public ImmutableSet<FieldRef> nullableFields() {
+ return ImmutableSet.<FieldRef>builder()
+ .add(
+ fieldRef("com.uber.lib.unannotated.UnannotatedWithModels", "nullableFieldUnannotated1"),
+ fieldRef("com.uber.lib.unannotated.UnannotatedWithModels", "nullableFieldUnannotated2"))
+ .build();
+ }
}