diff --git a/nullaway/src/main/java/com/uber/nullaway/Config.java b/nullaway/src/main/java/com/uber/nullaway/Config.java index 5745666bc7..42997cd75f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Config.java +++ b/nullaway/src/main/java/com/uber/nullaway/Config.java @@ -285,6 +285,13 @@ public interface Config { */ boolean isJarInferEnabled(); + /** + * Checks if JSpecify JDK models should be enabled. + * + * @return true if JSpecify JDK models should be enabled, false otherwise + */ + boolean isJSpecifyJDKModels(); + /** * Gets the URL to show with NullAway error messages. * diff --git a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java index c6908e355a..1860ae1fc7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java @@ -204,6 +204,11 @@ public boolean isJarInferEnabled() { throw new IllegalStateException(ERROR_MESSAGE); } + @Override + public boolean isJSpecifyJDKModels() { + throw new IllegalStateException(ERROR_MESSAGE); + } + @Override public String getErrorURL() { throw new IllegalStateException(ERROR_MESSAGE); diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java index fbc1f8ff7d..d56618cadc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -87,6 +87,8 @@ final class ErrorProneCLIFlagsConfig implements Config { /** --- JarInfer configs --- */ static final String FL_JI_ENABLED = EP_FL_NAMESPACE + ":JarInferEnabled"; + static final String FL_JSPECIFY_JDK_ENABLED = EP_FL_NAMESPACE + ":JSpecifyJDKModels"; + static final String FL_ERROR_URL = EP_FL_NAMESPACE + ":ErrorURL"; /** --- Serialization configs --- */ @@ -247,6 +249,8 @@ final class ErrorProneCLIFlagsConfig implements Config { /** --- JarInfer configs --- */ private final boolean jarInferEnabled; + private final boolean jspecifyJDKModelsEnabled; + private final String errorURL; /** --- Fully qualified names of custom nonnull/nullable annotation --- */ @@ -333,6 +337,7 @@ final class ErrorProneCLIFlagsConfig implements Config { /* --- JarInfer configs --- */ jarInferEnabled = flags.getBoolean(FL_JI_ENABLED).orElse(false); + jspecifyJDKModelsEnabled = flags.getBoolean(FL_JSPECIFY_JDK_ENABLED).orElse(false); errorURL = flags.get(FL_ERROR_URL).orElse(DEFAULT_URL); if (acknowledgeAndroidRecent && !isAcknowledgeRestrictive) { throw new IllegalStateException( @@ -589,6 +594,11 @@ public boolean isJarInferEnabled() { return jarInferEnabled; } + @Override + public boolean isJSpecifyJDKModels() { + return jspecifyJDKModelsEnabled; + } + @Override public String getErrorURL() { return errorURL; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 2350302aba..61955133fc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -62,7 +62,9 @@ import com.uber.nullaway.librarymodel.AddAnnotationToNestedTypeVisitor; import com.uber.nullaway.librarymodel.NestedAnnotationInfo; import com.uber.nullaway.librarymodel.NestedAnnotationInfo.Annotation; +import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.HashSet; import java.util.LinkedHashMap; @@ -478,8 +480,9 @@ private static LibraryModels loadLibraryModels(Config config) { ServiceLoader.load(LibraryModels.class, LibraryModels.class.getClassLoader()); ImmutableSet.Builder libModelsBuilder = new ImmutableSet.Builder<>(); libModelsBuilder.add(new DefaultLibraryModels(config)).addAll(externalLibraryModels); - if (config.isJarInferEnabled()) { - libModelsBuilder.add(new ExternalStubxLibraryModels()); + if (config.isJarInferEnabled() || config.isJSpecifyJDKModels()) { + libModelsBuilder.add( + new ExternalStubxLibraryModels(config.isJarInferEnabled(), config.isJSpecifyJDKModels())); } return new CombinedLibraryModels(libModelsBuilder.build(), config); } @@ -1560,26 +1563,42 @@ private static class ExternalStubxLibraryModels implements LibraryModels { private final Multimap methodTypeParamNullableUpperBoundCache; private final Map> nestedAnnotationInfo; - ExternalStubxLibraryModels() { + ExternalStubxLibraryModels(boolean isJarInferEnabled, boolean isJSpecifyJDKEnabled) { String libraryModelLogName = "LM"; StubxCacheUtil cacheUtil = new StubxCacheUtil(libraryModelLogName); // hardcoded loading of stubx files from android-jarinfer-models-sdkXX artifacts - try { - InputStream androidStubxIS = - Class.forName(ANDROID_MODEL_CLASS) - .getClassLoader() - .getResourceAsStream(ANDROID_ASTUBX_LOCATION); - if (androidStubxIS != null) { - cacheUtil.parseStubStream(androidStubxIS, "android.jar: " + ANDROID_ASTUBX_LOCATION); - astubxLoadLog("Loaded Android RT models."); + if (isJarInferEnabled) { + try { + InputStream androidStubxIS = + Class.forName(ANDROID_MODEL_CLASS) + .getClassLoader() + .getResourceAsStream(ANDROID_ASTUBX_LOCATION); + if (androidStubxIS != null) { + cacheUtil.parseStubStream(androidStubxIS, "android.jar: " + ANDROID_ASTUBX_LOCATION); + astubxLoadLog("Loaded Android RT models."); + } + } catch (ClassNotFoundException e) { + astubxLoadLog( + "Cannot find Android RT models locator class." + + " This is expected if not in an Android project, or the Android SDK JarInfer models Jar has not been set up for this build."); + + } catch (Exception e) { + astubxLoadLog("Cannot load Android RT models."); } - } catch (ClassNotFoundException e) { - astubxLoadLog( - "Cannot find Android RT models locator class." - + " This is expected if not in an Android project, or the Android SDK JarInfer models Jar has not been set up for this build."); + } - } catch (Exception e) { - astubxLoadLog("Cannot load Android RT models."); + // hardcoded loading of stubx files from jdk nullness inferred output.astubx + if (isJSpecifyJDKEnabled) { + try (InputStream in = getClass().getClassLoader().getResourceAsStream("output.astubx")) { + if (in == null) { + astubxLoadLog("JDK astubx model not found on classpath: output.astubx"); + } else { + cacheUtil.parseStubStream(in, "output.astubx"); + astubxLoadLog("Loaded JDK astubx model."); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } } argAnnotCache = cacheUtil.getArgAnnotCache(); diff --git a/nullaway/src/main/resources/output.astubx b/nullaway/src/main/resources/output.astubx new file mode 100644 index 0000000000..1cdf4a7c14 Binary files /dev/null and b/nullaway/src/main/resources/output.astubx differ diff --git a/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java b/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java new file mode 100644 index 0000000000..682b27c932 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java @@ -0,0 +1,115 @@ +package com.uber.nullaway; + +import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.generics.JSpecifyJavacConfig; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class JSpecifyJDKModelsTest extends NullAwayTestsBase { + + @Test + public void modelsEnabledLoadsAstubxModel() { + makeTestHelperWithArgs( + List.of( + "-XepOpt:NullAway:AnnotatedPackages=foo", + "-XepOpt:NullAway:JSpecifyJDKModels=true")) + .addSourceLines( + "Test.java", + """ + package foo; + import javax.naming.directory.Attributes; + import org.jspecify.annotations.NullMarked; + @NullMarked + class Test { + void use(Attributes attrs) { + // BUG: Diagnostic contains: @Nullable + attrs.get("key").toString(); + } + } + """) + .doTest(); + } + + @Test + public void modelsDisabledDoesNotLoadAstubxModel() { + CompilationTestHelper compilationTestHelper = + makeTestHelperWithArgs(List.of("-XepOpt:NullAway:AnnotatedPackages=foo")) + .addSourceLines( + "Test.java", + """ + package foo; + import javax.naming.directory.Attributes; + import org.jspecify.annotations.NullMarked; + @NullMarked + class Test { + void use(Attributes attrs) { + attrs.get("key").toString(); + } + } + """); + compilationTestHelper.doTest(); + } + + @Test + public void listContainingNullsWithModel() { + makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + List.of( + "-XepOpt:NullAway:AnnotatedPackages=foo", + "-XepOpt:NullAway:JSpecifyJDKModels=true"))) + .addSourceLines( + "Test.java", + """ + package foo; + import java.util.List; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + void testNullableContents(List<@Nullable String> list) { + list.add(null); + // BUG: Diagnostic contains: dereferenced expression list.get(0) is @Nullable + list.get(0).toString(); + } + void testNonNullContents(List list) { + // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required + list.add(null); + list.get(0).toString(); + } + } + """) + .doTest(); + } + + @Test + public void listContainingNullsWithoutModel() { + makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + List.of("-XepOpt:NullAway:AnnotatedPackages=foo"))) + .addSourceLines( + "Test.java", + """ + package foo; + import java.util.List; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + void use(List<@Nullable String> list) { + list.add(null); + // no warning, since List.get() is unmarked without the model + list.get(0).toString(); + } + void testNonNullContents(List list) { + // no warning, since List.add() is unmarked without the model + list.add(null); + list.get(0).toString(); + } + } + """) + .doTest(); + } +}