From a92ba5a2a9b3dc930d42136ec7ff51ef3167e160 Mon Sep 17 00:00:00 2001
From: Anders Papitto <andersj@fb.com>
Date: Tue, 17 Oct 2017 15:16:49 -0700
Subject: [PATCH] Eradicate: model get and remove as nullable for stdlib Map
 variants

Reviewed By: jeremydubreil

Differential Revision: D6083471

fbshipit-source-id: 08c133b
---
 infer/src/eradicate/modelTables.ml            | 24 +++++++++++++++++++
 .../java/eradicate/NullMethodCall.java        | 22 +++++++++++++++++
 .../codetoanalyze/java/eradicate/issues.exp   | 20 ++++++++++------
 3 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/infer/src/eradicate/modelTables.ml b/infer/src/eradicate/modelTables.ml
index 6b2beaa05..3ea1c8e07 100644
--- a/infer/src/eradicate/modelTables.ml
+++ b/infer/src/eradicate/modelTables.ml
@@ -55,6 +55,9 @@ let cg = if strict_containers then (n, [o]) else (n, [n])
 (* container put *)
 let cp = if strict_containers then (n, [o; o]) else (n, [n; n])
 
+(* container remove *)
+let cr = if strict_containers then (n, [o]) else (n, [n])
+
 (* nullable getter *)
 let ng = (n, [])
 
@@ -229,7 +232,28 @@ let annotated_list_nullable =
     ; (ca, "java.util.ArrayList.add(java.lang.Object):boolean")
     ; (ca, "java.util.List.add(java.lang.Object):boolean")
     ; (cg, "java.util.Map.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.Map.remove(java.lang.Object):java.lang.Object")
     ; (cp, "java.util.Map.put(java.lang.Object,java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.HashMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.HashMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.concurrent.ConcurrentHashMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.concurrent.ConcurrentHashMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.AbstractMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.AbstractMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.concurrent.ConcurrentSkipListMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.concurrent.ConcurrentSkipListMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.EnumMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.EnumMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.Hashtable.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.Hashtable.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.IdentityHashMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.IdentityHashMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.LinkedHashMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.LinkedHashMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.TreeMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.TreeMap.remove(java.lang.Object):java.lang.Object")
+    ; (cg, "java.util.WeakHashMap.get(java.lang.Object):java.lang.Object")
+    ; (cr, "java.util.WeakHashMap.remove(java.lang.Object):java.lang.Object")
     ; ( (n, [o])
       , "javax.lang.model.element.Element.getAnnotation(java.lang.Class):java.lang.annotation.Annotation"
       )
diff --git a/infer/tests/codetoanalyze/java/eradicate/NullMethodCall.java b/infer/tests/codetoanalyze/java/eradicate/NullMethodCall.java
index 25d9c632a..2f163c9c3 100644
--- a/infer/tests/codetoanalyze/java/eradicate/NullMethodCall.java
+++ b/infer/tests/codetoanalyze/java/eradicate/NullMethodCall.java
@@ -15,6 +15,9 @@ import java.lang.System;
 import javax.annotation.Nullable;
 import com.facebook.infer.annotation.Assertions;
 
+import java.util.Map;
+import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class NullMethodCall {
 
@@ -258,4 +261,23 @@ public class NullMethodCall {
       int n = s.length();
     }
   }
+
+  public void testMapGetBad
+      (Map<String, String> m,
+       HashMap<String, String> hm,
+       ConcurrentHashMap<String, String> chm) {
+      m.get("foo").toString();
+      hm.get("foo").toString();
+      chm.get("foo").toString();
+  }
+
+  public void testMapRemoveBad
+      (Map<String, String> m,
+       HashMap<String, String> hm,
+       ConcurrentHashMap<String, String> chm) {
+      m.remove("foo").toString();
+      hm.remove("foo").toString();
+      chm.remove("foo").toString();
+  }
+
 }
