No need to report when all paths are verified but return is wrong

uber · Sep 30, 2024 · 56a0984 · 56a0984
1 parent 3cb7d51
commit 56a0984
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 18 deletions.
diff --git a/.../main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java b/.../main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullIfHandler.java
@@ -211,30 +211,16 @@ public void onDataflowVisitReturn(
      * The decision is as follows:
      *
      * If all fields in the path are verified:
-     * - If the literal boolean is equals to the configured non-null boolean, semantics are correct,
-     * as the method does return correctly in case the semantics hold.
-     * - If the literal boolean isn't equals to the configured non-null boolean, semantics are wrong,
-     * as the method incorrectly returns when it should have returned true.
-     * - If the expression isn't a literal boolean, but something more complex,
-     * we assume semantics are correct as we trust the data-flow engine.
+     * - Semantics are valid
      *
      * If fields in path aren't verified:
      * - If the literal boolean is the opposite of the configured non-null boolean, semantics
      * are then correct, as the method correctly returns in case the semantics don't hold.
      * - Otherwise, semantics are wrong, as the method incorrectly returns.
      * - If the expression isn't a literal boolean, then semantics are wrong, as we
      * assume the data-flow engine is correct.
-     *
-     * The implementation below is an optimized version of the decision table above.
      */
-    if (allFieldsAreNonNull) {
-      if (isBooleanLiteral && !evaluatesToNonNullLiteral) {
-        raiseError(
-            returnTree,
-            state,
-            "The method ensures the non-nullability of the fields, but returns incorrectly");
-      }
-    } else {
+    if (!allFieldsAreNonNull) {
       if (evaluatesToNonNullLiteral || !isBooleanLiteral) {
         fieldNames.removeAll(nonNullFieldsInPath);
         String message =

diff --git a/nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullIfTests.java b/nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullIfTests.java
@@ -559,7 +559,6 @@ public void semanticIssues_doesntEnsureNonNullabilityOfFields() {
             "  @EnsuresNonNullIf(value=\"nullableItem\", result=true)",
             "  public boolean hasNullableItem() {",
             "    if(nullableItem != null) {",
-            "      // BUG: Diagnostic contains: The method ensures the non-nullability of the fields, but returns incorrectly",
             "      return false;",
             "    } else {",
             "      // BUG: Diagnostic contains: Method is annotated with @EnsuresNonNullIf but does not ensure fields [nullableItem]",
@@ -713,7 +712,6 @@ public void semanticIssues_methodDeclarationReversed() {
             "  @EnsuresNonNullIf(value=\"nullableItem\", result=true)",
             "  public boolean hasNullableItem() {",
             "    if(nullableItem != null) {",
-            "      // BUG: Diagnostic contains: The method ensures the non-nullability of the fields, but returns incorrectly",
             "      return false;",
             "    } else {",
             "      // BUG: Diagnostic contains: Method is annotated with @EnsuresNonNullIf but does not ensure fields [nullableItem]",