From 88215675450b38597895d943895423518d595ea3 Mon Sep 17 00:00:00 2001
From: Christopher Lambert <xn137@gmx.de>
Date: Wed, 5 Apr 2023 18:24:28 +0200
Subject: [PATCH] refactor LibraryModelsHandler.onOverrideMayBeNullExpr (#754)

Also removes the call to `nullnessFromDataflow` from within this handler.
---
 .../handlers/LibraryModelsHandler.java        | 46 ++++++++++---------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
index 8d8b48641a..954651bccc 100644
--- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
+++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
@@ -128,29 +128,31 @@ public boolean onOverrideMayBeNullExpr(
       @Nullable Symbol exprSymbol,
       VisitorState state,
       boolean exprMayBeNull) {
-    if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
-        && exprSymbol instanceof Symbol.MethodSymbol) {
-      OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
-      Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
-      // When looking up library models of annotated code, we match the exact method signature only;
-      // overriding methods in subclasses must be explicitly given their own library model.
-      // When dealing with unannotated code, we default to generality: a model applies to a method
-      // and any of its overriding implementations.
-      // see https://github.com/uber/NullAway/issues/445 for why this is needed.
-      boolean isMethodAnnotated =
-          !getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config);
-      if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes(), !isMethodAnnotated)
-          || !optLibraryModels.nullImpliesNullParameters(methodSymbol).isEmpty()) {
-        // These mean the method might be null, depending on dataflow and arguments. We force
-        // dataflow to run.
-        return analysis.nullnessFromDataflow(state, expr) || exprMayBeNull;
-      } else if (optLibraryModels.hasNonNullReturn(
-          methodSymbol, state.getTypes(), !isMethodAnnotated)) {
-        // This means the method can't be null, so we return false outright.
-        return false;
-      }
+    if (!(expr.getKind() == Tree.Kind.METHOD_INVOCATION
+        && exprSymbol instanceof Symbol.MethodSymbol)) {
+      return exprMayBeNull;
+    }
+    OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
+    Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
+    // When looking up library models of annotated code, we match the exact method signature only;
+    // overriding methods in subclasses must be explicitly given their own library model.
+    // When dealing with unannotated code, we default to generality: a model applies to a method
+    // and any of its overriding implementations.
+    // see https://github.com/uber/NullAway/issues/445 for why this is needed.
+    boolean isMethodUnannotated =
+        getCodeAnnotationInfo(state.context).isSymbolUnannotated(methodSymbol, this.config);
+    if (exprMayBeNull) {
+      // This is the only case in which we may switch the result from @Nullable to @NonNull:
+      return !optLibraryModels.hasNonNullReturn(
+          methodSymbol, state.getTypes(), isMethodUnannotated);
+    }
+    if (optLibraryModels.hasNullableReturn(methodSymbol, state.getTypes(), isMethodUnannotated)) {
+      return true;
+    }
+    if (!optLibraryModels.nullImpliesNullParameters(methodSymbol).isEmpty()) {
+      return true;
     }
-    return exprMayBeNull;
+    return false;
   }
 
   @Override