From 719b167ded3f65c4eb454d1909f436e9dd514076 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 22 Apr 2024 11:10:25 -0700 Subject: [PATCH 01/13] Rename test classes (#951) This is a pure refactoring PR that only touches test code and does the following: * Renames test classes to remove the `NullAway` prefix, which hinders readability * Moves JSpecify tests into their own package * Remove `nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java` and move its one test into `nullaway/src/test/java/com/uber/nullaway/TypeUseAnnotationsTests.java` (this must have been a typo before) --- ...Tests.java => InstanceOfBindingTests.java} | 2 +- ...uleInfoTests.java => ModuleInfoTests.java} | 2 +- ...mptyTests.java => OptionalEmptyTests.java} | 2 +- ...lAwayRecordTests.java => RecordTests.java} | 2 +- ...lAwaySwitchTests.java => SwitchTests.java} | 2 +- nullaway/build.gradle | 4 +- ...sPathsTests.java => AccessPathsTests.java} | 2 +- ...knowledgeRestrictiveAnnotationsTests.java} | 2 +- ...lAwayAndroidTest.java => AndroidTest.java} | 2 +- ...LibsTests.java => AssertionLibsTests.java} | 2 +- ...stTest.java => AutoSuggestNoCastTest.java} | 2 +- ...oSuggestTest.java => AutoSuggestTest.java} | 2 +- ...ts.java => BytecodeInteractionsTests.java} | 2 +- ...nTests.java => ContractsBooleanTests.java} | 2 +- ...ontractsTests.java => ContractsTests.java} | 2 +- ...{NullAwayCoreTests.java => CoreTests.java} | 2 +- ...sts.java => CustomLibraryModelsTests.java} | 2 +- ...ullTests.java => EnsuresNonNullTests.java} | 2 +- ...rameworkTests.java => FrameworkTests.java} | 2 +- ... FunctionalInterfaceNullabilityTests.java} | 2 +- ...nsTests.java => GuavaAssertionsTests.java} | 2 +- ...ionTests.java => InitializationTests.java} | 2 +- ...ullAwayJava8Tests.java => Java8Tests.java} | 2 +- ...torTests.java => KeySetIteratorTests.java} | 2 +- .../NullAwayTypeUseAnnotationTests.java | 38 ------------------- ...Tests.java => OptionalEmptinessTests.java} | 2 +- ...zationTest.java => SerializationTest.java} | 4 +- ...lAwayThriftTests.java => ThriftTests.java} | 2 +- ...ests.java => TypeUseAnnotationsTests.java} | 35 ++++++++++++++++- ...otatedTests.java => UnannotatedTests.java} | 2 +- ...ndnessTests.java => UnsoundnessTests.java} | 2 +- ...wayVarargsTests.java => VarargsTests.java} | 2 +- .../ArrayTests.java} | 5 ++- .../GenericMethodTests.java} | 5 ++- .../GenericsTests.java} | 5 ++- .../NullMarkednessTests.java} | 5 ++- .../{NullAwayGrpcTest.java => GrpcTest.java} | 2 +- .../unannotated/MinimalUnannotatedClass.java | 4 +- 38 files changed, 82 insertions(+), 81 deletions(-) rename jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/{NullAwayInstanceOfBindingTests.java => InstanceOfBindingTests.java} (96%) rename jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/{NullAwayModuleInfoTests.java => ModuleInfoTests.java} (97%) rename jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/{NullAwayOptionalEmptyTests.java => OptionalEmptyTests.java} (99%) rename jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/{NullAwayRecordTests.java => RecordTests.java} (99%) rename jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/{NullAwaySwitchTests.java => SwitchTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAccessPathsTests.java => AccessPathsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAcknowledgeRestrictiveAnnotationsTests.java => AcknowledgeRestrictiveAnnotationsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAndroidTest.java => AndroidTest.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAssertionLibsTests.java => AssertionLibsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAutoSuggestNoCastTest.java => AutoSuggestNoCastTest.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayAutoSuggestTest.java => AutoSuggestTest.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayBytecodeInteractionsTests.java => BytecodeInteractionsTests.java} (97%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayContractsBooleanTests.java => ContractsBooleanTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayContractsTests.java => ContractsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayCoreTests.java => CoreTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayCustomLibraryModelsTests.java => CustomLibraryModelsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayEnsuresNonNullTests.java => EnsuresNonNullTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayFrameworkTests.java => FrameworkTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayFunctionalInterfaceNullabilityTests.java => FunctionalInterfaceNullabilityTests.java} (98%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayGuavaAssertionsTests.java => GuavaAssertionsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayInitializationTests.java => InitializationTests.java} (98%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayJava8Tests.java => Java8Tests.java} (96%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayKeySetIteratorTests.java => KeySetIteratorTests.java} (98%) delete mode 100644 nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java rename nullaway/src/test/java/com/uber/nullaway/{NullAwayOptionalEmptinessTests.java => OptionalEmptinessTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwaySerializationTest.java => SerializationTest.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayThriftTests.java => ThriftTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayTypeUseAnnotationsTests.java => TypeUseAnnotationsTests.java} (85%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayUnannotatedTests.java => UnannotatedTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayUnsoundnessTests.java => UnsoundnessTests.java} (95%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayVarargsTests.java => VarargsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayJSpecifyArrayTests.java => jspecify/ArrayTests.java} (98%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayJSpecifyGenericMethodTests.java => jspecify/GenericMethodTests.java} (90%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayJSpecifyGenericsTests.java => jspecify/GenericsTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/{NullAwayJSpecifyTests.java => jspecify/NullMarkednessTests.java} (99%) rename nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/{NullAwayGrpcTest.java => GrpcTest.java} (99%) 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..77cccedb30 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -124,9 +124,9 @@ def jdk8Test = tasks.register("testJdk8", Test) { jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}" filter { // JDK 8 does not support diamonds on anonymous classes - excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests.overrideDiamondAnonymousClass" + excludeTestsMatching "com.uber.nullaway.jspecify.GenericsTests.overrideDiamondAnonymousClass" // tests cannot run on JDK 8 since Mockito version no longer supports it - excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.initializationError" + excludeTestsMatching "com.uber.nullaway.SerializationTest.initializationError" excludeTestsMatching "com.uber.nullaway.handlers.contract.ContractUtilsTest.getEmptyAntecedent" } } 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/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 98% 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..f4074ec686 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() { 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/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 { From edcfe49ea5c305cc85b55878f3d5977e17bee2a4 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 29 Apr 2024 20:03:05 -0700 Subject: [PATCH 02/13] Propagate more nullability info to lambdas known to be invoked synchronously (#952) Fixes #941 We propagate full nullability info from the enclosing context to callbacks passed to `Map.forEach`, `Iterable.forEach`, `List.removeIf`, and all methods on `java.util.stream.Stream` --- .../java/com/uber/nullaway/LibraryModels.java | 2 +- .../main/java/com/uber/nullaway/NullAway.java | 3 +- .../dataflow/AccessPathNullnessAnalysis.java | 19 +- .../handlers/AccessPathPredicates.java | 25 +++ .../nullaway/handlers/BaseNoOpHandler.java | 7 +- .../nullaway/handlers/CompositeHandler.java | 25 ++- .../com/uber/nullaway/handlers/Handler.java | 15 +- .../com/uber/nullaway/handlers/Handlers.java | 1 + .../handlers/OptionalEmptinessHandler.java | 22 +-- .../handlers/SynchronousCallbackHandler.java | 94 ++++++++++ .../com/uber/nullaway/SyncLambdasTests.java | 166 ++++++++++++++++++ .../NullAwayStreamSupportPositiveCases.java | 3 +- 12 files changed, 350 insertions(+), 32 deletions(-) create mode 100644 nullaway/src/main/java/com/uber/nullaway/handlers/AccessPathPredicates.java create mode 100644 nullaway/src/main/java/com/uber/nullaway/handlers/SynchronousCallbackHandler.java create mode 100644 nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java 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..ab88cf7c1d 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( 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/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..e8a70bc855 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java @@ -50,6 +50,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Predicate; import javax.annotation.Nullable; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; @@ -164,17 +165,18 @@ private boolean isOptionalContentNullable( } @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; + public Predicate getAccessPathPredicateForNestedMethod( + TreePath path, VisitorState state) { + return ap -> { + if (ap.getElements().size() == 1) { + final Element e = ap.getRoot(); + if (e != null) { + return e.getKind().equals(ElementKind.LOCAL_VARIABLE) + && ap.getElements().get(0).getJavaElement() instanceof OptionalContentVariableElement; + } } - } - return false; + return false; + }; } private void handleTestAssertions( 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/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/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)); } } From 2a3cd47699412d3f29cdfca77f810cb78023b145 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Fri, 3 May 2024 09:06:08 -0400 Subject: [PATCH 03/13] Prepare for release 0.10.26. --- CHANGELOG.md | 6 ++++++ gradle.properties | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06235efa7c..11a11cbc5b 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ Changelog ========= +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/gradle.properties b/gradle.properties index 429211e021..8ec342c0a3 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.10.26 POM_DESCRIPTION=A fast annotation-based null checker for Java From a017158303e0b138f748e1e88497e932a5a7def2 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Fri, 3 May 2024 09:09:44 -0400 Subject: [PATCH 04/13] Prepare next development version. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 8ec342c0a3..3c2a47b4f8 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 +VERSION_NAME=0.10.27-SNAPSHOT POM_DESCRIPTION=A fast annotation-based null checker for Java From c26ae17ca7f49d08867647bcf42690e6e87bc0b7 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 13 May 2024 18:33:13 -0700 Subject: [PATCH 05/13] Delete OptionalEmptinessHandler method that is no longer needed (#954) Fixes #953 --- .../handlers/OptionalEmptinessHandler.java | 16 ---------------- 1 file changed, 16 deletions(-) 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 e8a70bc855..4e1858b050 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java @@ -50,7 +50,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.Consumer; -import java.util.function.Predicate; import javax.annotation.Nullable; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; @@ -164,21 +163,6 @@ private boolean isOptionalContentNullable( == Nullness.NULLABLE; } - @Override - public Predicate getAccessPathPredicateForNestedMethod( - TreePath path, VisitorState state) { - return ap -> { - if (ap.getElements().size() == 1) { - final Element e = ap.getRoot(); - if (e != null) { - return e.getKind().equals(ElementKind.LOCAL_VARIABLE) - && ap.getElements().get(0).getJavaElement() instanceof OptionalContentVariableElement; - } - } - return false; - }; - } - private void handleTestAssertions( VisitorState state, AccessPath.AccessPathContext apContext, From e0d5c6b5b1631d45cde1576d3c36abb8f15f774e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 13 May 2024 19:56:35 -0700 Subject: [PATCH 06/13] Refactor PreservedAnnotationTreeVisitor (#955) We extract some code from `visitParameterizedType` into a proper `visitAnnotatedType` method. This will be useful for an upcoming PR that handles `AnnotatedTypeTree`s in new array expressions. The change introduces some complexity due to API changes in JDK 21, which we work around through further use of `MethodHandle`s. --- .../PreservedAnnotationTreeVisitor.java | 130 ++++++++++++------ 1 file changed, 88 insertions(+), 42 deletions(-) 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..2bf38d89bc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java @@ -2,7 +2,6 @@ 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; @@ -12,6 +11,7 @@ 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; @@ -44,52 +44,42 @@ public Type visitArrayType(ArrayTypeTree tree, Void p) { @Override public Type visitParameterizedType(ParameterizedTypeTree tree, Void p) { - Type.ClassType type = (Type.ClassType) ASTHelpers.getType(tree); - Preconditions.checkNotNull(type); - Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state); + Type.ClassType baseType = (Type.ClassType) tree.getType().accept(this, null); List 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 +94,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 +121,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 +139,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 +160,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 +213,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 +239,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. */ From f4c4734373811418de06255205a30c9b32c01c97 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 17 May 2024 07:36:52 -0700 Subject: [PATCH 07/13] Update to Error Prone 2.27.1 (#957) --- .github/workflows/continuous-integration.yml | 10 +++++----- gradle/dependencies.gradle | 2 +- .../uber/nullaway/dataflow/cfg/NullAwayCFGBuilder.java | 2 +- .../handlers/contract/ContractCheckHandler.java | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 2473f17ba3..3043a6821a 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -21,16 +21,16 @@ jobs: epVersion: 2.10.0 - os: macos-latest java: 11 - epVersion: 2.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/gradle/dependencies.gradle b/gradle/dependencies.gradle index 596096adc8..54a5e3ef8f 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -19,7 +19,7 @@ import org.gradle.util.VersionNumber // The oldest version of Error Prone that we support running on def oldestErrorProneVersion = "2.10.0" // Latest released Error Prone version that we've tested with -def latestErrorProneVersion = "2.26.1" +def latestErrorProneVersion = "2.27.1" // Default to using latest tested Error Prone version def defaultErrorProneVersion = latestErrorProneVersion def errorProneVersionToCompileAgainst = defaultErrorProneVersion 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/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); } From e26e194c98a10f2e0eca1b6b154f222efa9acc32 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 17 May 2024 11:30:12 -0700 Subject: [PATCH 08/13] JSpecify subtyping checks for arrays (#956) Add checks for proper array subtyping (including nullability of array contents) at pseudo-assignments and for method overriding. We do not yet handle multi-dimensional arrays. Note that JSpecify allows for covariant array subtyping with respect to nullability (see https://github.com/jspecify/jspecify/issues/65) which complicates the implementation a bit. Also some hacks are required since javac does not propagate the array contents nullability annotation into the types of the relevant trees in all cases. --- .../main/java/com/uber/nullaway/NullAway.java | 2 +- ... => CheckIdenticalNullabilityVisitor.java} | 43 ++---- .../nullaway/generics/GenericsChecks.java | 133 ++++++++++++---- .../PreservedAnnotationTreeVisitor.java | 7 + .../uber/nullaway/jspecify/ArrayTests.java | 143 ++++++++++++++++++ 5 files changed, 271 insertions(+), 57 deletions(-) rename nullaway/src/main/java/com/uber/nullaway/generics/{CompareNullabilityVisitor.java => CheckIdenticalNullabilityVisitor.java} (62%) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index ab88cf7c1d..e5de3a9e1b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -495,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/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 2bf38d89bc..42797b0ff6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java @@ -7,6 +7,7 @@ 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; @@ -36,6 +37,12 @@ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor 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( From 5d3e2642f3a1cc32524aefe01bd445a2a21cc06e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 20 May 2024 12:12:49 -0700 Subject: [PATCH 09/13] Bump to Checker Framework 3.43.0 (#959) The previous bug that was preventing us from upgrading (https://github.com/typetools/checker-framework/issues/6396) is fixed in this release. Benchmarks show no performance regression. --- gradle/dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 54a5e3ef8f..e280f90875 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -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. From 2f9bd83923938ffca98da5b253c8861e204c9dfd Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 10:53:24 -0400 Subject: [PATCH 10/13] Drop Java 8 support (#961) This PR drops support for running NullAway on a JDK 8 JVM. After it lands, running NullAway will require JDK 11 or above (like current versions of Error Prone). There are various cleanups that this change enables, but to keep the PR small, I tried to do close to the minimum. In particular: * I updated the minimum supported Error Prone version to 2.14.0, which is ~2 years old. Depending on what we decide about policy (see #882) we may want to bump this to an even more recent version. * Update our JDK 8 test tasks to instead just test building with the most recent supported Error Prone version but running on the oldest supported version. I tested locally that this does still detect binary compatibility issues. * Remove references to Java 11 in the jarinfer build files, as we now use that version everywhere. We will have to change the required CI job names before landing this. Will do other enabled cleanups in separate PRs. --- .github/workflows/continuous-integration.yml | 4 +- README.md | 2 +- build.gradle | 8 +-- gradle.properties | 2 +- gradle/dependencies.gradle | 2 +- guava-recent-unit-tests/build.gradle | 46 +++++++++--- jar-infer/jar-infer-cli/build.gradle | 6 -- jar-infer/jar-infer-lib/build.gradle | 6 -- nullaway/build.gradle | 73 ++++++++++---------- 9 files changed, 82 insertions(+), 67 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 3043a6821a..ee76b9b8ac 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -15,10 +15,10 @@ 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.27.1 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 3c2a47b4f8..36d35b785a 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.27-SNAPSHOT +VERSION_NAME=0.11.0-SNAPSHOT POM_DESCRIPTION=A fast annotation-based null checker for Java diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index e280f90875..1acb6179ed 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -17,7 +17,7 @@ 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.27.1" // Default to using latest tested Error Prone version 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/nullaway/build.gradle b/nullaway/build.gradle index 77cccedb30..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.jspecify.GenericsTests.overrideDiamondAnonymousClass" - // tests cannot run on JDK 8 since Mockito version no longer supports it - excludeTestsMatching "com.uber.nullaway.SerializationTest.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 From 4aff96a505259b73d762e6187c1020b236923634 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Fri, 31 May 2024 21:42:04 -0400 Subject: [PATCH 11/13] Prepare for release 0.11.0. --- CHANGELOG.md | 11 +++++++++++ gradle.properties | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11a11cbc5b..bb9c847ffb 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ 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) diff --git a/gradle.properties b/gradle.properties index 36d35b785a..f3f712b9dd 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.11.0-SNAPSHOT +VERSION_NAME=0.11.0 POM_DESCRIPTION=A fast annotation-based null checker for Java From 87ec10d4f26630d4bb91aefe5ff7c0fc181f030a Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Fri, 31 May 2024 21:46:24 -0400 Subject: [PATCH 12/13] Prepare next development version. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index f3f712b9dd..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.11.0 +VERSION_NAME=0.11.1-SNAPSHOT POM_DESCRIPTION=A fast annotation-based null checker for Java From 0ab6c22731c61546759f549eb1d3c32fd6db04de Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 3 Jun 2024 10:19:07 -0400 Subject: [PATCH 13/13] disable benchmarking (#965) --- .github/workflows/jmh-benchmark.yml | 73 ----------------------------- 1 file changed, 73 deletions(-) delete mode 100644 .github/workflows/jmh-benchmark.yml 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 - - -