Refactor configuration for unmarked and generated code (Fixes #978)

jmh/src/main/java/com/uber/nullaway/jmh/NullawayReleaseCompiler.java

@@ -40,7 +40,7 @@ protected String getSourceDirectory() {
   protected List<String> getExtraErrorProneArgs() {
     return Arrays.asList(
         "-XepOpt:NullAway:CheckOptionalEmptiness=true",
-        "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true",
+        "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false",
         "-XepOpt:NullAway:CastToNonNullMethod=com.uber.nullaway.NullabilityUtil.castToNonNull");
   }
 

nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java

@@ -267,7 +267,7 @@ record =
   private boolean shouldTreatAsUnannotated(Symbol.ClassSymbol classSymbol, Config config) {
     if (config.isUnannotatedClass(classSymbol)) {
       return true;
-    } else if (config.treatGeneratedAsUnannotated()) {
+    } else if (config.ignoreAnnotationsInGeneratedCode()) {
       // Generated code is or isn't excluded, depending on configuration
       // Note: In the future, we might want finer grain controls to distinguish code that is
       // generated with nullability info and without.

nullaway/src/main/java/com/uber/nullaway/Config.java

@@ -69,12 +69,11 @@ public interface Config {
   boolean fromExplicitlyUnannotatedPackage(String className);
 
   /**
-   * Checks if (tool) generated code should be considered always unannoatated.
+   * Checks if annotations in (tool) generated code should be ignored.
    *
-   * @return true if code marked as generated code should be treated as unannotated, even if it
-   *     comes from a package otherwise configured as annotated.
+   * @return true if annotations in code marked as generated code should be ignored.
    */
-  boolean treatGeneratedAsUnannotated();
+  boolean ignoreAnnotationsInGeneratedCode();
 
   /**
    * Checks if a class should be excluded.
@@ -190,15 +189,13 @@ public interface Config {
   boolean assertsEnabled();
 
   /**
-   * Checks if acknowledging restrictive annotations is enabled.
+   * Checks if annotations in unmarked code should be ignored.
    *
-   * @return true if the null checker should acknowledge stricter nullability annotations whenever
-   *     they are available in unannotated code, defaulting to optimistic defaults only when
-   *     explicit annotations are missing. false if any annotations in code not explicitly marked as
-   *     annotated should be ignored completely and unannotated code should always be treated
-   *     optimistically.
+   * @return true if the null checker should ignore annotations in unmarked code.
+   * false if annotations in code not explicitly marked as
+   * annotated should be acknowledged (the default).
    */
-  boolean acknowledgeRestrictiveAnnotations();
+  boolean ignoreAnnotationsInUnmarkedCode();
 
   /**
    * Checks if optional emptiness checking is enabled.

nullaway/src/main/java/com/uber/nullaway/DummyOptionsConfig.java

@@ -65,7 +65,7 @@ public boolean fromExplicitlyUnannotatedPackage(String className) {
   }
 
   @Override
-  public boolean treatGeneratedAsUnannotated() {
+  public boolean ignoreAnnotationsInGeneratedCode() {
     throw new IllegalStateException(ERROR_MESSAGE);
   }
 
@@ -150,7 +150,7 @@ public boolean assertsEnabled() {
   }
 
   @Override
-  public boolean acknowledgeRestrictiveAnnotations() {
+  public boolean ignoreAnnotationsInUnmarkedCode() {
     throw new IllegalStateException(ERROR_MESSAGE);
   }
 

nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java

@@ -58,7 +58,7 @@ final class ErrorProneCLIFlagsConfig implements Config {
 
   static final String FL_CLASS_ANNOTATIONS_GENERATED =
       EP_FL_NAMESPACE + ":CustomGeneratedCodeAnnotations";
-  static final String FL_GENERATED_UNANNOTATED = EP_FL_NAMESPACE + ":TreatGeneratedAsUnannotated";
+  static final String FL_IGNORE_ANNOTATIONS_IN_GENERATED_CODE = EP_FL_NAMESPACE + ":IgnoreAnnotationsInGeneratedCode";
   static final String FL_ACKNOWLEDGE_ANDROID_RECENT = EP_FL_NAMESPACE + ":AcknowledgeAndroidRecent";
   static final String FL_JSPECIFY_MODE = EP_FL_NAMESPACE + ":JSpecifyMode";
   static final String FL_EXCLUDED_FIELD_ANNOT = EP_FL_NAMESPACE + ":ExcludedFieldAnnotations";
@@ -69,8 +69,8 @@ final class ErrorProneCLIFlagsConfig implements Config {
   static final String FL_EXTERNAL_INIT_ANNOT = EP_FL_NAMESPACE + ":ExternalInitAnnotations";
   static final String FL_CONTRACT_ANNOT = EP_FL_NAMESPACE + ":CustomContractAnnotations";
   static final String FL_UNANNOTATED_CLASSES = EP_FL_NAMESPACE + ":UnannotatedClasses";
-  static final String FL_ACKNOWLEDGE_RESTRICTIVE =
-      EP_FL_NAMESPACE + ":AcknowledgeRestrictiveAnnotations";
+  static final String FL_IGNORE_ANNOTATIONS_IN_UNMARKED_CODE =
+      EP_FL_NAMESPACE + ":IgnoreAnnotationsInUnmarkedCode";
   static final String FL_CHECK_OPTIONAL_EMPTINESS = EP_FL_NAMESPACE + ":CheckOptionalEmptiness";
   static final String FL_CHECK_CONTRACTS = EP_FL_NAMESPACE + ":CheckContracts";
   static final String FL_HANDLE_TEST_ASSERTION_LIBRARIES =
@@ -224,13 +224,13 @@ final class ErrorProneCLIFlagsConfig implements Config {
   private final Pattern fieldAnnotPattern;
   private final boolean isExhaustiveOverride;
   private final boolean isSuggestSuppressions;
-  private final boolean isAcknowledgeRestrictive;
+  private final boolean ignoreAnnotationsInUnmarkedCode;
   private final boolean checkOptionalEmptiness;
   private final boolean checkContracts;
   private final boolean handleTestAssertionLibraries;
   private final ImmutableSet<String> optionalClassPaths;
   private final boolean assertsEnabled;
-  private final boolean treatGeneratedAsUnannotated;
+  private final boolean ignoreAnnotationsInGeneratedCode;
   private final boolean acknowledgeAndroidRecent;
   private final boolean jspecifyMode;
   private final boolean handleWildcardGenerics;
@@ -298,12 +298,12 @@ final class ErrorProneCLIFlagsConfig implements Config {
     contractAnnotations = getFlagStringSet(flags, FL_CONTRACT_ANNOT, DEFAULT_CONTRACT_ANNOT);
     isExhaustiveOverride = flags.getBoolean(FL_EXHAUSTIVE_OVERRIDE).orElse(false);
     isSuggestSuppressions = flags.getBoolean(FL_SUGGEST_SUPPRESSIONS).orElse(false);
-    isAcknowledgeRestrictive = flags.getBoolean(FL_ACKNOWLEDGE_RESTRICTIVE).orElse(false);
+    ignoreAnnotationsInUnmarkedCode = flags.getBoolean(FL_IGNORE_ANNOTATIONS_IN_UNMARKED_CODE).orElse(false);
     checkOptionalEmptiness = flags.getBoolean(FL_CHECK_OPTIONAL_EMPTINESS).orElse(false);
     checkContracts = flags.getBoolean(FL_CHECK_CONTRACTS).orElse(false);
     handleTestAssertionLibraries =
         flags.getBoolean(FL_HANDLE_TEST_ASSERTION_LIBRARIES).orElse(false);
-    treatGeneratedAsUnannotated = flags.getBoolean(FL_GENERATED_UNANNOTATED).orElse(false);
+    ignoreAnnotationsInGeneratedCode = flags.getBoolean(FL_IGNORE_ANNOTATIONS_IN_GENERATED_CODE).orElse(false);
     acknowledgeAndroidRecent = flags.getBoolean(FL_ACKNOWLEDGE_ANDROID_RECENT).orElse(false);
     jspecifyMode = flags.getBoolean(FL_JSPECIFY_MODE).orElse(false);
     handleWildcardGenerics = flags.getBoolean(FL_HANDLE_WILDCARD_GENERICS).orElse(false);
@@ -339,13 +339,13 @@ final class ErrorProneCLIFlagsConfig implements Config {
     /* --- JarInfer configs --- */
     jarInferEnabled = flags.getBoolean(FL_JI_ENABLED).orElse(false);
     errorURL = flags.get(FL_ERROR_URL).orElse(DEFAULT_URL);
-    if (acknowledgeAndroidRecent && !isAcknowledgeRestrictive) {
+    if (acknowledgeAndroidRecent && ignoreAnnotationsInUnmarkedCode) {
       throw new IllegalStateException(
           "-XepOpt:"
               + FL_ACKNOWLEDGE_ANDROID_RECENT
-              + " should only be set when -XepOpt:"
-              + FL_ACKNOWLEDGE_RESTRICTIVE
-              + " is also set");
+              + " should not be set when -XepOpt:"
+              + FL_IGNORE_ANNOTATIONS_IN_UNMARKED_CODE
+              + " is set");
     }
     serializationActivationFlag = flags.getBoolean(FL_FIX_SERIALIZATION).orElse(false);
     Optional<String> fixSerializationConfigPath = flags.get(FL_FIX_SERIALIZATION_CONFIG_PATH);
@@ -429,8 +429,8 @@ public boolean fromExplicitlyUnannotatedPackage(String className) {
   }
 
   @Override
-  public boolean treatGeneratedAsUnannotated() {
-    return treatGeneratedAsUnannotated;
+  public boolean ignoreAnnotationsInGeneratedCode() {
+    return ignoreAnnotationsInGeneratedCode;
   }
 
   @Override
@@ -519,9 +519,9 @@ public boolean suggestSuppressions() {
   }
 
   @Override
-  public boolean acknowledgeRestrictiveAnnotations() {
+  public boolean ignoreAnnotationsInUnmarkedCode() {
     // restrictive annotations must always be acknowledged in JSpecify mode
-    return isAcknowledgeRestrictive || jspecifyMode;
+    return ignoreAnnotationsInUnmarkedCode && !jspecifyMode;
   }
 
   @Override

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -997,7 +997,7 @@ private boolean paramOfOverridingMethodIsNonNull(
     boolean result = false;
     if (isMethodAnnotated) {
       result = !Nullness.hasNullableAnnotation(paramSymbol, config);
-    } else if (config.acknowledgeRestrictiveAnnotations()) {
+    } else if (!config.ignoreAnnotationsInUnmarkedCode()) {
       // can still be @NonNull if there is a restrictive annotation
       result = Nullness.hasNonNullAnnotation(paramSymbol, config);
     }

nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java

@@ -47,7 +47,7 @@ public static Handler buildDefault(Config config) {
     MethodNameUtil methodNameUtil = new MethodNameUtil();
 
     RestrictiveAnnotationHandler restrictiveAnnotationHandler = null;
-    if (config.acknowledgeRestrictiveAnnotations()) {
+    if (!config.ignoreAnnotationsInUnmarkedCode()) {
       // This runs before LibraryModelsHandler, so that library models can override third-party
       // bytecode annotations
       restrictiveAnnotationHandler = new RestrictiveAnnotationHandler(config);

nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java

@@ -57,10 +57,10 @@ public void initMainHandler(Handler mainHandler) {
 
   /**
    * Returns true iff the symbol is considered unannotated but restrictively annotated
-   * {@code @Nullable} under {@code AcknowledgeRestrictiveAnnotations=true} logic.
+   * {@code @Nullable} under {@code IgnoreAnnotationsInUnmarkedCode=false} logic.
    *
    * <p>In particular, this means the symbol is explicitly annotated as {@code @Nullable} and, if
-   * {@code TreatGeneratedAsUnannotated=true}, it is not within generated code.
+   * {@code IgnoreAnnotationsInGeneratedCode=true}, it is not within generated code.
    *
    * @param symbol the symbol being checked
    * @param context Javac Context or Error Prone SubContext
@@ -72,7 +72,7 @@ private boolean isSymbolRestrictivelyNullable(Symbol symbol, Context context) {
     return (codeAnnotationInfo.isSymbolUnannotated(symbol, config, mainHandler)
         // with the generated-as-unannotated option enabled, we want to ignore annotations in
         // generated code no matter what
-        && !(config.treatGeneratedAsUnannotated() && codeAnnotationInfo.isGenerated(symbol, config))
+        && !(config.ignoreAnnotationsInGeneratedCode() && codeAnnotationInfo.isGenerated(symbol, config))
         && Nullness.hasNullableAnnotation(symbol, config));
   }
 

nullaway/src/test/java/com/uber/nullaway/AcknowledgeRestrictiveAnnotationsTests.java

@@ -12,8 +12,8 @@ public void generatedAsUnannotatedPlusRestrictive() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
-                "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInGeneratedCode=true",
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Generated.java",
             """
@@ -45,7 +45,7 @@ public void defaultPermissiveOnUnannotated() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=false"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=true"))
         .addSourceLines(
             "Test.java",
             """
@@ -73,7 +73,7 @@ public void acknowledgeRestrictiveAnnotationsWhenFlagSet() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -104,7 +104,7 @@ public void defaultPermissiveOnRecently() {
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
                 // should be permissive even when AcknowledgeRestrictiveAnnotations is set
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -132,7 +132,7 @@ public void acknowledgeRecentlyAnnotationsWhenFlagSet() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true",
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false",
                 "-XepOpt:NullAway:AcknowledgeAndroidRecent=true"))
         .addSourceLines(
             "Test.java",
@@ -161,7 +161,7 @@ public void restrictivelyAnnotatedMethodsWorkWithNullnessFromDataflow() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -194,7 +194,7 @@ public void restrictivelyAnnotatedMethodsWorkWithNullnessFromDataflow2() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -221,7 +221,7 @@ public void overridingRestrictivelyAnnotatedMethod() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "TestNegativeCases.java",
             """
@@ -265,7 +265,7 @@ public void lambdaPlusRestrictivePositive() {
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -292,6 +292,7 @@ public void lambdaPlusRestrictiveNegative() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=true",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated"))
         .addSourceLines(
             "Test.java",
@@ -321,7 +322,7 @@ public void annotatedVsUnannotatedMethodRefOverrideChecks() {
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated",
                 // Note: this is the OFF case.
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=false"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=true"))
         .addSourceLines(
             "AnnotatedStringIDFunctions.java",
             """
@@ -394,7 +395,7 @@ public void annotatedVsUnannotatedMethodRefOverrideWithRestrictiveAnnotations()
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "AnnotatedStringIDFunctions.java",
             """
@@ -468,7 +469,7 @@ public void methodRefToNullUnmarked() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """
@@ -499,7 +500,7 @@ public void methodRefToNullUnmarkedVarargs() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines(
             "Test.java",
             """

nullaway/src/test/java/com/uber/nullaway/CoreTests.java

@@ -617,7 +617,7 @@ public void testCustomNonnullAnnotation() {
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
                 "-XepOpt:NullAway:UnannotatedClasses=com.uber.Other",
                 "-XepOpt:NullAway:CustomNonnullAnnotations=qual.NoNull",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines("qual/NoNull.java", "package qual;", "public @interface NoNull {", "}")
         .addSourceLines(
             "Other.java",

nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java

@@ -705,7 +705,7 @@ public void testLombokBuilderWithGeneratedAsUnannotated() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
-                "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInGeneratedCode=true"))
         .addSourceLines(
             "Test.java",
             """
@@ -731,7 +731,7 @@ LombokDTO testBuilderSafe(@Nullable String s1, String s2) {
               }
               LombokDTO testBuilderUnsafe(@Nullable String s1, @Nullable String s2) {
                  // No error, because the code of LombokDTO.Builder is @Generated and we are
-                 // building with TreatGeneratedAsUnannotated=true
+                 // building with IgnoreAnnotationsInGeneratedCode=true
                  return LombokDTO.builder().nullableField(s1).field(s2).build();
               }
             }
@@ -855,7 +855,8 @@ public void mapGetOrDefault() {
             Arrays.asList(
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
-                "-XepOpt:NullAway:AnnotatedPackages=com.uber"))
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=true"))
         .addSourceLines("Test.java", sourceLines)
         .doTest();
     // test *with* restrictive annotations enabled
@@ -864,7 +865,7 @@ public void mapGetOrDefault() {
                 "-d",
                 temporaryFolder.getRoot().getAbsolutePath(),
                 "-XepOpt:NullAway:AnnotatedPackages=com.uber",
-                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+                "-XepOpt:NullAway:IgnoreAnnotationsInUnmarkedCode=false"))
         .addSourceLines("Test.java", sourceLines)
         .doTest();
   }