From a354901705095fd34003e5dc800334c433729b3f Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 4 Oct 2024 15:58:29 -0700 Subject: [PATCH] Remove unused or unneeded JarInfer flags --- .../jarinfer/JarInferIntegrationTest.java | 3 +- .../libmodel/LibraryModelIntegrationTest.java | 26 +++++---------- .../main/java/com/uber/nullaway/Config.java | 22 ------------- .../com/uber/nullaway/DummyOptionsConfig.java | 16 --------- .../nullaway/ErrorProneCLIFlagsConfig.java | 30 ----------------- .../com/uber/nullaway/handlers/Handlers.java | 2 +- .../handlers/InferredJARModelsHandler.java | 33 ++++++++----------- 7 files changed, 25 insertions(+), 107 deletions(-) diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index f77ae54ae3..ba5259f5a9 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -101,8 +101,7 @@ public void jarinferNullableReturnsTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", diff --git a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java index 89dc76b332..b2220ac1b5 100644 --- a/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java +++ b/library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java @@ -32,8 +32,7 @@ public void libraryModelNullableReturnsTest() { "-d", temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -63,8 +62,7 @@ public void libraryModelNullableReturnsArrayTest() { "-d", temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -98,7 +96,7 @@ public void libraryModelWithoutJarInferEnabledTest() { " static void test(String value){", " }", " static void testNegative() {", - " // Since the JarInferEnabled and JarInferUseReturnAnnotations flags are not set, we don't get an error here", + " // Since the JarInferEnabled flag is not set, we don't get an error here", " test(annotationExample.makeUpperCase(\"nullaway\"));", " }", "}") @@ -113,8 +111,7 @@ public void libraryModelInnerClassNullableReturnsTest() { "-d", temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -140,8 +137,7 @@ public void libraryModelInnerClassNullableUpperBoundsTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -189,8 +185,7 @@ public void libraryModelDefaultParameterNullabilityTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -214,8 +209,7 @@ public void libraryModelParameterNullabilityTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -242,8 +236,7 @@ public void nullableArrayTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", @@ -269,8 +262,7 @@ public void genericParameterTest() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true", - "-XepOpt:NullAway:JarInferEnabled=true", - "-XepOpt:NullAway:JarInferUseReturnAnnotations=true")) + "-XepOpt:NullAway:JarInferEnabled=true")) .addSourceLines( "Test.java", "package com.uber;", diff --git a/nullaway/src/main/java/com/uber/nullaway/Config.java b/nullaway/src/main/java/com/uber/nullaway/Config.java index 5c683b49a9..3dc955265e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/Config.java +++ b/nullaway/src/main/java/com/uber/nullaway/Config.java @@ -278,28 +278,6 @@ public interface Config { */ boolean isJarInferEnabled(); - /** - * Checks if NullAway should use @Nullable return value annotations inferred by JarInfer. - * - * @return true if NullAway should use the @Nullable return value annotations inferred by - * JarInfer. - */ - boolean isJarInferUseReturnAnnotations(); - - /** - * Used by JarInfer - * - * @return the regex to extract jar name from the JarInfer model jar's path. - */ - String getJarInferRegexStripModelJarName(); - - /** - * Used by JarInfer. - * - * @return the regex to extract jar name from the classfile jar's path. - */ - String getJarInferRegexStripCodeJarName(); - /** * 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 43852e96b9..f255989dee 100644 --- a/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java @@ -199,22 +199,6 @@ public boolean isJarInferEnabled() { throw new IllegalStateException(ERROR_MESSAGE); } - /** --- JarInfer configs --- */ - @Override - public boolean isJarInferUseReturnAnnotations() { - throw new IllegalStateException(ERROR_MESSAGE); - } - - @Override - public String getJarInferRegexStripModelJarName() { - throw new IllegalStateException(ERROR_MESSAGE); - } - - @Override - public String getJarInferRegexStripCodeJarName() { - 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 95a498cf54..fb20d04883 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -45,8 +45,6 @@ */ final class ErrorProneCLIFlagsConfig implements Config { - private static final String BASENAME_REGEX = ".*/([^/]+)\\.[ja]ar$"; - static final String EP_FL_NAMESPACE = "NullAway"; static final String FL_ANNOTATED_PACKAGES = EP_FL_NAMESPACE + ":AnnotatedPackages"; static final String FL_ASSERTS_ENABLED = EP_FL_NAMESPACE + ":AssertsEnabled"; @@ -88,10 +86,6 @@ final class ErrorProneCLIFlagsConfig implements Config { /** --- JarInfer configs --- */ static final String FL_JI_ENABLED = EP_FL_NAMESPACE + ":JarInferEnabled"; - static final String FL_JI_USE_RETURN = EP_FL_NAMESPACE + ":JarInferUseReturnAnnotations"; - - static final String FL_JI_REGEX_MODEL_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripModelJar"; - static final String FL_JI_REGEX_CODE_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripCodeJar"; static final String FL_ERROR_URL = EP_FL_NAMESPACE + ":ErrorURL"; /** --- Serialization configs --- */ @@ -226,9 +220,6 @@ final class ErrorProneCLIFlagsConfig implements Config { /** --- JarInfer configs --- */ private final boolean jarInferEnabled; - private final boolean jarInferUseReturnAnnotations; - private final String jarInferRegexStripModelJarName; - private final String jarInferRegexStripCodeJarName; private final String errorURL; /** --- Fully qualified names of custom nonnull/nullable annotation --- */ @@ -316,12 +307,6 @@ final class ErrorProneCLIFlagsConfig implements Config { /* --- JarInfer configs --- */ jarInferEnabled = flags.getBoolean(FL_JI_ENABLED).orElse(false); - jarInferUseReturnAnnotations = flags.getBoolean(FL_JI_USE_RETURN).orElse(false); - // The defaults of these two options translate to: remove .aar/.jar from the file name, and also - // implicitly mean that NullAway will search for jarinfer models in the same jar which contains - // the analyzed classes. - jarInferRegexStripModelJarName = flags.get(FL_JI_REGEX_MODEL_PATH).orElse(BASENAME_REGEX); - jarInferRegexStripCodeJarName = flags.get(FL_JI_REGEX_CODE_PATH).orElse(BASENAME_REGEX); errorURL = flags.get(FL_ERROR_URL).orElse(DEFAULT_URL); if (acknowledgeAndroidRecent && !isAcknowledgeRestrictive) { throw new IllegalStateException( @@ -567,21 +552,6 @@ public boolean isJarInferEnabled() { return jarInferEnabled; } - @Override - public boolean isJarInferUseReturnAnnotations() { - return jarInferUseReturnAnnotations; - } - - @Override - public String getJarInferRegexStripModelJarName() { - return jarInferRegexStripModelJarName; - } - - @Override - public String getJarInferRegexStripCodeJarName() { - return jarInferRegexStripCodeJarName; - } - @Override public String getErrorURL() { return errorURL; 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 8804f9149e..5e8d5ac689 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java @@ -52,7 +52,7 @@ public static Handler buildDefault(Config config) { handlerListBuilder.add(new RestrictiveAnnotationHandler(config)); } if (config.isJarInferEnabled()) { - handlerListBuilder.add(new InferredJARModelsHandler(config)); + handlerListBuilder.add(new InferredJARModelsHandler()); } if (config.handleTestAssertionLibraries()) { handlerListBuilder.add(new AssertionHandler(methodNameUtil)); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index 88f474205e..6915406d7c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -29,7 +29,6 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.util.Context; -import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; @@ -62,12 +61,10 @@ private static void LOG(boolean cond, String tag, String msg) { private final Map>>> argAnnotCache; - private final Config config; private final StubxCacheUtil cacheUtil; - public InferredJARModelsHandler(Config config) { + public InferredJARModelsHandler() { super(); - this.config = config; String jarInferLogName = "JI"; this.cacheUtil = new StubxCacheUtil(jarInferLogName); argAnnotCache = cacheUtil.getArgAnnotCache(); @@ -181,21 +178,19 @@ && isReturnAnnotatedNullable((Symbol.MethodSymbol) exprSymbol)) { } private boolean isReturnAnnotatedNullable(Symbol.MethodSymbol methodSymbol) { - if (config.isJarInferUseReturnAnnotations()) { - Preconditions.checkNotNull(methodSymbol); - Symbol.ClassSymbol classSymbol = methodSymbol.enclClass(); - String className = classSymbol.getQualifiedName().toString(); - if (argAnnotCache.containsKey(className)) { - String methodSign = getMethodSignature(methodSymbol); - Map> methodArgAnnotations = lookupMethodInCache(className, methodSign); - if (methodArgAnnotations != null) { - Set methodAnnotations = methodArgAnnotations.get(RETURN); - if (methodAnnotations != null) { - if (methodAnnotations.contains("javax.annotation.Nullable") - || methodAnnotations.contains("org.jspecify.annotations.Nullable")) { - LOG(DEBUG, "DEBUG", "Nullable return for method: " + methodSign); - return true; - } + Preconditions.checkNotNull(methodSymbol); + Symbol.ClassSymbol classSymbol = methodSymbol.enclClass(); + String className = classSymbol.getQualifiedName().toString(); + if (argAnnotCache.containsKey(className)) { + String methodSign = getMethodSignature(methodSymbol); + Map> methodArgAnnotations = lookupMethodInCache(className, methodSign); + if (methodArgAnnotations != null) { + Set methodAnnotations = methodArgAnnotations.get(RETURN); + if (methodAnnotations != null) { + if (methodAnnotations.contains("javax.annotation.Nullable") + || methodAnnotations.contains("org.jspecify.annotations.Nullable")) { + LOG(DEBUG, "DEBUG", "Nullable return for method: " + methodSign); + return true; } } }