From cacadbe2d16b5cafaeb0374b1ce95f156f59300d Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Thu, 28 Sep 2023 19:41:07 +0530 Subject: [PATCH 1/7] Prepare for JDK 21 Added test for null case (covering #831) and changed JDK 17 to JDK 21 for unit testing --- code-coverage-report/build.gradle | 6 ++--- .../build.gradle | 4 +-- .../jdk17/NullAwayInstanceOfBindingTests.java | 0 .../jdk17/NullAwayModuleInfoTests.java | 0 .../jdk17/NullAwayOptionalEmptyTests.java | 0 .../nullaway/jdk17/NullAwayRecordTests.java | 0 .../nullaway/jdk17/NullAwaySwitchTests.java | 27 +++++++++++++++++++ settings.gradle | 2 +- 8 files changed, 33 insertions(+), 6 deletions(-) rename {jdk17-unit-tests => jdk-recent-unit-tests}/build.gradle (94%) rename {jdk17-unit-tests => jdk-recent-unit-tests}/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java (100%) rename {jdk17-unit-tests => jdk-recent-unit-tests}/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java (100%) rename {jdk17-unit-tests => jdk-recent-unit-tests}/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java (100%) rename {jdk17-unit-tests => jdk-recent-unit-tests}/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java (100%) rename {jdk17-unit-tests => jdk-recent-unit-tests}/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java (87%) diff --git a/code-coverage-report/build.gradle b/code-coverage-report/build.gradle index e9d3da6bcd..7036e17c86 100644 --- a/code-coverage-report/build.gradle +++ b/code-coverage-report/build.gradle @@ -20,12 +20,12 @@ plugins { id 'jacoco' } -// Use JDK 17 for this module, via a toolchain. We need JDK 17 since this module -// depends on jdk17-unit-tests. +// Use JDK 21 for this module, via a toolchain. We need JDK 21 since this module +// depends on jdk-recent-unit-tests. // We must null out sourceCompatibility and targetCompatibility to use toolchains. java.sourceCompatibility = null java.targetCompatibility = null -java.toolchain.languageVersion.set JavaLanguageVersion.of(17) +java.toolchain.languageVersion.set JavaLanguageVersion.of(21) // A resolvable configuration to collect source code def sourcesPath = configurations.create("sourcesPath") { diff --git a/jdk17-unit-tests/build.gradle b/jdk-recent-unit-tests/build.gradle similarity index 94% rename from jdk17-unit-tests/build.gradle rename to jdk-recent-unit-tests/build.gradle index 1c9f7f908d..7082d035a9 100644 --- a/jdk17-unit-tests/build.gradle +++ b/jdk-recent-unit-tests/build.gradle @@ -18,11 +18,11 @@ plugins { id 'nullaway.java-test-conventions' } -// Use JDK 17 for this module, via a toolchain +// Use JDK 21 for this module, via a toolchain // We must null out sourceCompatibility and targetCompatibility to use toolchains. java.sourceCompatibility = null java.targetCompatibility = null -java.toolchain.languageVersion.set JavaLanguageVersion.of(17) +java.toolchain.languageVersion.set JavaLanguageVersion.of(21) configurations { // We use this configuration to expose a module path that can be diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java similarity index 100% rename from jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayInstanceOfBindingTests.java diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java similarity index 100% rename from jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayModuleInfoTests.java diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java similarity index 100% rename from jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayOptionalEmptyTests.java diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java similarity index 100% rename from jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwayRecordTests.java diff --git a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java similarity index 87% rename from jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java rename to jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java index faeb145b64..ff9e909e0e 100644 --- a/jdk17-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java +++ b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java @@ -25,6 +25,7 @@ import com.uber.nullaway.NullAway; import java.util.Arrays; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -174,4 +175,30 @@ public void testSwitchExprLambda() { "}") .doTest(); } + + @Ignore + @Test + public void testSwitchExprNullCase() { + defaultCompilationHelper + .addSourceLines( + "SwitchExpr.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "class SwitchExpr {", + " public enum NullableEnum {", + " a,", + " b,", + " }", + " // NOTE: we should report a bug here for nullableEn but cannot do so until", + " // Error Prone supports matching switch expressions", + " static Object handleNullableEnum(@Nullable NullableEnum nullableEnum) {", + " return switch (nullableEnum) {", + " case a -> new Object();", + " case b -> new Object();", + " case null -> throw new IllegalArgumentException(\"NullableEnum parameter is required\");", + " };", + " }", + "}") + .doTest(); + } } diff --git a/settings.gradle b/settings.gradle index b1fe598c56..bfafe10aff 100644 --- a/settings.gradle +++ b/settings.gradle @@ -23,7 +23,7 @@ include ':jar-infer:test-java-lib-jarinfer' include ':jar-infer:nullaway-integration-test' include ':jmh' include ':guava-recent-unit-tests' -include ':jdk17-unit-tests' +include ':jdk-recent-unit-tests' include ':code-coverage-report' include ':sample-app' include ':jar-infer:test-android-lib-jarinfer' From 4ecb46e9aa6d4adfb1c21091144d4209e2a4e972 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Thu, 28 Sep 2023 19:44:29 +0530 Subject: [PATCH 2/7] Fix typo --- .../test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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/NullAwaySwitchTests.java index ff9e909e0e..c7a9799551 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/NullAwaySwitchTests.java @@ -189,7 +189,7 @@ public void testSwitchExprNullCase() { " a,", " b,", " }", - " // NOTE: we should report a bug here for nullableEn but cannot do so until", + " // NOTE: we should report a bug here for nullableEnum but cannot do so until", " // Error Prone supports matching switch expressions", " static Object handleNullableEnum(@Nullable NullableEnum nullableEnum) {", " return switch (nullableEnum) {", From 3f64acf0250e9e4c08821695e2ce85e526fa030d Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Thu, 28 Sep 2023 19:50:49 +0530 Subject: [PATCH 3/7] Fixed code coverage report Changed module name --- code-coverage-report/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code-coverage-report/build.gradle b/code-coverage-report/build.gradle index 7036e17c86..3e5420f298 100644 --- a/code-coverage-report/build.gradle +++ b/code-coverage-report/build.gradle @@ -79,5 +79,5 @@ dependencies { implementation project(':jar-infer:jar-infer-lib') implementation project(':jar-infer:nullaway-integration-test') implementation project(':guava-recent-unit-tests') - implementation project(':jdk17-unit-tests') + implementation project(':jdk-recent-unit-tests') } From 2ee89d6dccaebcdbe442273e11eb51d96eefbe09 Mon Sep 17 00:00:00 2001 From: Md Armughanuddin Date: Thu, 28 Sep 2023 20:33:16 +0530 Subject: [PATCH 4/7] Update NullAwaySwitchTests.java --- .../test/java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java | 3 +++ 1 file changed, 3 insertions(+) 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/NullAwaySwitchTests.java index c7a9799551..bc9eaa4031 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/NullAwaySwitchTests.java @@ -176,6 +176,9 @@ public void testSwitchExprLambda() { .doTest(); } + // Ignoring this unit test to prevent crashes until Checker Framework + // can handle null case. + @Ignore @Test public void testSwitchExprNullCase() { From a9ca068b1966c23f8351a578b4bcd48f33c0534c Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 09:14:24 -0700 Subject: [PATCH 5/7] shift comment --- .../java/com/uber/nullaway/jdk17/NullAwaySwitchTests.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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/NullAwaySwitchTests.java index bc9eaa4031..4af5ab14f4 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/NullAwaySwitchTests.java @@ -176,10 +176,7 @@ public void testSwitchExprLambda() { .doTest(); } - // Ignoring this unit test to prevent crashes until Checker Framework - // can handle null case. - - @Ignore + @Ignore("requires fix for crash in Checker dataflow library") @Test public void testSwitchExprNullCase() { defaultCompilationHelper From 4003737ab4e03be78d7f7f4196b05fe5f3ae7e69 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 09:22:23 -0700 Subject: [PATCH 6/7] tweaks to test --- .../com/uber/nullaway/jdk17/NullAwaySwitchTests.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) 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/NullAwaySwitchTests.java index 4af5ab14f4..6b7a1d28fe 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/NullAwaySwitchTests.java @@ -186,15 +186,13 @@ public void testSwitchExprNullCase() { "import javax.annotation.Nullable;", "class SwitchExpr {", " public enum NullableEnum {", - " a,", - " b,", + " A,", + " B,", " }", - " // NOTE: we should report a bug here for nullableEnum but cannot do so until", - " // Error Prone supports matching switch expressions", " static Object handleNullableEnum(@Nullable NullableEnum nullableEnum) {", " return switch (nullableEnum) {", - " case a -> new Object();", - " case b -> new Object();", + " case A -> new Object();", + " case B -> new Object();", " case null -> throw new IllegalArgumentException(\"NullableEnum parameter is required\");", " };", " }", From f65a1c0286c3541011bec77d37600460261b2268 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Thu, 28 Sep 2023 09:24:06 -0700 Subject: [PATCH 7/7] disable testJdk21 task --- jdk-recent-unit-tests/build.gradle | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/jdk-recent-unit-tests/build.gradle b/jdk-recent-unit-tests/build.gradle index 7082d035a9..b5573a0e2a 100644 --- a/jdk-recent-unit-tests/build.gradle +++ b/jdk-recent-unit-tests/build.gradle @@ -52,9 +52,6 @@ test { } tasks.getByName('testJdk21').configure { - jvmArgs += [ - // Expose a module path for tests as a JVM property. - // Used by com.uber.nullaway.jdk17.NullAwayModuleInfoTests - "-Dtest.module.path=${configurations.testModulePath.asPath}" - ] + // We don't need this task since we already run the tests on JDK 21 + onlyIf { false } }