[thread-safety] remove special treatment for immutable collections

Summary:
This was a crutch from the days before ownership analysis.
We shouldn't need it anymore, and it was actually causing FP's because we were skipping analysis of `ImmutableList.builder()` and not understanding that the return value is owned.

Reviewed By: jeremydubreil

Differential Revision: D6035631

fbshipit-source-id: afa0ade

infer/src/checkers/ThreadSafety.ml

@@ -932,23 +932,6 @@ end
 module Analyzer =
   AbstractInterpreter.Make (ProcCfg.Normal) (LowerHil.MakeDefault (TransferFunctions))
 
-(* 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 _ -> List.mem ~equal:String.equal immutable_collections (Typ.Name.name typename))
-    class_name
-
-let is_call_to_immutable_collection_method tenv = function
-  | Typ.Procname.Java java_pname
-   -> is_immutable_collection_class (Typ.Procname.java_get_class_type_name java_pname) tenv
-  | _
-   -> false
-
 (* Methods in @ThreadConfined classes and methods annotated with @ThreadConfined are assumed to all
    run on the same thread. For the moment we won't warn on accesses resulting from use of such
    methods at all. In future we should account for races between these methods and methods from
@@ -1005,7 +988,6 @@ let pdesc_is_assumed_thread_safe pdesc tenv =
 let should_analyze_proc pdesc tenv =
   let pn = Procdesc.get_proc_name pdesc in
   not (Typ.Procname.is_class_initializer pn) && not (FbThreadSafety.is_logging_method pn)
-  && not (is_call_to_immutable_collection_method tenv pn)
   && not (pdesc_is_assumed_thread_safe pdesc tenv)
 
 let get_current_class_and_threadsafe_superclasses tenv pname =

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

@@ -12,7 +12,6 @@ package codetoanalyze.java.checkers;
 import com.facebook.infer.annotation.ThreadSafe;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.MyImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 
 public class Builders {
@@ -80,11 +79,6 @@ public class Builders {
     return builder.setFromObj(obj).build();
   }
 
-  @ThreadSafe
-  public void writeImmutableListFieldOk(MyImmutableList<Object> list) {
-    list.writeFld();
-  }
-
   @ThreadSafe
   public Obj mutateBad(Obj o) {
     o.g = "";

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

@@ -1,22 +0,0 @@
-/*
- * 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();
-  }
-
-}