diff --git a/infer/tests/codetoanalyze/java/eradicate/issues.exp b/infer/tests/codetoanalyze/java/eradicate/issues.exp
index 811cd53a5..ed2691b29 100644
--- a/infer/tests/codetoanalyze/java/eradicate/issues.exp
+++ b/infer/tests/codetoanalyze/java/eradicate/issues.exp
@@ -37,13 +37,19 @@ codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.arrayLeng
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useInterface(NullFieldAccess$I), 2, ERADICATE_NULL_FIELD_ACCESS, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess$I.c at line 52)]
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useX(), 2, ERADICATE_NULL_FIELD_ACCESS, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess.x at line 37)]
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useZ(), 2, ERADICATE_NULL_FIELD_ACCESS, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess.z at line 47)]
-codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerField(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.fld at line 71)]
-codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerPrivateField(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.pfld at line 82)]
-codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall.testSystemGetenvBad(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `envValue` in the call to `length()` could be null. (Origin: call to getenv(...) modelled in modelTables.ml at line 243)]
-codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.callOnNull(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 22)]
-codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testExceptionPerInstruction(int), 6, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 183)]
-codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testFieldAssignmentIfThenElse(String), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 174)]
-codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testSystemGetPropertyReturn(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: call to getProperty(...) modelled in modelTables.ml at line 238)]
+codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerField(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.fld at line 74)]
+codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerPrivateField(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.pfld at line 85)]
+codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall.testSystemGetenvBad(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `envValue` in the call to `length()` could be null. (Origin: call to getenv(...) modelled in modelTables.ml at line 246)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.callOnNull(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 25)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testExceptionPerInstruction(int), 6, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 186)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testFieldAssignmentIfThenElse(String), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: null constant at line 177)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapGetBad(Map,HashMap,ConcurrentHashMap), 4, ERADICATE_NULL_METHOD_CALL, [origin,The value of `m.get(...)` in the call to `toString()` could be null. (Origin: call to get(...) modelled in modelTables.ml at line 269)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapGetBad(Map,HashMap,ConcurrentHashMap), 5, ERADICATE_NULL_METHOD_CALL, [origin,The value of `hm.get(...)` in the call to `toString()` could be null. (Origin: call to get(...) modelled in modelTables.ml at line 270)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapGetBad(Map,HashMap,ConcurrentHashMap), 6, ERADICATE_NULL_METHOD_CALL, [origin,The value of `chm.get(...)` in the call to `toString()` could be null. (Origin: call to get(...) modelled in modelTables.ml at line 271)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapRemoveBad(Map,HashMap,ConcurrentHashMap), 4, ERADICATE_NULL_METHOD_CALL, [origin,The value of `m.remove(...)` in the call to `toString()` could be null. (Origin: call to remove(...) modelled in modelTables.ml at line 278)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapRemoveBad(Map,HashMap,ConcurrentHashMap), 5, ERADICATE_NULL_METHOD_CALL, [origin,The value of `hm.remove(...)` in the call to `toString()` could be null. (Origin: call to remove(...) modelled in modelTables.ml at line 279)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testMapRemoveBad(Map,HashMap,ConcurrentHashMap), 6, ERADICATE_NULL_METHOD_CALL, [origin,The value of `chm.remove(...)` in the call to `toString()` could be null. (Origin: call to remove(...) modelled in modelTables.ml at line 280)]
+codetoanalyze/java/eradicate/NullMethodCall.java, void NullMethodCall.testSystemGetPropertyReturn(), 2, ERADICATE_NULL_METHOD_CALL, [origin,The value of `s` in the call to `length()` could be null. (Origin: call to getProperty(...) modelled in modelTables.ml at line 241)]
 codetoanalyze/java/eradicate/ParameterNotNullable.java, ParameterNotNullable$ConstructorCall.<init>(ParameterNotNullable,int), 1, ERADICATE_PARAMETER_NOT_NULLABLE, [origin,`ParameterNotNullable$ConstructorCall(...)` needs a non-null value in parameter 2 but argument `null` can be null. (Origin: null constant at line 102)]
 codetoanalyze/java/eradicate/ParameterNotNullable.java, String ParameterNotNullable.testSystemGetPropertyArgument(), 1, ERADICATE_PARAMETER_NOT_NULLABLE, [origin,`getProperty(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 71)]
 codetoanalyze/java/eradicate/ParameterNotNullable.java, String ParameterNotNullable.testSystemGetenvBad(), 1, ERADICATE_PARAMETER_NOT_NULLABLE, [origin,`getenv(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 76)]