[annot-reach] complete the set of android thread annotations

Summary:
Treat `MainThread` and `WorkerThread` annotations.

Fix wrong test (`AnyThread` cannot call a UI-only method, because it can be called by ANY thread ;) ) See https://developer.android.com/reference/android/support/annotation/AnyThread

Clean up the code a bit.

Reviewed By: jvillard

Differential Revision: D16183798

fbshipit-source-id: 6b7e3b27e

infer/src/base/IssueType.ml

@@ -136,14 +136,18 @@ let buffer_overrun_u5 = from_string ~enabled:false "BUFFER_OVERRUN_U5"
 
 let cannot_star = from_string "Cannot_star"
 
-let checkers_allocates_memory = from_string "CHECKERS_ALLOCATES_MEMORY"
+let checkers_allocates_memory = from_string "CHECKERS_ALLOCATES_MEMORY" ~hum:"Allocates Memory"
 
-let checkers_annotation_reachability_error = from_string "CHECKERS_ANNOTATION_REACHABILITY_ERROR"
+let checkers_annotation_reachability_error =
+  from_string "CHECKERS_ANNOTATION_REACHABILITY_ERROR" ~hum:"Annotation Reachability Error"
+
+
+let checkers_calls_expensive_method =
+  from_string "CHECKERS_CALLS_EXPENSIVE_METHOD" ~hum:"Expensive Method Called"
 
-let checkers_calls_expensive_method = from_string "CHECKERS_CALLS_EXPENSIVE_METHOD"
 
 let checkers_expensive_overrides_unexpensive =
-  from_string "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED"
+  from_string "CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED" ~hum:"Expensive Overrides Unannotated"
 
 
 let checkers_fragment_retain_view =

infer/src/checkers/annotationReachability.ml

@@ -252,14 +252,14 @@ module AnnotationSpec = struct
 end
 
 module StandardAnnotationSpec = struct
-  let from_annotations src_annots snk_annot =
+  let from_annotations str_src_annots str_snk_annot =
+    let src_annots = List.map str_src_annots ~f:annotation_of_str in
+    let snk_annot = annotation_of_str str_snk_annot in
+    let has_annot ia = Annotations.ia_ends_with ia snk_annot.Annot.class_name in
     let open AnnotationSpec in
     { source_predicate=
         (fun tenv pname -> List.exists src_annots ~f:(fun a -> method_overrides_annot a tenv pname))
-    ; sink_predicate=
-        (fun tenv pname ->
-          let has_annot ia = Annotations.ia_ends_with ia snk_annot.Annot.class_name in
-          check_attributes has_annot tenv pname )
+    ; sink_predicate= (fun tenv pname -> check_attributes has_annot tenv pname)
     ; sanitizer_predicate= default_sanitizer
     ; sink_annotation= snk_annot
     ; report=
@@ -491,23 +491,22 @@ let parse_user_defined_specs = function
 
 
 let annot_specs =
+  let user_defined_specs =
+    parse_user_defined_specs Config.annotation_reachability_custom_pairs
+    |> List.map ~f:(fun (str_src_annots, str_snk_annot) ->
+           StandardAnnotationSpec.from_annotations str_src_annots str_snk_annot )
+  in
+  let open Annotations in
+  let cannot_call_ui_annots = [any_thread; for_non_ui_thread; worker_thread] in
+  let cannot_call_non_ui_annots = [any_thread; for_ui_thread; mainthread; ui_thread] in
   [ (Language.Clang, CxxAnnotationSpecs.from_config ())
   ; ( Language.Java
-    , let user_defined_specs =
-        let specs = parse_user_defined_specs Config.annotation_reachability_custom_pairs in
-        List.map specs ~f:(fun (src_annots, snk_annot) ->
-            StandardAnnotationSpec.from_annotations
-              (List.map ~f:annotation_of_str src_annots)
-              (annotation_of_str snk_annot) )
-      in
-      ExpensiveAnnotationSpec.spec :: NoAllocationAnnotationSpec.spec
-      :: StandardAnnotationSpec.from_annotations
-           [ annotation_of_str Annotations.any_thread
-           ; annotation_of_str Annotations.for_non_ui_thread ]
-           (annotation_of_str Annotations.ui_thread)
-      :: StandardAnnotationSpec.from_annotations
-           [annotation_of_str Annotations.ui_thread; annotation_of_str Annotations.for_ui_thread]
-           (annotation_of_str Annotations.for_non_ui_thread)
+    , ExpensiveAnnotationSpec.spec :: NoAllocationAnnotationSpec.spec
+      :: StandardAnnotationSpec.from_annotations cannot_call_ui_annots ui_thread
+      :: StandardAnnotationSpec.from_annotations cannot_call_ui_annots mainthread
+      :: StandardAnnotationSpec.from_annotations cannot_call_ui_annots for_ui_thread
+      :: StandardAnnotationSpec.from_annotations cannot_call_non_ui_annots worker_thread
+      :: StandardAnnotationSpec.from_annotations cannot_call_non_ui_annots for_non_ui_thread
       :: user_defined_specs ) ]
 
 

infer/src/checkers/annotations.mli

@@ -45,6 +45,8 @@ val mainthread : string
 
 val ui_thread : string
 
+val worker_thread : string
+
 val visibleForTesting : string
 
 val generated_graphql : string

infer/tests/codetoanalyze/java/checkers/UiThreads.java

@@ -7,7 +7,9 @@
 
 package codetoanalyze.java.checkers;
 
+import android.support.annotation.MainThread;
 import android.support.annotation.UiThread;
+import android.support.annotation.WorkerThread;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -30,6 +32,9 @@ public class UiThreads {
   @UiThread
   void uiThread() {}
 
+  @MainThread
+  void mainThread() {}
+
   @AnyThread
   void anyThread() {}
 
@@ -49,6 +54,16 @@ public class UiThreads {
     forNonUiThread();
   }
 
+  @MainThread
+  void callForNonUiThreadBad3() {
+    forNonUiThread();
+  }
+
+  @AnyThread
+  void callForNonUiThreadBad4() {
+    forNonUiThread();
+  }
+
   @AnyThread
   void callUiThreadBad1() {
     uiThread();
@@ -59,6 +74,11 @@ public class UiThreads {
     uiThread();
   }
 
+  @WorkerThread
+  void callUiThreadBad3() {
+    uiThread();
+  }
+
   @ForUiThread
   void callUiThreadOk() {
     uiThread();
@@ -83,9 +103,4 @@ public class UiThreads {
   void callAnyThreadOk3() {
     anyThread();
   }
-
-  @AnyThread
-  void callForNonUiThreadOk() {
-    forNonUiThread();
-  }
 }

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

@@ -52,5 +52,8 @@ codetoanalyze/java/checkers/TwoCheckersExample.java, codetoanalyze.java.checkers
 codetoanalyze/java/checkers/TwoCheckersExample.java, codetoanalyze.java.checkers.TwoCheckersExample.shouldRaisePerformanceCriticalError():java.util.List, 1, CHECKERS_CALLS_EXPENSIVE_METHOD, no_bucket, ERROR, []
 codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callForNonUiThreadBad1():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
 codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callForNonUiThreadBad2():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
+codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callForNonUiThreadBad3():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
+codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callForNonUiThreadBad4():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
 codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callUiThreadBad1():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
 codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callUiThreadBad2():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
+codetoanalyze/java/checkers/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callUiThreadBad3():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []