From 73f3c5e0cda5b1a944d67cbaaeceb0c6d2a0dd72 Mon Sep 17 00:00:00 2001
From: Sam Blackshear <shb@fb.com>
Date: Fri, 27 Jan 2017 09:31:56 -0800
Subject: [PATCH] [cleanup] separate functions for checking annotations of
 return value and parameters

Summary:
`pdesc_has_annot` checks the annotations of both the return values and the parameters, which seems like a bad idea in general.
The client should have to specify which annotations they actually care about.
Converting existing uses of `pdesc_has_annot` to what I read as the intended behavior (checking the return annotation).
Will make better use of the other new functions in a follow-up.

Reviewed By: jeremydubreil

Differential Revision: D4469885

fbshipit-source-id: de5531e
---
 infer/src/backend/rearrange.ml     |  2 +-
 infer/src/checkers/annotations.ml  | 19 +++++++++++++------
 infer/src/checkers/annotations.mli | 11 ++++++++++-
 infer/src/checkers/checkers.ml     |  2 +-
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/infer/src/backend/rearrange.ml b/infer/src/backend/rearrange.ml
index 4a295c924..602546b0e 100644
--- a/infer/src/backend/rearrange.ml
+++ b/infer/src/backend/rearrange.ml
@@ -845,7 +845,7 @@ let add_guarded_by_constraints tenv prop lexp pdesc =
             | _ -> false)
           prop.Prop.sigma in
       Procdesc.get_access pdesc <> PredSymb.Private &&
-      not (Annotations.pdesc_has_annot pdesc Annotations.visibleForTesting) &&
+      not (Annotations.pdesc_return_annot_ends_with pdesc Annotations.visibleForTesting) &&
       not (Procname.java_is_access_method (Procdesc.get_proc_name pdesc)) &&
       not (is_accessible_through_local_ref lexp) &&
       not guardedby_is_self_referential &&
diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml
index a91ef38ab..32ccecad0 100644
--- a/infer/src/checkers/annotations.ml
+++ b/infer/src/checkers/annotations.ml
@@ -58,7 +58,7 @@ let thread_safe_method = "ThreadSafeMethod"
 let true_on_null = "TrueOnNull"
 let ui_thread = "UiThread"
 let verify_annotation = "com.facebook.infer.annotation.Verify"
-let visibleForTesting = "com.google.common.annotations.VisibleForTesting"
+let visibleForTesting = "VisibleForTesting"
 let volatile = "volatile"
 
 (** Method signature with annotations. *)
@@ -94,13 +94,20 @@ let ia_get ia ann_name =
   try Some (fst (IList.find (class_name_matches ann_name) ia))
   with Not_found -> None
 
-let pdesc_has_annot pdesc annot =
-  let f (a : Annot.t) = String.equal a.class_name annot in
-  ma_has_annotation_with (Procdesc.get_attributes pdesc).ProcAttributes.method_annotation f
+let pdesc_has_parameter_annot pdesc predicate =
+  let _, param_annotations = (Procdesc.get_attributes pdesc).ProcAttributes.method_annotation in
+  IList.exists predicate param_annotations
 
-let field_has_annot fieldname (struct_typ : StructTyp.t) f =
+let pdesc_has_return_annot pdesc predicate =
+  let return_annotation, _ = (Procdesc.get_attributes pdesc).ProcAttributes.method_annotation in
+  predicate return_annotation
+
+let pdesc_return_annot_ends_with pdesc annot =
+  pdesc_has_return_annot pdesc (fun ia -> ia_ends_with ia annot)
+
+let field_has_annot fieldname (struct_typ : StructTyp.t) predicate =
   let fld_has_taint_annot (fname, _, annot) =
-    Ident.equal_fieldname fieldname fname && f annot in
+    Ident.equal_fieldname fieldname fname && predicate annot in
   IList.exists fld_has_taint_annot struct_typ.fields ||
   IList.exists fld_has_taint_annot struct_typ.statics
 
diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli
index b8649d6ce..0d8182975 100644
--- a/infer/src/checkers/annotations.mli
+++ b/infer/src/checkers/annotations.mli
@@ -111,7 +111,16 @@ val ia_is_ui_thread : Annot.Item.t -> bool
 val ia_is_thread_confined : Annot.Item.t -> bool
 val ia_is_volatile : Annot.Item.t -> bool
 
-val pdesc_has_annot : Procdesc.t -> string -> bool
+(** return true if the given predicate evaluates to true on an annotation of one of [pdesc]'s
+    parameters *)
+val pdesc_has_parameter_annot : Procdesc.t -> (Annot.Item.t -> bool) -> bool
+
+(** return true if the given predicate evaluates to true on the annotation of [pdesc]'s return
+    value *)
+val pdesc_has_return_annot : Procdesc.t -> (Annot.Item.t -> bool) -> bool
+
+(** return true if [pdesc]'s return value is annotated with a value ending with the given string *)
+val pdesc_return_annot_ends_with : Procdesc.t -> string -> bool
 
 val ma_has_annotation_with : Annot.Method.t -> (Annot.t -> bool) -> bool
 
diff --git a/infer/src/checkers/checkers.ml b/infer/src/checkers/checkers.ml
index c884a498b..0d64ca31d 100644
--- a/infer/src/checkers/checkers.ml
+++ b/infer/src/checkers/checkers.ml
@@ -404,7 +404,7 @@ let callback_test_state { Callbacks.proc_name } =
 
 (** Check the uses of VisibleForTesting *)
 let callback_checkVisibleForTesting { Callbacks.proc_desc } =
-  if Annotations.pdesc_has_annot proc_desc Annotations.visibleForTesting then
+  if Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting then
     begin
       let loc = Procdesc.get_loc proc_desc in
       let linereader = Printer.LineReader.create () in