Remove unused or unneeded JarInfer flags (#1050)

The `JarInferRegexStripModelJar` and `JarInferRegexStripCodeJar` options
seem completely unused in our code base; so removing them is just a
cleanup. (We should still document in the release notes.)

We remove `JarInferUseReturnAnnotations` as well. Recall that JarInfer
infers a `@Nullable` return annotation if it sees the statement `return
null` in the body of the method. We remove this flag for a number of
reasons:

* I would argue this kind of thing is better controlled when
_generating_ `astubx` files rather than while reading them. I would
expect that methods which have `return null` in them should usually have
a `@Nullable` return. In cases where this is not desired, the
configuration really should be on the generation side I think, excluding
a few methods.
* This flag doesn't even work when instrumenting bytecodes; there the
injected `@Nullable` annotations will be used independent of the flag
setting. It only works for models read from `astubx`. So the most common
internal use of JarInfer doesn't make use of this flag anyway.
* The presence of this flag complicates merging our JarInfer handling
code with our other library model handling code, a very desirable
cleanup we want to do.
* I confirmed that internally at Uber, not setting this flag has no
impact on the build (no new errors pop up).

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;",

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;",

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.
    *

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);

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;

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));

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<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();
@@ -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;
           }
         }
       }