Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused or unneeded JarInfer flags #1050

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Remove unused or unneeded JarInfer flags #1050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a354901
Remove unused or unneeded JarInfer flags
msridhar Oct 4, 2024
b81796d
Merge branch 'master' into remove-jarinfer-return-annotations-flag
msridhar Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...ay-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand Down
26 changes: 9 additions & 17 deletions ...ntegration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand Down Expand Up @@ -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;",
Expand Down Expand Up @@ -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\"));",
" }",
"}")
Expand All @@ -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;",
Expand All @@ -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;",
Expand Down Expand Up @@ -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;",
Expand All @@ -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;",
Expand All @@ -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;",
Expand All @@ -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;",
Expand Down
22 changes: 0 additions & 22 deletions nullaway/src/main/java/com/uber/nullaway/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
16 changes: 0 additions & 16 deletions nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 0 additions & 30 deletions nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 --- */
Expand Down Expand Up @@ -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 --- */
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
33 changes: 14 additions & 19 deletions nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -62,12 +61,10 @@ private static void LOG(boolean cond, String tag, String msg) {

private final Map<String, Map<String, Map<Integer, Set<String>>>> 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();
Expand Down Expand Up @@ -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<Integer, Set<String>> methodArgAnnotations = lookupMethodInCache(className, methodSign);
if (methodArgAnnotations != null) {
Set<String> 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<Integer, Set<String>> methodArgAnnotations = lookupMethodInCache(className, methodSign);
if (methodArgAnnotations != null) {
Set<String> 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;
}
}
}
Expand Down
Loading