Ignore library models return nullability on first-party code. (#446)

By definition, library models are designed to handle third-party
libraries. However, as currently implemented, a library model
indicating an `@Nullable` return will apply to all subtypes of
the type in the model, including those in first-party code.
This change fixes that, ignoring library models information
whenever code is marked as unannotated.

See #445 for a discussion.

Generally, the principle behind this is that a method in
"annotated" code without an `@Nullable` annotation should
never magically just be considered to return `@Nullable`
implicitly, which is what the current/previous behavior
regarding library models did.

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

@@ -55,7 +55,7 @@ public static Handler buildDefault(Config config) {
     if (config.handleTestAssertionLibraries()) {
       handlerListBuilder.add(new AssertionHandler(methodNameUtil));
     }
-    handlerListBuilder.add(new LibraryModelsHandler());
+    handlerListBuilder.add(new LibraryModelsHandler(config));
     handlerListBuilder.add(StreamNullabilityPropagatorFactory.getRxStreamNullabilityPropagator());
     handlerListBuilder.add(StreamNullabilityPropagatorFactory.getJavaStreamNullabilityPropagator());
     handlerListBuilder.add(new ContractHandler());

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

@@ -38,9 +38,11 @@
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.Name;
 import com.sun.tools.javac.util.Names;
+import com.uber.nullaway.Config;
 import com.uber.nullaway.LibraryModels;
 import com.uber.nullaway.LibraryModels.MethodRef;
 import com.uber.nullaway.NullAway;
+import com.uber.nullaway.NullabilityUtil;
 import com.uber.nullaway.dataflow.AccessPath;
 import com.uber.nullaway.dataflow.AccessPathNullnessPropagation;
 import java.util.ArrayList;
@@ -63,12 +65,14 @@
  */
 public class LibraryModelsHandler extends BaseNoOpHandler {
 
+  private final Config config;
   private final LibraryModels libraryModels;
 
   @Nullable private OptimizedLibraryModels optLibraryModels;
 
-  public LibraryModelsHandler() {
+  public LibraryModelsHandler(Config config) {
     super();
+    this.config = config;
     libraryModels = loadLibraryModels();
   }
 
@@ -103,7 +107,10 @@ public boolean onOverrideMayBeNullExpr(
     if (expr.getKind() == Tree.Kind.METHOD_INVOCATION) {
       OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
       Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) ASTHelpers.getSymbol(expr);
-      if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes())
+      if (!NullabilityUtil.isUnannotated(methodSymbol, this.config)) {
+        // We only look at library models for unannotated (i.e. third-party) code.
+        return exprMayBeNull;
+      } else if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes())
           || !optLibraryModels.nullImpliesNullParameters(methodSymbol).isEmpty()) {
         // These mean the method might be null, depending on dataflow and arguments. We force
         // dataflow to run.
@@ -127,6 +134,10 @@ public NullnessHint onDataflowVisitMethodInvocation(
       AccessPathNullnessPropagation.Updates bothUpdates) {
     Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
     Preconditions.checkNotNull(callee);
+    if (!NullabilityUtil.isUnannotated(callee, this.config)) {
+      // Ignore annotated methods, library models should only apply to "unannotated" code.
+      return NullnessHint.UNKNOWN;
+    }
     setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, context);
     setConditionalArgumentNullness(thenUpdates, elseUpdates, node.getArguments(), callee, context);
     ImmutableSet<Integer> nullImpliesNullIndexes =

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

@@ -2959,4 +2959,53 @@ public void supportEnsuresAndRequiresNonNullContract() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void overridingNativeModelsInAnnotatedCodeDoesNotPropagateTheModel() {
+    // See https://github.com/uber/NullAway/issues/445
+    compilationHelper
+        .addSourceLines(
+            "NonNullGetMessage.java",
+            "package com.uber;",
+            "import java.util.Objects;",
+            "import javax.annotation.Nullable;",
+            "class NonNullGetMessage extends RuntimeException {",
+            "  NonNullGetMessage(final String message) {",
+            "     super(message);",
+            "  }",
+            "  @Override",
+            "  public String getMessage() {",
+            "    return Objects.requireNonNull(super.getMessage());",
+            "  }",
+            "  public static void foo(NonNullGetMessage e) {",
+            "    expectsNonNull(e.getMessage());",
+            "  }",
+            "  public static void expectsNonNull(String str) {",
+            "    System.out.println(str);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void overridingNativeModelsInAnnotatedCodeDoesNotGenerateSafetyHoles() {
+    // See https://github.com/uber/NullAway/issues/445
+    compilationHelper
+        .addSourceLines(
+            "NonNullGetMessage.java",
+            "package com.uber;",
+            "import java.util.Objects;",
+            "import javax.annotation.Nullable;",
+            "class NonNullGetMessage extends RuntimeException {",
+            "  NonNullGetMessage(@Nullable String message) {",
+            "     super(message);",
+            "  }",
+            "  @Override",
+            "  public String getMessage() {",
+            "    // BUG: Diagnostic contains: returning @Nullable expression",
+            "    return super.getMessage();",
+            "  }",
+            "}")
+        .doTest();
+  }
 }