Model Map.getOrDefault (#724)

We model the method so that the nullability of the return matches the nullability of the second argument. We also add an explicit model of `ImmutableMap.getOrDefault`, to override the annotations on that method for the case where `AcknowledgeRestrictiveAnnotations` is set. Fixes #723 Co-authored-by: Lázaro Clapp <lazaro@uber.com>
uber · Feb 1, 2023 · 8e948db · 8e948db
1 parent 8a24aa3
commit 8e948db
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 1 deletion.
diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java
@@ -84,7 +84,9 @@ public interface LibraryModels {
    * Get (method, parameter) pairs that cause the method to return <code>null</code> when passed
    * <code>null</code> on that parameter.
    *
-   * <p>This is equivalent to annotating a method with a contract like:
+   * <p>This is equivalent to annotating a method with both a {@code @Nullable} return type
+   * <em>and</em> a {@code @Contract} annotation specifying that if the parameter is
+   * {@code @NonNull} then the return is {@code @NonNull}, e.g.:
    *
    * <pre><code>@Contract("!null -&gt; !null") @Nullable</code></pre>
    *

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
@@ -614,6 +614,14 @@ private static class DefaultLibraryModels implements LibraryModels {
         new ImmutableSetMultimap.Builder<MethodRef, Integer>()
             .put(methodRef("java.util.Optional", "orElse(T)"), 0)
             .put(methodRef("com.google.common.io.Closer", "<C>register(C)"), 0)
+            .put(methodRef("java.util.Map", "getOrDefault(java.lang.Object,V)"), 1)
+            // We add ImmutableMap.getOrDefault explicitly, since when
+            // AcknowledgeRestrictiveAnnotations is enabled, the explicit annotations in the code
+            // override the inherited library model
+            .put(
+                methodRef(
+                    "com.google.common.collect.ImmutableMap", "getOrDefault(java.lang.Object,V)"),
+                1)
             .build();
 
     private static final ImmutableSet<MethodRef> NULLABLE_RETURNS =

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java
@@ -365,4 +365,50 @@ public void systemConsoleNullable() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void mapGetOrDefault() {
+    String[] sourceLines =
+        new String[] {
+          "package com.uber;",
+          "import java.util.Map;",
+          "import com.google.common.collect.ImmutableMap;",
+          "import org.jspecify.annotations.Nullable;",
+          "class Test {",
+          "  void testGetOrDefaultMap(Map<String, String> m, String nonNullString, @Nullable String nullableString) {",
+          "    m.getOrDefault(\"key\", \"value\").toString();",
+          "    m.getOrDefault(\"key\", nonNullString).toString();",
+          "    // BUG: Diagnostic contains: dereferenced",
+          "    m.getOrDefault(\"key\", null).toString();",
+          "    // BUG: Diagnostic contains: dereferenced",
+          "    m.getOrDefault(\"key\", nullableString).toString();",
+          "  }",
+          "  void testGetOrDefaultImmutableMap(ImmutableMap<String, String> im, String nonNullString, @Nullable String nullableString) {",
+          "    im.getOrDefault(\"key\", \"value\").toString();",
+          "    im.getOrDefault(\"key\", nonNullString).toString();",
+          "    // BUG: Diagnostic contains: dereferenced",
+          "    im.getOrDefault(\"key\", null).toString();",
+          "    // BUG: Diagnostic contains: dereferenced",
+          "    im.getOrDefault(\"key\", nullableString).toString();",
+          "  }",
+          "}"
+        };
+    // test *without* restrictive annotations enabled
+    makeTestHelperWithArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber"))
+        .addSourceLines("Test.java", sourceLines)
+        .doTest();
+    // test *with* restrictive annotations enabled
+    makeTestHelperWithArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+        .addSourceLines("Test.java", sourceLines)
+        .doTest();
+  }
 }