diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 2473f17ba3..ee76b9b8ac 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -15,22 +15,22 @@ jobs: include: - os: ubuntu-latest java: 11 - epVersion: 2.10.0 + epVersion: 2.14.0 - os: ubuntu-latest java: 17 - epVersion: 2.10.0 + epVersion: 2.14.0 - os: macos-latest java: 11 - epVersion: 2.26.1 + epVersion: 2.27.1 - os: ubuntu-latest java: 11 - epVersion: 2.26.1 + epVersion: 2.27.1 - os: windows-latest java: 11 - epVersion: 2.26.1 + epVersion: 2.27.1 - os: ubuntu-latest java: 17 - epVersion: 2.26.1 + epVersion: 2.27.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.26.1' && github.repository == 'uber/NullAway' + if: runner.os == 'Linux' && matrix.java == '11' && matrix.epVersion == '2.27.1' && github.repository == 'uber/NullAway' - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 with: diff --git a/.github/workflows/jmh-benchmark.yml b/.github/workflows/jmh-benchmark.yml deleted file mode 100644 index aed98872d7..0000000000 --- a/.github/workflows/jmh-benchmark.yml +++ /dev/null @@ -1,73 +0,0 @@ -# This GitHub Actions workflow runs JMH benchmarks when a new comment is created on a pull request -name: Run JMH Benchmarks for Pull Request - -on: - issue_comment: # This workflow triggers when a comment is created - types: [created] - -# Only allow one instance of JMH benchmarking to be running at any given time -concurrency: all - -jobs: - benchmarking: - # Only run this job if a comment on a pull request contains '/benchmark' and is a PR on the uber/NullAway repository - if: github.event.issue.pull_request && contains(github.event.comment.body, '/benchmark') && github.repository == 'uber/NullAway' - runs-on: ubuntu-latest - permissions: write-all - - steps: - - name: Add reaction - uses: peter-evans/create-or-update-comment@v3 - with: - comment-id: ${{ github.event.comment.id }} - reactions: '+1' - - - name: Checkout repository - uses: actions/checkout@v3 - - - name: Set branch name - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - chmod +x ./.github/workflows/get_repo_details.sh - ./.github/workflows/get_repo_details.sh "${{ secrets.GITHUB_TOKEN }}" "${{ github.event.issue.number }}" "${{ github.repository }}" - - - id: 'auth' - name: Authenticating - uses: 'google-github-actions/auth@v1' - with: - credentials_json: '${{ secrets.GCP_SA_KEY_1 }}' - - - name: Set up Google Cloud SDK - uses: google-github-actions/setup-gcloud@v1 - - - name: Start VM - run: gcloud compute instances start nullway-jmh --zone=us-central1-a - - - name: Run benchmarks - run: | - chmod +x ./.github/workflows/run_gcp_benchmarks.sh - ./.github/workflows/run_gcp_benchmarks.sh - - - name: Cleanup - # Delete the branch directory on the Google Cloud instance - if: always() - run: | - ./.github/workflows/gcloud_ssh.sh " export BRANCH_NAME=${BRANCH_NAME} && rm -r -f $BRANCH_NAME" - - - name: Formatting Benchmark # Create a text file containing the benchmark results - run: | - (echo 'Main Branch:'; echo '```' ; cat main_text.txt; echo '```'; echo 'With This PR:'; echo '```' ; cat pr_text.txt; echo '```') > benchmark.txt - - - name: Comment Benchmark - uses: mshick/add-pr-comment@v2 - if: always() # This step is for adding the comment - with: - message-path: benchmark.txt # The path to the message file to leave as a comment - message-id: benchmark - - name: Stop VM - if: always() - run: gcloud compute instances stop nullway-jmh --zone=us-central1-a - - - diff --git a/CHANGELOG.md b/CHANGELOG.md index 06235efa7c..bb9c847ffb 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ Changelog ========= +Version 0.11.0 +--------------- +IMPORTANT: Support for JDK 8 is dropped and NullAway now requires 2.14.0 or higher. + +* Delete OptionalEmptinessHandler method that is no longer needed (#954) +* Refactor PreservedAnnotationTreeVisitor (#955) +* Update to Error Prone 2.27.1 (#957) +* JSpecify subtyping checks for arrays (#956) +* Bump to Checker Framework 3.43.0 (#959) +* Drop Java 8 support (#961) + +Version 0.10.26 +--------------- +* External Library Models Integration (#922) +* Rename test classes (#951) +* Propagate more nullability info to lambdas known to be invoked synchronously (#952) + Version 0.10.25 --------------- * JSpecify: Handle @nullable assignments to @nonnull arrays (#929) diff --git a/README.md b/README.md index 87497a6528..09690f71a7 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ NullAway is *fast*. It is built as a plugin to [Error Prone](http://errorprone. ### Overview -NullAway requires that you build your code with [Error Prone](http://errorprone.info), version 2.10.0 or higher. See the [Error Prone documentation](http://errorprone.info/docs/installation) for instructions on getting started with Error Prone and integration with your build system. The instructions below assume you are using Gradle; see [the docs](https://github.com/uber/NullAway/wiki/Configuration#other-build-systems) for discussion of other build systems. +NullAway requires that you build your code with [Error Prone](http://errorprone.info), version 2.14.0 or higher. See the [Error Prone documentation](http://errorprone.info/docs/installation) for instructions on getting started with Error Prone and integration with your build system. The instructions below assume you are using Gradle; see [the docs](https://github.com/uber/NullAway/wiki/Configuration#other-build-systems) for discussion of other build systems. ### Gradle diff --git a/build.gradle b/build.gradle index 5bf4aebf21..8a5760f562 100644 --- a/build.gradle +++ b/build.gradle @@ -79,11 +79,11 @@ subprojects { project -> } } - // Target JDK 8. We need to use the older sourceCompatibility / targetCompatibility settings to get - // the build to work on JDK 11+. Once we stop supporting JDK 8, switch to using the javac "release" option + // We need to use the older sourceCompatibility / targetCompatibility settings, rather than the newer "release" + // option, since we use internal javac APIs, which "release" doesn't allow tasks.withType(JavaCompile) { - java.sourceCompatibility = "1.8" - java.targetCompatibility = "1.8" + java.sourceCompatibility = JavaVersion.VERSION_11 + java.targetCompatibility = JavaVersion.VERSION_11 } tasks.withType(Test).configureEach { diff --git a/gradle.properties b/gradle.properties index 429211e021..c3fadbc216 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.26-SNAPSHOT +VERSION_NAME=0.11.1-SNAPSHOT POM_DESCRIPTION=A fast annotation-based null checker for Java diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 596096adc8..1acb6179ed 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -17,9 +17,9 @@ import org.gradle.util.VersionNumber */ // The oldest version of Error Prone that we support running on -def oldestErrorProneVersion = "2.10.0" +def oldestErrorProneVersion = "2.14.0" // Latest released Error Prone version that we've tested with -def latestErrorProneVersion = "2.26.1" +def latestErrorProneVersion = "2.27.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.40.0", + checkerFramework : "3.43.0", // for comparisons in other parts of the build errorProneLatest : latestErrorProneVersion, // The version of Error Prone used to check NullAway's code. diff --git a/guava-recent-unit-tests/build.gradle b/guava-recent-unit-tests/build.gradle index 7740123bba..b8a0662643 100644 --- a/guava-recent-unit-tests/build.gradle +++ b/guava-recent-unit-tests/build.gradle @@ -18,6 +18,11 @@ plugins { id 'nullaway.java-test-conventions' } +configurations { + // A configuration holding the jars for the oldest supported version of Error Prone, to use with tests + errorProneOldest +} + // We need this separate build target to test newer versions of Guava // (e.g. 31+) than that which NullAway currently depends on. @@ -29,29 +34,48 @@ dependencies { } testImplementation deps.build.jsr305Annotations testImplementation "com.google.guava:guava:31.1-jre" -} -// Create a task to test on JDK 8 -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" + errorProneOldest deps.build.errorProneCheckApiOld + errorProneOldest(deps.build.errorProneTestHelpersOld) { + exclude group: "junit", module: "junit" } +} + +// Create a task to test with the oldest supported version of Error Prone +// (while still building against the latest supported version) +def epOldestTest = tasks.register("testErrorProneOldest", Test) { javaLauncher = javaToolchains.launcherFor { - languageVersion = JavaLanguageVersion.of(8) + languageVersion = JavaLanguageVersion.of(11) } - description = "Runs the test suite on JDK 8" + description = "Runs the test suite using the oldest supported version of Error Prone" group = LifecycleBasePlugin.VERIFICATION_GROUP // 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}" + + 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", + // Accessed by Lombok tests + "--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED", + ] } tasks.named('check').configure { - dependsOn(jdk8Test) + dependsOn(epOldestTest) } diff --git a/jar-infer/jar-infer-cli/build.gradle b/jar-infer/jar-infer-cli/build.gradle index 9dafc4dd54..b665bee2ce 100644 --- a/jar-infer/jar-infer-cli/build.gradle +++ b/jar-infer/jar-infer-cli/build.gradle @@ -4,12 +4,6 @@ plugins { id "com.github.johnrengelman.shadow" } -// JarInfer requires JDK 11+, due to its dependence on WALA -tasks.withType(JavaCompile) { - java.sourceCompatibility = JavaVersion.VERSION_11 - java.targetCompatibility = JavaVersion.VERSION_11 -} - repositories { mavenCentral() } diff --git a/jar-infer/jar-infer-lib/build.gradle b/jar-infer/jar-infer-lib/build.gradle index c0f6c348a7..fa1d560420 100644 --- a/jar-infer/jar-infer-lib/build.gradle +++ b/jar-infer/jar-infer-lib/build.gradle @@ -18,12 +18,6 @@ plugins { id 'nullaway.java-test-conventions' } -// JarInfer requires JDK 11+, due to its dependence on WALA -tasks.withType(JavaCompile) { - java.sourceCompatibility = JavaVersion.VERSION_11 - java.targetCompatibility = JavaVersion.VERSION_11 -} - repositories { mavenCentral() // uncomment if you want to use wala.dalvik or wala.scandroid diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/InstanceOfBindingTests.java similarity index 96% rename from jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/InstanceOfBindingTests.java index 38079e5353..f80a08953b 100644 --- a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/InstanceOfBindingTests.java @@ -8,7 +8,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; -public class NullAwayInstanceOfBindingTests { +public class InstanceOfBindingTests { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java similarity index 97% rename from jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java index 64f0966369..a8dcbc9449 100644 --- a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java @@ -8,7 +8,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; -public class NullAwayModuleInfoTests { +public class ModuleInfoTests { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/OptionalEmptyTests.java similarity index 99% rename from jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/OptionalEmptyTests.java index 2e2ceee802..05f31de20b 100644 --- a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/OptionalEmptyTests.java @@ -10,7 +10,7 @@ import org.junit.rules.TemporaryFolder; /** Tests for support of the {@code Optional.isEmpty()} API. This API was introduced in JDK 11. */ -public class NullAwayOptionalEmptyTests { +public class OptionalEmptyTests { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/RecordTests.java similarity index 99% rename from jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/RecordTests.java index 69047bac82..4e83490177 100644 --- a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/RecordTests.java @@ -8,7 +8,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; -public class NullAwayRecordTests { +public class RecordTests { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java similarity index 99% rename from jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java index ab5ef6a1f6..f59bc3d6ac 100644 --- a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java @@ -30,7 +30,7 @@ import org.junit.rules.TemporaryFolder; /** NullAway unit tests involving language features available on JDK 17 but not JDK 11. */ -public class NullAwaySwitchTests { +public class SwitchTests { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/nullaway/build.gradle b/nullaway/build.gradle index 15327aed88..c2b56fd2d1 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -82,35 +82,31 @@ javadoc { apply plugin: 'com.vanniktech.maven.publish' // These --add-exports arguments are required when targeting JDK 11+ since Error Prone and NullAway access a bunch of -// JDK-internal APIs that are not exposed otherwise. Since we currently target JDK 8, we do not need to pass the -// arguments, as encapsulation of JDK internals is not enforced on JDK 8. In fact, the arguments cause a compiler error -// when targeting JDK 8. Leaving commented so we can easily add them back once we target JDK 11. -// tasks.withType(JavaCompile).configureEach { -// options.compilerArgs += [ -// "--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.code=ALL-UNNAMED", -// "--add-exports=jdk.compiler/com.sun.tools.javac.comp=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-exports=jdk.compiler/com.sun.source.tree=ALL-UNNAMED", -// ] -// } - -// 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) { +// JDK-internal APIs that are not exposed otherwise. +tasks.withType(JavaCompile).configureEach { + options.compilerArgs += [ + "--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.code=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.comp=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-exports=jdk.compiler/com.sun.source.tree=ALL-UNNAMED", + ] +} + +// Create a task to test with the oldest supported version of Error Prone +// (while still building against the latest supported version) +def epOldestTest = tasks.register("testErrorProneOldest", Test) { javaLauncher = javaToolchains.launcherFor { - languageVersion = JavaLanguageVersion.of(8) + languageVersion = JavaLanguageVersion.of(11) } - description = "Runs the test suite on JDK 8" + description = "Runs the test suite using the oldest supported version of Error Prone" group = LifecycleBasePlugin.VERIFICATION_GROUP // Copy inputs from normal Test task. @@ -121,18 +117,25 @@ def jdk8Test = tasks.register("testJdk8", Test) { classpath = configurations.errorProneOldest + testTask.classpath testClassesDirs = testTask.testClassesDirs - jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}" - filter { - // JDK 8 does not support diamonds on anonymous classes - excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests.overrideDiamondAnonymousClass" - // tests cannot run on JDK 8 since Mockito version no longer supports it - excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.initializationError" - excludeTestsMatching "com.uber.nullaway.handlers.contract.ContractUtilsTest.getEmptyAntecedent" - } + + 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", + // Accessed by Lombok tests + "--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED", + ] } tasks.named('check').configure { - dependsOn(jdk8Test) + dependsOn(epOldestTest) } // Create a task to build NullAway with NullAway checking enabled diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index 1f8e0be782..98d479f658 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -196,7 +196,7 @@ default ImmutableList customStreamNullabilitySpecs() { * * */ - final class MethodRef { + public final class MethodRef { public final String enclosingClass; diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index ee1944ad04..e5de3a9e1b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -463,7 +463,8 @@ private void updateEnvironmentMapping(TreePath treePath, VisitorState state) { // 2. we keep info on all locals rather than just effectively final ones for simplicity EnclosingEnvironmentNullness.instance(state.context) .addEnvironmentMapping( - treePath.getLeaf(), analysis.getNullnessInfoBeforeNewContext(treePath, state, handler)); + treePath.getLeaf(), + analysis.getNullnessInfoBeforeNestedMethodNode(treePath, state, handler)); } private Symbol.MethodSymbol getSymbolOfSuperConstructor( @@ -494,7 +495,7 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) { doUnboxingCheck(state, tree.getExpression()); } // generics check - if (lhsType != null && lhsType.getTypeArguments().length() > 0 && config.isJSpecifyMode()) { + if (lhsType != null && config.isJSpecifyMode()) { GenericsChecks.checkTypeParameterNullnessForAssignability(tree, this, state); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java index e68f05e8b5..2e1054a8e0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java @@ -211,18 +211,23 @@ public Set getNonnullStaticFieldsBefore(TreePath path, Context context) } /** - * Get nullness info for local variables (and final fields) before some node + * Get nullness info for local variables (and final fields) before some node represented a nested + * method (lambda or anonymous class) * - * @param path tree path to some AST node within a method / lambda / initializer + * @param pathToNestedMethodNode tree path to some AST node representing a nested method * @param state visitor state - * @return nullness info for local variables just before the node + * @param handler handler instance + * @return nullness info for local variables just before the leaf of the tree path */ - public NullnessStore getNullnessInfoBeforeNewContext( - TreePath path, VisitorState state, Handler handler) { - NullnessStore store = dataFlow.resultBefore(path, state.context, nullnessPropagation); + public NullnessStore getNullnessInfoBeforeNestedMethodNode( + TreePath pathToNestedMethodNode, VisitorState state, Handler handler) { + NullnessStore store = + dataFlow.resultBefore(pathToNestedMethodNode, state.context, nullnessPropagation); if (store == null) { return NullnessStore.empty(); } + Predicate handlerPredicate = + handler.getAccessPathPredicateForNestedMethod(pathToNestedMethodNode, state); return store.filterAccessPaths( (ap) -> { boolean allAPNonRootElementsAreFinalFields = true; @@ -243,7 +248,7 @@ public NullnessStore getNullnessInfoBeforeNewContext( && e.getModifiers().contains(Modifier.FINAL)); } - return handler.includeApInfoInSavedContext(ap, state); + return handlerPredicate.test(ap); }); } diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/cfg/NullAwayCFGBuilder.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/cfg/NullAwayCFGBuilder.java index 768eb0a4ea..c848d4665c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/cfg/NullAwayCFGBuilder.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/cfg/NullAwayCFGBuilder.java @@ -223,7 +223,7 @@ public R accept(TreeVisitor visitor, D data) { @Override public MethodInvocationNode visitMethodInvocation(MethodInvocationTree tree, Void p) { - MethodInvocationNode originalNode = super.visitMethodInvocation(tree, p); + MethodInvocationNode originalNode = super.visitMethodInvocation(tree, null); return handler.onCFGBuildPhase1AfterVisitMethodInvocation(this, tree, originalNode); } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java similarity index 62% rename from nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java rename to nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index b26ad80814..dda48e44ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -1,22 +1,20 @@ package com.uber.nullaway.generics; import com.google.errorprone.VisitorState; -import com.google.errorprone.util.ASTHelpers; -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 - * within a type. Compares the Type it is called upon, i.e. the LHS type and the Type passed as an - * argument, i.e. The RHS type. + * Visitor that checks for identical nullability annotations at all nesting levels within two types. + * Compares the Type it is called upon, i.e. the LHS type and the Type passed as an argument, i.e. + * The RHS type. */ -public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor { +public class CheckIdenticalNullabilityVisitor extends Types.DefaultTypeVisitor { private final VisitorState state; - CompareNullabilityVisitor(VisitorState state) { + CheckIdenticalNullabilityVisitor(VisitorState state) { this.state = state; } @@ -45,26 +43,8 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) { for (int i = 0; i < lhsTypeArguments.size(); i++) { Type lhsTypeArgument = lhsTypeArguments.get(i); Type rhsTypeArgument = rhsTypeArguments.get(i); - boolean isLHSNullableAnnotated = false; - List lhsAnnotations = lhsTypeArgument.getAnnotationMirrors(); - // To ensure that we are checking only jspecify nullable annotations - Type jspecifyNullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); - for (Attribute.TypeCompound annotation : lhsAnnotations) { - if (ASTHelpers.isSameType( - (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) { - isLHSNullableAnnotated = true; - break; - } - } - boolean isRHSNullableAnnotated = false; - List rhsAnnotations = rhsTypeArgument.getAnnotationMirrors(); - for (Attribute.TypeCompound annotation : rhsAnnotations) { - if (ASTHelpers.isSameType( - (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) { - isRHSNullableAnnotated = true; - break; - } - } + boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsTypeArgument, state); + boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsTypeArgument, state); if (isLHSNullableAnnotated != isRHSNullableAnnotated) { return false; } @@ -85,7 +65,14 @@ public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) { return true; } Type.ArrayType arrRhsType = (Type.ArrayType) rhsType; - return lhsType.getComponentType().accept(this, arrRhsType.getComponentType()); + Type lhsComponentType = lhsType.getComponentType(); + Type rhsComponentType = arrRhsType.getComponentType(); + boolean isLHSNullableAnnotated = GenericsChecks.isNullableAnnotated(lhsComponentType, state); + boolean isRHSNullableAnnotated = GenericsChecks.isNullableAnnotated(rhsComponentType, state); + if (isRHSNullableAnnotated != isLHSNullableAnnotated) { + return false; + } + return lhsComponentType.accept(this, rhsComponentType); } @Override 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 c863781da9..ffe45190ea 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -14,6 +14,7 @@ import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; @@ -34,6 +35,7 @@ import java.util.Map; import javax.annotation.Nullable; import javax.lang.model.type.ExecutableType; +import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeVariable; /** Methods for performing checks related to generic types and nullability. */ @@ -244,7 +246,7 @@ private static void reportInvalidOverridingMethodParamTypeError( * *

This method is required because in some cases, the type returned by {@link * com.google.errorprone.util.ASTHelpers#getType(Tree)} fails to preserve type use annotations, - * particularly when dealing with {@link com.sun.source.tree.NewClassTree} (e.g., {@code new + * e.g., when dealing with {@link com.sun.source.tree.NewClassTree} (e.g., {@code new * Foo<@Nullable A>}). * * @param tree A tree for which we need the type with preserved annotations. @@ -263,8 +265,29 @@ private static Type getTreeType(Tree tree, VisitorState state) { return null; } return typeWithPreservedAnnotations(paramTypedTree, state); + } else if (tree instanceof NewArrayTree + && ((NewArrayTree) tree).getType() instanceof AnnotatedTypeTree) { + return typeWithPreservedAnnotations(tree, state); } else { - Type result = ASTHelpers.getType(tree); + Type result; + if (tree instanceof VariableTree) { + // type on the tree itself can be missing nested annotations for arrays; get the type from + // the symbol for the declared variable instead + VariableTree varTree = (VariableTree) tree; + result = ASTHelpers.getSymbol(varTree).type; + } else if (tree instanceof AssignmentTree) { + // type on the tree itself can be missing nested annotations for arrays; get the type from + // the symbol for the assigned location instead, if available + AssignmentTree assignmentTree = (AssignmentTree) tree; + Symbol lhsSymbol = ASTHelpers.getSymbol(assignmentTree.getVariable()); + if (lhsSymbol != null) { + result = lhsSymbol.type; + } else { + result = ASTHelpers.getType(assignmentTree); + } + } else { + result = ASTHelpers.getType(tree); + } if (result != null && result.isRaw()) { // bail out of any checking involving raw types for now return null; @@ -289,15 +312,13 @@ public static void checkTypeParameterNullnessForAssignability( if (!analysis.getConfig().isJSpecifyMode()) { return; } - Tree lhsTree; + Type lhsType = getTreeType(tree, state); Tree rhsTree; if (tree instanceof VariableTree) { VariableTree varTree = (VariableTree) tree; - lhsTree = varTree.getType(); rhsTree = varTree.getInitializer(); } else { AssignmentTree assignmentTree = (AssignmentTree) tree; - lhsTree = assignmentTree.getVariable(); rhsTree = assignmentTree.getExpression(); } // rhsTree can be null for a VariableTree. Also, we don't need to do a check @@ -305,11 +326,10 @@ public static void checkTypeParameterNullnessForAssignability( if (rhsTree == null || rhsTree.getKind().equals(Tree.Kind.NULL_LITERAL)) { return; } - Type lhsType = getTreeType(lhsTree, state); Type rhsType = getTreeType(rhsTree, state); if (lhsType != null && rhsType != null) { - boolean isAssignmentValid = compareNullabilityAnnotations(lhsType, rhsType, state); + boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state); if (!isAssignmentValid) { reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis); } @@ -342,7 +362,7 @@ public static void checkTypeParameterNullnessForFunctionReturnType( Type returnExpressionType = getTreeType(retExpr, state); if (formalReturnType != null && returnExpressionType != null) { boolean isReturnTypeValid = - compareNullabilityAnnotations(formalReturnType, returnExpressionType, state); + subtypeParameterNullability(formalReturnType, returnExpressionType, state); if (!isReturnTypeValid) { reportInvalidReturnTypeError( retExpr, formalReturnType, returnExpressionType, state, analysis); @@ -351,9 +371,8 @@ public static void checkTypeParameterNullnessForFunctionReturnType( } /** - * Compare two types from an assignment for identical type parameter nullability, recursively - * checking nested generic types. See the JSpecify + * Compare two types for identical type parameter nullability, recursively checking nested generic + * types. See the JSpecify * specification and the JLS * subtyping rules for class and interface types. @@ -362,11 +381,39 @@ public static void checkTypeParameterNullnessForFunctionReturnType( * @param rhsType type for the rhs of the assignment * @param state the visitor state */ - private static boolean compareNullabilityAnnotations( + private static boolean identicalTypeParameterNullability( Type lhsType, Type rhsType, VisitorState state) { - // it is fair to assume rhyType should be the same as lhsType as the Java compiler has passed - // before NullAway. - return lhsType.accept(new CompareNullabilityVisitor(state), rhsType); + return lhsType.accept(new CheckIdenticalNullabilityVisitor(state), rhsType); + } + + /** + * Like {@link #identicalTypeParameterNullability(Type, Type, VisitorState)}, but allows for + * covariant array subtyping at the top level. + * + * @param lhsType type for the lhs of the assignment + * @param rhsType type for the rhs of the assignment + * @param state the visitor state + */ + private static boolean subtypeParameterNullability( + Type lhsType, Type rhsType, VisitorState state) { + if (lhsType.getKind().equals(TypeKind.ARRAY) && rhsType.getKind().equals(TypeKind.ARRAY)) { + // for array types we must allow covariance, i.e., an array of @NonNull references is a + // subtype of an array of @Nullable references; see + // https://github.com/jspecify/jspecify/issues/65 + Type.ArrayType lhsArrayType = (Type.ArrayType) lhsType; + Type.ArrayType rhsArrayType = (Type.ArrayType) rhsType; + Type lhsComponentType = lhsArrayType.getComponentType(); + Type rhsComponentType = rhsArrayType.getComponentType(); + boolean isLHSNullableAnnotated = isNullableAnnotated(lhsComponentType, state); + boolean isRHSNullableAnnotated = isNullableAnnotated(rhsComponentType, state); + // an array of @Nullable references is _not_ a subtype of an array of @NonNull references + if (isRHSNullableAnnotated && !isLHSNullableAnnotated) { + return false; + } + return identicalTypeParameterNullability(lhsComponentType, rhsComponentType, state); + } else { + return identicalTypeParameterNullability(lhsType, rhsType, state); + } } /** @@ -378,9 +425,8 @@ private static boolean compareNullabilityAnnotations( * @param state the visitor state * @return A Type with preserved annotations. */ - private static Type.ClassType typeWithPreservedAnnotations( - ParameterizedTypeTree tree, VisitorState state) { - return (Type.ClassType) tree.accept(new PreservedAnnotationTreeVisitor(state), null); + private static Type typeWithPreservedAnnotations(Tree tree, VisitorState state) { + return tree.accept(new PreservedAnnotationTreeVisitor(state), null); } /** @@ -407,7 +453,7 @@ public static void checkTypeParameterNullnessForConditionalExpression( Tree truePartTree = tree.getTrueExpression(); Tree falsePartTree = tree.getFalseExpression(); - Type condExprType = getTreeType(tree, state); + Type condExprType = getConditionalExpressionType(tree, state); Type truePartType = getTreeType(truePartTree, state); Type falsePartType = getTreeType(falsePartTree, state); // The condExpr type should be the least-upper bound of the true and false part types. To check @@ -415,13 +461,13 @@ public static void checkTypeParameterNullnessForConditionalExpression( // type of the whole expression if (condExprType != null) { if (truePartType != null) { - if (!compareNullabilityAnnotations(condExprType, truePartType, state)) { + if (!subtypeParameterNullability(condExprType, truePartType, state)) { reportMismatchedTypeForTernaryOperator( truePartTree, condExprType, truePartType, state, analysis); } } if (falsePartType != null) { - if (!compareNullabilityAnnotations(condExprType, falsePartType, state)) { + if (!subtypeParameterNullability(condExprType, falsePartType, state)) { reportMismatchedTypeForTernaryOperator( falsePartTree, condExprType, falsePartType, state, analysis); } @@ -429,6 +475,18 @@ public static void checkTypeParameterNullnessForConditionalExpression( } } + @Nullable + private static Type getConditionalExpressionType( + ConditionalExpressionTree tree, VisitorState state) { + // hack: sometimes array nullability doesn't get computed correctly for a conditional expression + // on the RHS of an assignment. So, look at the type of the assignment tree. + Tree parent = state.getPath().getParentPath().getLeaf(); + if (parent instanceof AssignmentTree || parent instanceof VariableTree) { + return getTreeType(parent, state); + } + return getTreeType(tree, state); + } + /** * Checks that for each parameter p at a call, the type parameter nullability for p's type matches * that of the corresponding formal parameter. If a mismatch is found, report an error. @@ -459,7 +517,7 @@ public static void compareGenericTypeParameterNullabilityForCall( if (!formalParameter.getTypeArguments().isEmpty()) { Type actualParameter = getTreeType(actualParams.get(i), state); if (actualParameter != null) { - if (!compareNullabilityAnnotations(formalParameter, actualParameter, state)) { + if (!subtypeParameterNullability(formalParameter, actualParameter, state)) { reportInvalidParametersNullabilityError( formalParameter, actualParameter, actualParams.get(i), state, analysis); } @@ -474,7 +532,7 @@ public static void compareGenericTypeParameterNullabilityForCall( for (int i = formalParams.size() - 1; i < actualParams.size(); i++) { Type actualParameter = getTreeType(actualParams.get(i), state); if (actualParameter != null) { - if (!compareNullabilityAnnotations(varargsElementType, actualParameter, state)) { + if (!subtypeParameterNullability(varargsElementType, actualParameter, state)) { reportInvalidParametersNullabilityError( varargsElementType, actualParameter, actualParams.get(i), state, analysis); } @@ -781,8 +839,9 @@ private static void checkTypeParameterNullnessForOverridingMethodParameterType( Type overridingMethodParameterType = getTreeType(methodParameters.get(i), state); Type overriddenMethodParameterType = overriddenMethodParameterTypes.get(i); if (overriddenMethodParameterType != null && overridingMethodParameterType != null) { - if (!compareNullabilityAnnotations( - overriddenMethodParameterType, overridingMethodParameterType, state)) { + // allow contravariant subtyping + if (!subtypeParameterNullability( + overridingMethodParameterType, overriddenMethodParameterType, state)) { reportInvalidOverridingMethodParamTypeError( methodParameters.get(i), overriddenMethodParameterType, @@ -807,11 +866,14 @@ private static void checkTypeParameterNullnessForOverridingMethodParameterType( private static void checkTypeParameterNullnessForOverridingMethodReturnType( MethodTree tree, Type overriddenMethodType, NullAway analysis, VisitorState state) { Type overriddenMethodReturnType = overriddenMethodType.getReturnType(); - Type overridingMethodReturnType = getTreeType(tree.getReturnType(), state); - if (overriddenMethodReturnType == null || overridingMethodReturnType == null) { + // We get the return type from the Symbol; the type attached to tree may not have correct + // annotations for array types + Type overridingMethodReturnType = ASTHelpers.getSymbol(tree).getReturnType(); + if (overriddenMethodReturnType.isRaw() || overridingMethodReturnType.isRaw()) { return; } - if (!compareNullabilityAnnotations( + // allow covariant subtyping + if (!subtypeParameterNullability( overriddenMethodReturnType, overridingMethodReturnType, state)) { reportInvalidOverridingMethodReturnTypeError( tree, overriddenMethodReturnType, overridingMethodReturnType, analysis, state); @@ -882,4 +944,19 @@ public static boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( } return callingUnannotated; } + + public static boolean isNullableAnnotated(Type type, VisitorState state) { + boolean result = false; + // To ensure that we are checking only jspecify nullable annotations + Type jspecifyNullableType = JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); + List lhsAnnotations = type.getAnnotationMirrors(); + for (Attribute.TypeCompound annotation : lhsAnnotations) { + if (ASTHelpers.isSameType( + (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) { + result = true; + break; + } + } + return result; + } } 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 822fcf193c..42797b0ff6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java @@ -2,16 +2,17 @@ import static com.uber.nullaway.NullabilityUtil.castToNonNull; -import com.google.common.base.Preconditions; import com.google.errorprone.VisitorState; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.ArrayTypeTree; +import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; import com.sun.source.util.SimpleTreeVisitor; import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.TypeMetadata; import com.sun.tools.javac.util.ListBuffer; @@ -36,6 +37,12 @@ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor typeArguments = tree.getTypeArguments(); List newTypeArgs = new ArrayList<>(); for (int i = 0; i < typeArguments.size(); i++) { - AnnotatedTypeTree annotatedType = null; - Tree curTypeArg = typeArguments.get(i); - // If the type argument has an annotation, it will either be an AnnotatedTypeTree, or a - // ParameterizedTypeTree in the case of a nested generic type - if (curTypeArg instanceof AnnotatedTypeTree) { - annotatedType = (AnnotatedTypeTree) curTypeArg; - } else if (curTypeArg instanceof ParameterizedTypeTree - && ((ParameterizedTypeTree) curTypeArg).getType() instanceof AnnotatedTypeTree) { - annotatedType = (AnnotatedTypeTree) ((ParameterizedTypeTree) curTypeArg).getType(); - } - List annotations = - annotatedType != null ? annotatedType.getAnnotations() : Collections.emptyList(); - boolean hasNullableAnnotation = false; - for (AnnotationTree annotation : annotations) { - if (ASTHelpers.isSameType( - nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) { - hasNullableAnnotation = true; - break; - } - } - // construct a TypeMetadata object containing a nullability annotation if needed - com.sun.tools.javac.util.List nullableAnnotationCompound = - hasNullableAnnotation - ? com.sun.tools.javac.util.List.from( - Collections.singletonList( - new Attribute.TypeCompound( - nullableType, com.sun.tools.javac.util.List.nil(), null))) - : com.sun.tools.javac.util.List.nil(); - TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(nullableAnnotationCompound); - Type currentTypeArgType = curTypeArg.accept(this, null); - Type newTypeArgType = - TYPE_METADATA_BUILDER.cloneTypeWithMetadata(currentTypeArgType, typeMetadata); - newTypeArgs.add(newTypeArgType); + newTypeArgs.add(typeArguments.get(i).accept(this, null)); } - Type.ClassType finalType = - new Type.ClassType( - type.getEnclosingType(), com.sun.tools.javac.util.List.from(newTypeArgs), type.tsym); + Type finalType = TYPE_METADATA_BUILDER.createWithBaseTypeAndTypeArgs(baseType, newTypeArgs); return finalType; } + @Override + public Type visitAnnotatedType(AnnotatedTypeTree annotatedType, Void unused) { + List annotations = annotatedType.getAnnotations(); + boolean hasNullableAnnotation = false; + Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); + for (AnnotationTree annotation : annotations) { + if (ASTHelpers.isSameType( + nullableType, ASTHelpers.getType(annotation.getAnnotationType()), state)) { + hasNullableAnnotation = true; + break; + } + } + // construct a TypeMetadata object containing a nullability annotation if needed + com.sun.tools.javac.util.List nullableAnnotationCompound = + hasNullableAnnotation + ? com.sun.tools.javac.util.List.from( + Collections.singletonList( + new Attribute.TypeCompound( + nullableType, com.sun.tools.javac.util.List.nil(), null))) + : com.sun.tools.javac.util.List.nil(); + TypeMetadata typeMetadata = TYPE_METADATA_BUILDER.create(nullableAnnotationCompound); + Type underlyingType = annotatedType.getUnderlyingType().accept(this, null); + Type newType = TYPE_METADATA_BUILDER.cloneTypeWithMetadata(underlyingType, typeMetadata); + return newType; + } + /** By default, just use the type computed by javac */ @Override protected Type defaultAction(Tree node, Void unused) { @@ -104,6 +101,8 @@ private interface TypeMetadataBuilder { TypeMetadata create(com.sun.tools.javac.util.List attrs); Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metaData); + + Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs); } /** @@ -129,6 +128,15 @@ public TypeMetadata create(com.sun.tools.javac.util.List public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { return typeToBeCloned.cloneWithMetadata(metadata); } + + @Override + public Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs) { + return new Type.ClassType( + baseType.getEnclosingType(), + com.sun.tools.javac.util.List.from(typeArgs), + baseType.tsym, + baseType.getMetadata()); + } } /** @@ -138,11 +146,14 @@ public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { */ private static class JDK21TypeMetadataBuilder implements TypeMetadataBuilder { - private static final MethodHandle typeMetadataHandle = createHandle(); + private static final MethodHandle typeMetadataConstructorHandle = 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 final MethodHandle getMetadataHandler = createGetMetadataHandle(); + private static final MethodHandle classTypeConstructorHandle = + createClassTypeConstructorHandle(); private static MethodHandle createHandle() { MethodHandles.Lookup lookup = MethodHandles.lookup(); @@ -156,6 +167,32 @@ private static MethodHandle createHandle() { } } + private static MethodHandle createGetMetadataHandle() { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType mt = MethodType.methodType(com.sun.tools.javac.util.List.class); + try { + return lookup.findVirtual(Type.class, "getMetadata", mt); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + private static MethodHandle createClassTypeConstructorHandle() { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodType methodType = + MethodType.methodType( + void.class, // return type for a constructor is void + Type.class, + com.sun.tools.javac.util.List.class, + Symbol.TypeSymbol.class, + com.sun.tools.javac.util.List.class); + return lookup.findConstructor(Type.ClassType.class, methodType); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + /** * Used to get a MethodHandle for a virtual method from the specified class * @@ -183,7 +220,7 @@ public TypeMetadata create(com.sun.tools.javac.util.List ListBuffer b = new ListBuffer<>(); b.appendList(attrs); try { - return (TypeMetadata) typeMetadataHandle.invoke(b); + return (TypeMetadata) typeMetadataConstructorHandle.invoke(b); } catch (Throwable e) { throw new RuntimeException(e); } @@ -209,6 +246,22 @@ public Type cloneTypeWithMetadata(Type typeToBeCloned, TypeMetadata metadata) { throw new RuntimeException(e); } } + + @Override + public Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs) { + try { + com.sun.tools.javac.util.List metadata = + (com.sun.tools.javac.util.List) getMetadataHandler.invoke(baseType); + return (Type) + classTypeConstructorHandle.invoke( + baseType.getEnclosingType(), + com.sun.tools.javac.util.List.from(typeArgs), + baseType.tsym, + metadata); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } } /** The TypeMetadataBuilder to be used for the current JDK version. */ diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AccessPathPredicates.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AccessPathPredicates.java new file mode 100644 index 0000000000..1b239d5ac5 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AccessPathPredicates.java @@ -0,0 +1,25 @@ +package com.uber.nullaway.handlers; + +import com.google.errorprone.VisitorState; +import com.sun.source.util.TreePath; +import com.uber.nullaway.dataflow.AccessPath; +import java.util.function.Predicate; + +/** + * {@link java.util.function.Predicate}s over {@link com.uber.nullaway.dataflow.AccessPath}s useful + * in defining handlers. + */ +public class AccessPathPredicates { + + /** + * An AccessPath predicate that always returns false. Used to optimize {@link + * CompositeHandler#getAccessPathPredicateForNestedMethod(TreePath, VisitorState)} + */ + static final Predicate FALSE_AP_PREDICATE = ap -> false; + + /** + * An AccessPath predicate that always returns true. Used to optimize {@link + * CompositeHandler#getAccessPathPredicateForNestedMethod(TreePath, VisitorState)} + */ + static final Predicate TRUE_AP_PREDICATE = ap -> true; +} 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 3cc0e92f81..171836276b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java @@ -31,6 +31,7 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ReturnTree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Context; @@ -44,6 +45,7 @@ import com.uber.nullaway.dataflow.cfg.NullAwayCFGBuilder; import java.util.List; import java.util.Optional; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode; @@ -199,8 +201,9 @@ public Optional onExpressionDereference( } @Override - public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { - return false; + public Predicate getAccessPathPredicateForNestedMethod( + TreePath path, VisitorState state) { + return AccessPathPredicates.FALSE_AP_PREDICATE; } @Override 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 a8eec51ac7..b05128a00a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -22,6 +22,9 @@ package com.uber.nullaway.handlers; +import static com.uber.nullaway.handlers.AccessPathPredicates.FALSE_AP_PREDICATE; +import static com.uber.nullaway.handlers.AccessPathPredicates.TRUE_AP_PREDICATE; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.errorprone.VisitorState; @@ -32,6 +35,7 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ReturnTree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Context; @@ -45,6 +49,7 @@ import com.uber.nullaway.dataflow.cfg.NullAwayCFGBuilder; import java.util.List; import java.util.Optional; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode; @@ -253,12 +258,24 @@ public Optional onExpressionDereference( } @Override - public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { - boolean shouldFilter = false; + public Predicate getAccessPathPredicateForNestedMethod( + TreePath path, VisitorState state) { + Predicate filter = FALSE_AP_PREDICATE; for (Handler h : handlers) { - shouldFilter |= h.includeApInfoInSavedContext(accessPath, state); + Predicate curFilter = h.getAccessPathPredicateForNestedMethod(path, state); + // here we do some optimization, to try to avoid unnecessarily returning a deeply nested + // Predicate object (which would be more costly to test) + if (curFilter != FALSE_AP_PREDICATE) { + if (curFilter == TRUE_AP_PREDICATE) { + return curFilter; + } else if (filter == FALSE_AP_PREDICATE) { + filter = curFilter; + } else { + filter = filter.or(curFilter); + } + } } - return shouldFilter; + return filter; } @Override 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 ea084c3cb2..08477a056f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -31,6 +31,7 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.ReturnTree; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; import com.sun.tools.javac.util.Context; @@ -45,6 +46,7 @@ import com.uber.nullaway.dataflow.cfg.NullAwayCFGBuilder; import java.util.List; import java.util.Optional; +import java.util.function.Predicate; import javax.annotation.Nullable; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode; @@ -327,15 +329,16 @@ Optional onExpressionDereference( ExpressionTree expr, ExpressionTree baseExpr, VisitorState state); /** - * Called when the store access paths are filtered for local variable information before an - * expression. + * Called when determining which access path nullability information should be preserved when + * analyzing a nested method, i.e., a lambda expression or a method in an anonymous or local + * class. * - * @param accessPath The access path that needs to be checked if filtered. + * @param path The tree path to the node for the nested method. * @param state The current visitor state. - * @return true if the nullability information for this accesspath should be treated as part of - * the surrounding context when processing a lambda expression or anonymous class declaration. + * @return A predicate that determines which access paths should be preserved when analyzing the + * nested method. */ - boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state); + Predicate getAccessPathPredicateForNestedMethod(TreePath path, VisitorState state); /** * Called during dataflow analysis initialization to register structurally immutable types. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java index f82f343ffb..c9c012c3da 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java @@ -69,6 +69,7 @@ public static Handler buildDefault(Config config) { handlerListBuilder.add(new GrpcHandler()); handlerListBuilder.add(new RequiresNonNullHandler()); handlerListBuilder.add(new EnsuresNonNullHandler()); + handlerListBuilder.add(new SynchronousCallbackHandler()); if (config.serializationIsActive() && config.getSerializationConfig().fieldInitInfoEnabled) { handlerListBuilder.add( new FieldInitializationSerializationHandler(config.getSerializationConfig())); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java index e5818854dc..4e1858b050 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java @@ -163,20 +163,6 @@ private boolean isOptionalContentNullable( == Nullness.NULLABLE; } - @Override - public boolean includeApInfoInSavedContext(AccessPath accessPath, VisitorState state) { - - if (accessPath.getElements().size() == 1) { - final Element e = accessPath.getRoot(); - if (e != null) { - return e.getKind().equals(ElementKind.LOCAL_VARIABLE) - && accessPath.getElements().get(0).getJavaElement() - instanceof OptionalContentVariableElement; - } - } - return false; - } - private void handleTestAssertions( VisitorState state, AccessPath.AccessPathContext apContext, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java new file mode 100644 index 0000000000..ca49de3f98 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java @@ -0,0 +1,94 @@ +package com.uber.nullaway.handlers; + +import static com.uber.nullaway.handlers.AccessPathPredicates.FALSE_AP_PREDICATE; +import static com.uber.nullaway.handlers.AccessPathPredicates.TRUE_AP_PREDICATE; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import com.google.errorprone.VisitorState; +import com.google.errorprone.suppliers.Supplier; +import com.google.errorprone.suppliers.Suppliers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.LambdaExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Type; +import com.uber.nullaway.LibraryModels.MethodRef; +import com.uber.nullaway.dataflow.AccessPath; +import java.util.function.Predicate; + +public class SynchronousCallbackHandler extends BaseNoOpHandler { + + /** + * Maps method name to full information about the corresponding methods and what parameter is the + * relevant callback. We key on method name to quickly eliminate most cases when doing a lookup. + */ + private static final ImmutableMap> + METHOD_NAME_TO_SIG_AND_PARAM_INDEX = + ImmutableMap.of( + "forEach", + ImmutableMap.of( + MethodRef.methodRef( + "java.util.Map", + "forEach(java.util.function.BiConsumer)"), + 0, + MethodRef.methodRef( + "java.lang.Iterable", "forEach(java.util.function.Consumer)"), + 0), + "removeIf", + ImmutableMap.of( + MethodRef.methodRef( + "java.util.Collection", "removeIf(java.util.function.Predicate)"), + 0)); + + private static final Supplier STREAM_TYPE_SUPPLIER = + Suppliers.typeFromString("java.util.stream.Stream"); + + @Override + public Predicate getAccessPathPredicateForNestedMethod( + TreePath path, VisitorState state) { + Tree leafNode = path.getLeaf(); + Preconditions.checkArgument( + leafNode instanceof ClassTree || leafNode instanceof LambdaExpressionTree, + "Unexpected leaf type: %s", + leafNode.getClass()); + Tree parentNode = path.getParentPath().getLeaf(); + if (parentNode instanceof MethodInvocationTree) { + MethodInvocationTree methodInvocationTree = (MethodInvocationTree) parentNode; + Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(methodInvocationTree); + if (symbol == null) { + return FALSE_AP_PREDICATE; + } + Type ownerType = symbol.owner.type; + if (ASTHelpers.isSameType(ownerType, STREAM_TYPE_SUPPLIER.get(state), state)) { + // preserve access paths for all callbacks passed to stream methods + return TRUE_AP_PREDICATE; + } + String invokedMethodName = symbol.getSimpleName().toString(); + if (METHOD_NAME_TO_SIG_AND_PARAM_INDEX.containsKey(invokedMethodName)) { + ImmutableMap entriesForMethodName = + METHOD_NAME_TO_SIG_AND_PARAM_INDEX.get(invokedMethodName); + for (MethodRef methodRef : entriesForMethodName.keySet()) { + if (symbol.toString().equals(methodRef.fullMethodSig) + && ASTHelpers.isSubtype( + ownerType, state.getTypeFromString(methodRef.enclosingClass), state)) { + int parameterIndex = -1; + for (int i = 0; i < methodInvocationTree.getArguments().size(); i++) { + if (methodInvocationTree.getArguments().get(i) == leafNode) { + parameterIndex = i; + break; + } + } + if (parameterIndex == entriesForMethodName.get(methodRef)) { + return TRUE_AP_PREDICATE; + } + } + } + } + } + return FALSE_AP_PREDICATE; + } +} diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java index bfbadf7177..d0386909d6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java @@ -156,7 +156,7 @@ public Void visitReturn(ReturnTree returnTree, Void unused) { returnState, null)); } - return super.visitReturn(returnTree, unused); + return super.visitReturn(returnTree, null); } }.scan(state.getPath(), null); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java b/nullaway/src/test/java/com/uber/nullaway/AccessPathsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java rename to nullaway/src/test/java/com/uber/nullaway/AccessPathsTests.java index 7282e4e043..1061e9f7c6 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/AccessPathsTests.java @@ -2,7 +2,7 @@ import org.junit.Test; -public class NullAwayAccessPathsTests extends NullAwayTestsBase { +public class AccessPathsTests extends NullAwayTestsBase { @Test public void testConstantsInAccessPathsNegative() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java b/nullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java rename to nullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.java index e84016c37f..854ba18ac2 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayAcknowledgeRestrictiveAnnotationsTests extends NullAwayTestsBase { +public class AcknowledgeRestrictiveAnnotationsTests extends NullAwayTestsBase { @Test public void generatedAsUnannotatedPlusRestrictive() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAndroidTest.java b/nullaway/src/test/java/com/uber/nullaway/AndroidTest.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAndroidTest.java rename to nullaway/src/test/java/com/uber/nullaway/AndroidTest.java index 5a63cefbb0..c00de7f993 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAndroidTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/AndroidTest.java @@ -11,7 +11,7 @@ /** Unit tests for {@link com.uber.nullaway.NullAway}. */ @RunWith(JUnit4.class) -public class NullAwayAndroidTest { +public class AndroidTest { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); private CompilationTestHelper compilationHelper; diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java b/nullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java rename to nullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.java index 29674227a4..bd9cc1231a 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/AssertionLibsTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayAssertionLibsTests extends NullAwayTestsBase { +public class AssertionLibsTests extends NullAwayTestsBase { @Test public void supportTruthAssertThatIsNotNull_Object() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java b/nullaway/src/test/java/com/uber/nullaway/AutoSuggestNoCastTest.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java rename to nullaway/src/test/java/com/uber/nullaway/AutoSuggestNoCastTest.java index 1817d25f84..ccfed519b7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/AutoSuggestNoCastTest.java @@ -30,7 +30,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class NullAwayAutoSuggestNoCastTest { +public class AutoSuggestNoCastTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java b/nullaway/src/test/java/com/uber/nullaway/AutoSuggestTest.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java rename to nullaway/src/test/java/com/uber/nullaway/AutoSuggestTest.java index ba5eabea27..21c4b98bc7 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/AutoSuggestTest.java @@ -35,7 +35,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class NullAwayAutoSuggestTest { +public class AutoSuggestTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java b/nullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.java similarity index 97% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java rename to nullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.java index 338bc721c2..ddc04f4718 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/BytecodeInteractionsTests.java @@ -2,7 +2,7 @@ import org.junit.Test; -public class NullAwayBytecodeInteractionsTests extends NullAwayTestsBase { +public class BytecodeInteractionsTests extends NullAwayTestsBase { @Test public void typeUseJarReturn() { defaultCompilationHelper diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java b/nullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java rename to nullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.java index 201b4fccde..2c80acab4b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/ContractsBooleanTests.java @@ -4,7 +4,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayContractsBooleanTests extends NullAwayTestsBase { +public class ContractsBooleanTests extends NullAwayTestsBase { @Test public void nonNullCheckIsTrueIsNotNullable() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java b/nullaway/src/test/java/com/uber/nullaway/ContractsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java rename to nullaway/src/test/java/com/uber/nullaway/ContractsTests.java index d3849b041d..664dffaab8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/ContractsTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayContractsTests extends NullAwayTestsBase { +public class ContractsTests extends NullAwayTestsBase { @Test public void checkContractPositiveCases() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java b/nullaway/src/test/java/com/uber/nullaway/CoreTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java rename to nullaway/src/test/java/com/uber/nullaway/CoreTests.java index d8167564f9..14dbdea5c4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/CoreTests.java @@ -29,7 +29,7 @@ /** Unit tests for {@link com.uber.nullaway.NullAway}. */ @RunWith(JUnit4.class) -public class NullAwayCoreTests extends NullAwayTestsBase { +public class CoreTests extends NullAwayTestsBase { @Test public void coreNullabilityPositiveCases() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java rename to nullaway/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java index 098b98b6b3..830456b251 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/CustomLibraryModelsTests.java @@ -29,7 +29,7 @@ import java.util.List; import org.junit.Test; -public class NullAwayCustomLibraryModelsTests extends NullAwayTestsBase { +public class CustomLibraryModelsTests extends NullAwayTestsBase { private CompilationTestHelper makeLibraryModelsTestHelperWithArgs(List args) { // Adding directly to args will throw an UnsupportedOperationException, since that list is diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java b/nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java rename to nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.java index 1e9e09c851..dd94e36a5d 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.java @@ -2,7 +2,7 @@ import org.junit.Test; -public class NullAwayEnsuresNonNullTests extends NullAwayTestsBase { +public class EnsuresNonNullTests extends NullAwayTestsBase { @Test public void requiresNonNullInterpretation() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java rename to nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java index 5f46f7a901..b4e34a2533 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayFrameworkTests extends NullAwayTestsBase { +public class FrameworkTests extends NullAwayTestsBase { @Test public void lombokSupportTesting() { defaultCompilationHelper.addSourceFile("lombok/LombokBuilderInit.java").doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayFunctionalInterfaceNullabilityTests.java b/nullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.java similarity index 98% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayFunctionalInterfaceNullabilityTests.java rename to nullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.java index 25e8846537..7540404bab 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayFunctionalInterfaceNullabilityTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/FunctionalInterfaceNullabilityTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayFunctionalInterfaceNullabilityTests extends NullAwayTestsBase { +public class FunctionalInterfaceNullabilityTests extends NullAwayTestsBase { @Test public void multipleTypeParametersInstantiation() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java b/nullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java rename to nullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.java index 83ee0c94e5..8fb2ea61df 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/GuavaAssertionsTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayGuavaAssertionsTests extends NullAwayTestsBase { +public class GuavaAssertionsTests extends NullAwayTestsBase { @Test public void checkNotNullTest() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java b/nullaway/src/test/java/com/uber/nullaway/InitializationTests.java similarity index 98% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java rename to nullaway/src/test/java/com/uber/nullaway/InitializationTests.java index f953d8c8be..2bd7af9f0e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/InitializationTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayInitializationTests extends NullAwayTestsBase { +public class InitializationTests extends NullAwayTestsBase { @Test public void initFieldPositiveCases() { defaultCompilationHelper.addSourceFile("CheckFieldInitPositiveCases.java").doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJava8Tests.java b/nullaway/src/test/java/com/uber/nullaway/Java8Tests.java similarity index 96% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayJava8Tests.java rename to nullaway/src/test/java/com/uber/nullaway/Java8Tests.java index 72fe2a36e7..478d3e1275 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJava8Tests.java +++ b/nullaway/src/test/java/com/uber/nullaway/Java8Tests.java @@ -2,7 +2,7 @@ import org.junit.Test; -public class NullAwayJava8Tests extends NullAwayTestsBase { +public class Java8Tests extends NullAwayTestsBase { @Test public void java8PositiveCases() { defaultCompilationHelper.addSourceFile("NullAwayJava8PositiveCases.java").doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java b/nullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.java similarity index 98% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java rename to nullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.java index 5b39012149..ea0d11fdcd 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/KeySetIteratorTests.java @@ -2,7 +2,7 @@ import org.junit.Test; -public class NullAwayKeySetIteratorTests extends NullAwayTestsBase { +public class KeySetIteratorTests extends NullAwayTestsBase { @Test public void mapKeySetIteratorBasic() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java deleted file mode 100644 index 06e13fe46e..0000000000 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java +++ /dev/null @@ -1,38 +0,0 @@ -package com.uber.nullaway; - -import org.junit.Test; - -public class NullAwayTypeUseAnnotationTests extends NullAwayTestsBase { - @Test - public void typeParameterAnnotationIsDistinctFromMethodReturnAnnotation() { - defaultCompilationHelper - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.Nullable;", - "import java.util.function.Supplier;", - "public class Test {", - " public static @Nullable Supplier<@Nullable R> getNullableSupplierOfNullable() {", - " return new Supplier() {", - " @Nullable", - " public R get() { return null; }", - " };", - " }", - " public static Supplier<@Nullable R> getNonNullSupplierOfNullable() {", - " return new Supplier() {", - " @Nullable", - " public R get() { return null; }", - " };", - " }", - " public static String test1() {", - " // BUG: Diagnostic contains: dereferenced expression getNullableSupplierOfNullable() is @Nullable", - " return getNullableSupplierOfNullable().toString();", - " }", - " public static String test2() {", - " // The supplier contains null, but isn't itself nullable. Check against a past FP", - " return getNonNullSupplierOfNullable().toString();", - " }", - "}") - .doTest(); - } -} diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java b/nullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java rename to nullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.java index 32199f48d1..679652b63e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/OptionalEmptinessTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayOptionalEmptinessTests extends NullAwayTestsBase { +public class OptionalEmptinessTests extends NullAwayTestsBase { @Test public void optionalEmptinessHandlerTest() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java rename to nullaway/src/test/java/com/uber/nullaway/SerializationTest.java index 86eff7ce20..1dc585a23a 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/SerializationTest.java @@ -56,7 +56,7 @@ /** Unit tests for {@link com.uber.nullaway.NullAway}. */ @RunWith(JUnit4.class) -public class NullAwaySerializationTest extends NullAwayTestsBase { +public class SerializationTest extends NullAwayTestsBase { private String configPath; private Path root; private final DisplayFactory fixDisplayFactory; @@ -71,7 +71,7 @@ public class NullAwaySerializationTest extends NullAwayTestsBase { private static final String FIELD_INIT_FILE_NAME = "field_init.tsv"; private static final String FIELD_INIT_HEADER = FieldInitializationInfo.header(); - public NullAwaySerializationTest() { + public SerializationTest() { this.fixDisplayFactory = values -> { Preconditions.checkArgument( diff --git a/nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java b/nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java new file mode 100644 index 0000000000..0189b3d3e6 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java @@ -0,0 +1,166 @@ +package com.uber.nullaway; + +import org.junit.Test; + +/** + * Tests for cases where lambdas or anonymous class methods are invoked nearly synchronously, so it + * is reasonable to propagate more nullability information to their bodies. + */ +public class SyncLambdasTests extends NullAwayTestsBase { + + @Test + public void forEachOnMap() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.Map;", + "import java.util.HashMap;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable Map target;", + " private @Nullable Map resolved;", + " public void initialize() {", + " if (this.target == null) {", + " throw new IllegalArgumentException();", + " }", + " this.resolved = new HashMap<>();", + " this.target.forEach((key, value) -> {", + " // no error here as info gets propagated", + " this.resolved.put(key, value);", + " });", + " }", + "}") + .doTest(); + } + + @Test + public void forEachOnHashMap() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.HashMap;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable HashMap target;", + " private @Nullable HashMap resolved;", + " public void initialize() {", + " if (this.target == null) {", + " throw new IllegalArgumentException();", + " }", + " this.resolved = new HashMap<>();", + " this.target.forEach((key, value) -> {", + " // no error here as info gets propagated", + " this.resolved.put(key, value);", + " });", + " }", + "}") + .doTest(); + } + + @Test + public void otherForEach() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.HashMap;", + "import java.util.function.BiConsumer;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable MyMap target;", + " private @Nullable Object resolved;", + " static class MyMap {", + " public void forEach(BiConsumer consumer) {}", + " public void put(Object key, Object value) {}", + " }", + " public void initialize() {", + " if (this.target == null) {", + " throw new IllegalArgumentException();", + " }", + " this.resolved = new Object();", + " this.target.forEach((key, value) -> {", + " // error since this is a custom type, not inheriting from java.util.Map", + " // BUG: Diagnostic contains: dereferenced expression this.resolved is @Nullable", + " System.out.println(this.resolved.toString());", + " });", + " }", + "}") + .doTest(); + } + + @Test + public void forEachOnIterable() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.List;", + "import java.util.ArrayList;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable Object f;", + " public void test1() {", + " if (this.f == null) {", + " throw new IllegalArgumentException();", + " }", + " List l = new ArrayList<>();", + " l.forEach(v -> System.out.println(v + this.f.toString()));", + " Iterable l2 = l;", + " l2.forEach(v -> System.out.println(v + this.f.toString()));", + " this.f = null;", + " // BUG: Diagnostic contains: dereferenced expression this.f is @Nullable", + " l2.forEach(v -> System.out.println(v + this.f.toString()));", + " }", + "}") + .doTest(); + } + + @Test + public void removeIf() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.List;", + "import java.util.ArrayList;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable Object f;", + " public void test1() {", + " if (this.f == null) {", + " throw new IllegalArgumentException();", + " }", + " List l = new ArrayList<>();", + " l.removeIf(v -> this.f.toString().equals(v.toString()));", + " }", + "}") + .doTest(); + } + + @Test + public void streamMethods() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import java.util.List;", + "import java.util.ArrayList;", + "import org.jspecify.annotations.Nullable;", + "public class Test {", + " private @Nullable Object f;", + " public void test1() {", + " if (this.f == null) {", + " throw new IllegalArgumentException();", + " }", + " List l = new ArrayList<>();", + " // this.f being non-null gets propagated to all callback lambdas", + " l.stream().filter(v -> this.f.toString().equals(v.toString()))", + " .map(v -> this.f.toString())", + " .forEach(v -> System.out.println(this.f.hashCode() + v.toString()));", + " }", + "}") + .doTest(); + } +} diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java b/nullaway/src/test/java/com/uber/nullaway/ThriftTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java rename to nullaway/src/test/java/com/uber/nullaway/ThriftTests.java index 9753a5586b..a3180f2fe6 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/ThriftTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayThriftTests extends NullAwayTestsBase { +public class ThriftTests extends NullAwayTestsBase { @Test public void testThriftIsSet() { defaultCompilationHelper diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java b/nullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.java similarity index 85% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java rename to nullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.java index 5e13b02313..79d4ea5c0e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.java @@ -27,7 +27,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class NullAwayTypeUseAnnotationsTests extends NullAwayTestsBase { +public class TypeUseAnnotationsTests extends NullAwayTestsBase { @Test public void annotationAppliedToTypeParameter() { @@ -230,4 +230,37 @@ public void typeUseAnnotationOnInnerMultiLevel() { "}") .doTest(); } + + @Test + public void typeParameterAnnotationIsDistinctFromMethodReturnAnnotation() { + defaultCompilationHelper + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.function.Supplier;", + "public class Test {", + " public static @Nullable Supplier<@Nullable R> getNullableSupplierOfNullable() {", + " return new Supplier() {", + " @Nullable", + " public R get() { return null; }", + " };", + " }", + " public static Supplier<@Nullable R> getNonNullSupplierOfNullable() {", + " return new Supplier() {", + " @Nullable", + " public R get() { return null; }", + " };", + " }", + " public static String test1() {", + " // BUG: Diagnostic contains: dereferenced expression getNullableSupplierOfNullable() is @Nullable", + " return getNullableSupplierOfNullable().toString();", + " }", + " public static String test2() {", + " // The supplier contains null, but isn't itself nullable. Check against a past FP", + " return getNonNullSupplierOfNullable().toString();", + " }", + "}") + .doTest(); + } } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java b/nullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java rename to nullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java index 32e844ac5c..992cbff0d0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/UnannotatedTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayUnannotatedTests extends NullAwayTestsBase { +public class UnannotatedTests extends NullAwayTestsBase { @Test public void coreNullabilitySkipClass() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java b/nullaway/src/test/java/com/uber/nullaway/UnsoundnessTests.java similarity index 95% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java rename to nullaway/src/test/java/com/uber/nullaway/UnsoundnessTests.java index a765b1346b..150603262e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/UnsoundnessTests.java @@ -6,7 +6,7 @@ import org.junit.Test; /** Unit tests showing cases where NullAway is unsound. Useful for documentation purposes. */ -public class NullAwayUnsoundnessTests extends NullAwayTestsBase { +public class UnsoundnessTests extends NullAwayTestsBase { @Before @Override diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java rename to nullaway/src/test/java/com/uber/nullaway/VarargsTests.java index 63a6a9dfde..f404668695 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/VarargsTests.java @@ -3,7 +3,7 @@ import java.util.Arrays; import org.junit.Test; -public class NullAwayVarargsTests extends NullAwayTestsBase { +public class VarargsTests extends NullAwayTestsBase { @Test public void testNonNullVarargs() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java similarity index 57% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java rename to nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java index e16420cce4..108036fac3 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyArrayTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/ArrayTests.java @@ -1,10 +1,11 @@ -package com.uber.nullaway; +package com.uber.nullaway.jspecify; import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; import java.util.Arrays; import org.junit.Test; -public class NullAwayJSpecifyArrayTests extends NullAwayTestsBase { +public class ArrayTests extends NullAwayTestsBase { @Test public void arrayTopLevelAnnotationDereference() { @@ -212,6 +213,149 @@ public void nullableAssignmentParameterArray() { .doTest(); } + @Test + public void arraySubtyping() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static void test(@Nullable Integer[] nullableIntArr, Integer[] nonnullIntArr) {", + " // legal", + " Integer[] x1 = nonnullIntArr;", + " // legal", + " @Nullable Integer[] x2 = nullableIntArr;", + " // legal (covariant array subtypes)", + " x2 = nonnullIntArr;", + " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]", + " x1 = nullableIntArr;", + " }", + "}") + .doTest(); + } + + @Test + public void arraySubtypingWithNewExpression() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static void test() {", + " // legal", + " Integer[] x1 = new Integer[0];", + " // legal (covariant array subtypes)", + " @Nullable Integer[] x2 = new Integer[0];", + " // legal", + " x2 = new @Nullable Integer[0];", + " // BUG: Diagnostic contains: Cannot assign from type @Nullable Integer[] to type Integer[]", + " x1 = new @Nullable Integer[0];", + " }", + "}") + .doTest(); + } + + @Test + public void arraysAndGenerics() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.List;", + "class Test {", + " void foo(List<@Nullable Integer[]> l) {}", + " void testPositive(List p) {", + " // BUG: Diagnostic contains: Cannot pass parameter of type List", + " foo(p);", + " }", + " void testNegative(List<@Nullable Integer[]> p) {", + " foo(p);", + " }", + "}") + .doTest(); + } + + @Test + public void overridesReturnType() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.List;", + "class Test {", + " class Super {", + " @Nullable Integer[] foo() { return new @Nullable Integer[0]; }", + " Integer[] bar() { return new Integer[0]; }", + " }", + " class Sub extends Super {", + " @Override", + " Integer[] foo() { return new Integer[0]; }", + " @Override", + " // BUG: Diagnostic contains: Method returns @Nullable Integer[], but overridden method returns Integer[]", + " @Nullable Integer[] bar() { return new @Nullable Integer[0]; }", + " }", + "}") + .doTest(); + } + + @Test + public void overridesParameterType() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.util.List;", + "class Test {", + " class Super {", + " void foo(@Nullable Integer[] p) { }", + " void bar(Integer[] p) { }", + " }", + " class Sub extends Super {", + " @Override", + " // BUG: Diagnostic contains: Parameter has type Integer[], but overridden method has parameter type @Nullable Integer[]", + " void foo(Integer[] p) { }", + " @Override", + " void bar(@Nullable Integer[] p) { }", + " }", + "}") + .doTest(); + } + + @Test + public void ternaryOperator() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static Integer[] testPositive(Integer[] p, boolean t) {", + " // BUG: Diagnostic contains: Conditional expression must have type Integer[]", + " Integer[] t1 = t ? new Integer[0] : new @Nullable Integer[0];", + " // BUG: Diagnostic contains: Conditional expression must have type", + " return t ? new @Nullable Integer[0] : new @Nullable Integer[0];", + " }", + " static void testPositiveTernaryMethodArgument(boolean t) {", + " // BUG: Diagnostic contains: Conditional expression must have type", + " Integer[] a = testPositive(t ? new Integer[0] : new @Nullable Integer[0], t);", + " }", + " static @Nullable Integer[] testNegative(boolean n) {", + " @Nullable Integer[] t1 = n ? new @Nullable Integer[0] : new @Nullable Integer[0];", + " @Nullable Integer[] t2 = n ? new Integer[0] : new @Nullable Integer[0];", + " return n ? new @Nullable Integer[0] : new @Nullable Integer[0];", + " }", + " static void testNegativeTernaryMethodArgument(boolean t) {", + " Integer[] a = testPositive(t ? new Integer[0] : new Integer[0], t);", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java similarity index 90% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericMethodTests.java rename to nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java index e46172fa94..26ecc8eca3 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericMethodTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java @@ -1,11 +1,12 @@ -package com.uber.nullaway; +package com.uber.nullaway.jspecify; import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; import java.util.Arrays; import org.junit.Ignore; import org.junit.Test; -public class NullAwayJSpecifyGenericMethodTests extends NullAwayTestsBase { +public class GenericMethodTests extends NullAwayTestsBase { @Test @Ignore("requires generic method support") diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java rename to nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index 91063e4160..1fac4d03a0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1,11 +1,12 @@ -package com.uber.nullaway; +package com.uber.nullaway.jspecify; import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; import java.util.Arrays; import org.junit.Ignore; import org.junit.Test; -public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase { +public class GenericsTests extends NullAwayTestsBase { @Test public void basicTypeParamInstantiation() { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java rename to nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java index 246d7e8a00..2024a8313f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/NullMarkednessTests.java @@ -1,9 +1,10 @@ -package com.uber.nullaway; +package com.uber.nullaway.jspecify; +import com.uber.nullaway.NullAwayTestsBase; import java.util.Arrays; import org.junit.Test; -public class NullAwayJSpecifyTests extends NullAwayTestsBase { +public class NullMarkednessTests extends NullAwayTestsBase { @Test public void nullMarkedPackageLevel() { diff --git a/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java b/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/GrpcTest.java similarity index 99% rename from nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java rename to nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/GrpcTest.java index cded75fbe9..256f8eddc9 100644 --- a/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/GrpcTest.java @@ -35,7 +35,7 @@ /** Unit tests for {@link com.uber.nullaway.NullAway}. */ @RunWith(JUnit4.class) @SuppressWarnings("CheckTestExtendsBaseClass") -public class NullAwayGrpcTest { +public class GrpcTest { @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder(); diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayStreamSupportPositiveCases.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayStreamSupportPositiveCases.java index 82a12a00b9..4427260539 100644 --- a/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayStreamSupportPositiveCases.java +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/NullAwayStreamSupportPositiveCases.java @@ -191,7 +191,8 @@ private Stream test1(Stream stream) { private Stream test2(Stream stream) { Preconditions.checkNotNull(ref); - // BUG: Diagnostic contains: dereferenced expression ref is @Nullable + // no error since we propagate nullability facts to stream callbacks, which + // in sane code are invoked soon after the stream is created return stream.filter(s -> ref.equals(s)); } } diff --git a/nullaway/src/test/resources/com/uber/nullaway/testdata/unannotated/MinimalUnannotatedClass.java b/nullaway/src/test/resources/com/uber/nullaway/testdata/unannotated/MinimalUnannotatedClass.java index e78330af7f..537a587cf1 100644 --- a/nullaway/src/test/resources/com/uber/nullaway/testdata/unannotated/MinimalUnannotatedClass.java +++ b/nullaway/src/test/resources/com/uber/nullaway/testdata/unannotated/MinimalUnannotatedClass.java @@ -22,8 +22,10 @@ package com.uber.nullaway.testdata.unannotated; +import com.uber.nullaway.SerializationTest; + /** - * A minimal class, used from {@link com.uber.nullaway.NullAwaySerializationTest} to avoid extra + * A minimal class, used from {@link SerializationTest} to avoid extra * fixes. */ public class MinimalUnannotatedClass {