[threadsafety] Fix situation where @NotThreadSafe is reported on, when super is @ThreadSafe

Summary: Don't warn on NotThreadSafe class, particularly when super is ThreadSafe

Reviewed By: sblackshear

Differential Revision: D4334417

fbshipit-source-id: 0df3b9d

infer/src/checkers/ThreadSafety.ml

@@ -364,6 +364,11 @@ let should_report_on_file file_env =
           not (thread_safe_annotated_classes = [])
       | _ -> false
   in
+  let current_class_marked_not_threadsafe =
+    fun (_, tenv, pname, _) ->
+      PatternMatch.check_current_class_attributes Annotations.ia_is_not_thread_safe tenv pname
+  in
+  not (IList.exists current_class_marked_not_threadsafe file_env) &&
   IList.exists current_class_or_super_marked_threadsafe file_env
 
 (*This is a "cluster checker" *)

infer/src/checkers/annotations.ml

@@ -115,10 +115,13 @@ let guarded_by = "GuardedBy"
 let thread_safe = "ThreadSafe"
 let not_thread_safe = "NotThreadSafe"
 
+let ia_is_not_thread_safe ia =
+  ia_ends_with ia not_thread_safe
+
 (* we don't want it to just end with ThreadSafe, because this falls
    foul of the @NotThreadSafe annotation *)
 let ia_is_thread_safe ia =
-  ia_ends_with ia thread_safe && not (ia_ends_with ia not_thread_safe)
+  ia_ends_with ia thread_safe
 
 let ia_is_nullable ia =
   ia_ends_with ia nullable

infer/src/checkers/annotations.mli

@@ -93,6 +93,7 @@ val ia_is_privacy_sink : Annot.Item.t -> bool
 val ia_is_integrity_source : Annot.Item.t -> bool
 val ia_is_integrity_sink : Annot.Item.t -> bool
 val ia_is_guarded_by : Annot.Item.t -> bool
+val ia_is_not_thread_safe : Annot.Item.t -> bool
 val ia_is_thread_safe : Annot.Item.t -> bool
 
 val ia_iter : (Annot.t -> unit) -> Annot.Item.t -> unit

infer/src/checkers/patternMatch.ml

@@ -371,7 +371,8 @@ let is_exception tenv typename =
 let is_throwable tenv typename =
   is_subtype_of_str tenv typename "java.lang.Throwable"
 
-(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument*)
+(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument,
+    including for supertypes*)
 let check_class_attributes check tenv = function
   | Procname.Java java_pname ->
       let check_class_annots _ { StructTyp.annots; } = check annots in
@@ -380,6 +381,17 @@ let check_class_attributes check tenv = function
         (Procname.java_get_class_type_name java_pname)
   | _ -> false
 
+(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument,
+    for the current class only*)
+let check_current_class_attributes check tenv = function
+  | Procname.Java java_pname ->
+      (match Tenv.lookup tenv (Procname.java_get_class_type_name java_pname) with
+       | Some (struct_typ) -> check struct_typ.annots
+       | _ -> false
+      )
+  | _ -> false
+
+
 (** find superclasss with attributes (e.g., @ThreadSafe), including current class*)
 let rec find_superclasses_with_attributes check tenv tname =
   match Tenv.lookup tenv tname with

infer/src/checkers/patternMatch.mli

@@ -111,9 +111,14 @@ val is_throwable : Tenv.t -> Typename.t -> bool
     of type java.lang.RuntimeException *)
 val is_runtime_exception : Tenv.t -> Typename.t -> bool
 
-(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument*)
+(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument,
+     including supertypes*)
 val check_class_attributes : (Annot.Item.t -> bool) -> Tenv.t -> Procname.t -> bool
 
+(** tests whether any class attributes (e.g., @ThreadSafe) pass check of first argument,
+    for current class only*)
+val check_current_class_attributes : (Annot.Item.t -> bool) -> Tenv.t -> Procname.t -> bool
+
 (** find superclasss with attributes (e.g., @ThreadSafe), including current class*)
 val find_superclasses_with_attributes : (Annot.Item.t -> bool) -> Tenv.t
   -> Typename.t -> Typename.t list

infer/tests/codetoanalyze/java/threadsafety/ThreadSafeExample.java

@@ -107,3 +107,27 @@ class ExtendsThreadSafeExample extends ThreadSafeExample{
   }
 
 }
+
+@NotThreadSafe
+class NotThreadSafeExtendsThreadSafeExample extends ThreadSafeExample{
+
+  Integer field;
+
+/* We don't want to warn on this */
+  public void newmethodBad() {
+     field = 22;
+  }
+
+}
+
+@ThreadSafe
+class YesThreadSafeExtendsNotThreadSafeExample extends NotThreadSafeExtendsThreadSafeExample{
+
+  Integer subsubfield;
+
+/* We do want to warn on this */
+  public void subsubmethodBad() {
+     subsubfield = 22;
+  }
+
+}

infer/tests/codetoanalyze/java/threadsafety/issues.exp

@@ -17,3 +17,4 @@ codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.o
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.recursiveBad(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.recursiveBad(), 2, THREAD_SAFETY_ERROR, [call to void ThreadSafeExample.recursiveBad(),access to codetoanalyze.java.checkers.ThreadSafeExample.f]
 codetoanalyze/java/threadsafety/ThreadSafeExample.java, void ThreadSafeExample.tsBad(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.ThreadSafeExample.f]
+codetoanalyze/java/threadsafety/ThreadSafeExample.java, void YesThreadSafeExtendsNotThreadSafeExample.subsubmethodBad(), 1, THREAD_SAFETY_ERROR, [access to codetoanalyze.java.checkers.YesThreadSafeExtendsNotThreadSafeExample.subsubfield]