refactor simple onOverrideMayBeNullExpr handlers (#747)

When `exprMayBeNull` is true, return immediately, to make clear that these handlers only support `false -> true` transitions Co-authored-by: Manu Sridharan <msridhar@gmail.com>
uber · Mar 30, 2023 · 7714c45 · 7714c45
1 parent a1d1eed
commit 7714c45
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 10 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java
@@ -196,11 +196,15 @@ public boolean onOverrideMayBeNullExpr(
       @Nullable Symbol exprSymbol,
       VisitorState state,
       boolean exprMayBeNull) {
-    if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
-        && exprSymbol instanceof Symbol.MethodSymbol) {
-      return exprMayBeNull || isReturnAnnotatedNullable((Symbol.MethodSymbol) exprSymbol);
+    if (exprMayBeNull) {
+      return true;
     }
-    return exprMayBeNull;
+    if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
+        && exprSymbol instanceof Symbol.MethodSymbol
+        && isReturnAnnotatedNullable((Symbol.MethodSymbol) exprSymbol)) {
+      return true;
+    }
+    return false;
   }
 
   private boolean isReturnAnnotatedNullable(Symbol.MethodSymbol methodSymbol) {

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java
@@ -87,12 +87,15 @@ public boolean onOverrideMayBeNullExpr(
       @Nullable Symbol exprSymbol,
       VisitorState state,
       boolean exprMayBeNull) {
-    if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
+    if (exprMayBeNull) {
+      return true;
+    }
+    if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
         && exprSymbol instanceof Symbol.MethodSymbol
         && optionalIsGetCall((Symbol.MethodSymbol) exprSymbol, state.getTypes())) {
       return true;
     }
-    return exprMayBeNull;
+    return false;
   }
 
   @Override

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java
@@ -76,13 +76,16 @@ public boolean onOverrideMayBeNullExpr(
       @Nullable Symbol exprSymbol,
       VisitorState state,
       boolean exprMayBeNull) {
+    if (exprMayBeNull) {
+      return true;
+    }
     Tree.Kind exprKind = expr.getKind();
     if (exprSymbol != null
-        && (exprKind.equals(Tree.Kind.METHOD_INVOCATION)
-            || exprKind.equals(Tree.Kind.IDENTIFIER))) {
-      return exprMayBeNull || isSymbolRestrictivelyNullable(exprSymbol, state.context);
+        && (exprKind == Tree.Kind.METHOD_INVOCATION || exprKind == Tree.Kind.IDENTIFIER)
+        && isSymbolRestrictivelyNullable(exprSymbol, state.context)) {
+      return true;
     }
-    return exprMayBeNull;
+    return false;
   }
 
   @Nullable private CodeAnnotationInfo codeAnnotationInfo;