[thread-safety] don't report on fields of immutable collections

Summary:
If these collections don't encapsulate their state properly, there are bigger problems than thread safety issues :).
Plus, these warnings are less-than-actionable for non-Guava maintainers.

Reviewed By: peterogithub

Differential Revision: D4324277

fbshipit-source-id: cacfbf0

infer/src/checkers/ThreadSafety.ml

@@ -30,10 +30,26 @@ module Summary = Summary.Make (struct
 let is_builder_class class_name =
   String.is_suffix ~suffix:"Builder" class_name
 
+(* similarly, we assume that immutable classes safely encapsulate their state *)
+let is_immutable_collection_class class_name tenv =
+  let immutable_collections = [
+    "com.google.common.collect.ImmutableCollection";
+    "com.google.common.collect.ImmutableMap";
+    "com.google.common.collect.ImmutableTable";
+  ] in
+  PatternMatch.supertype_exists
+    tenv (fun typename _ -> IList.mem (=) (Typename.name typename) immutable_collections) class_name
+
 let is_call_to_builder_class_method = function
   | Procname.Java java_pname -> is_builder_class (Procname.java_get_class_name java_pname)
   | _ -> false
 
+let is_call_to_immutable_collection_method tenv = function
+  | Procname.Java java_pname ->
+      is_immutable_collection_class (Procname.java_get_class_type_name java_pname) tenv
+  | _ ->
+      false
+
 let is_initializer tenv proc_name =
   Procname.is_constructor proc_name ||
   Procname.is_class_initializer proc_name ||
@@ -143,7 +159,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
                          (e.g., constructors that access static fields) *)
                       if is_unprotected locks' &&
                          not (is_initializer tenv pn) &&
-                         not (is_call_to_builder_class_method pn)
+                         not (is_call_to_builder_class_method pn) &&
+                         not (is_call_to_immutable_collection_method tenv pn)
                       then
                         let call_site = CallSite.make pn loc in
                         let reads' =

infer/tests/codetoanalyze/java/threadsafety/Builders.java

@@ -10,6 +10,7 @@
 package codetoanalyze.java.checkers;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.MyImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 
 @ThreadSafe
@@ -70,6 +71,10 @@ public class Builders {
     return builder.setFromObj(input).build();
   }
 
+  public void writeImmutableListFieldOk(MyImmutableList<Object> list) {
+    list.writeFld();
+  }
+
   public Obj mutateBad(Obj o) {
     o.g = "";
     return o;

infer/tests/codetoanalyze/java/threadsafety/MyImmutableList.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+// have to pretend we're in the same package to access protected constructor
+package com.google.common.collect;
+
+import com.google.common.collect.ImmutableList;
+
+abstract public class MyImmutableList<T> extends ImmutableList<T> {
+  private Object mFld;
+
+  public void writeFld() {
+    mFld = new Object();
+  }
+
+}