allowing class-level Expensive and PerformanceCritical annotations

Reviewed By: cristianoc

Differential Revision: D3158000

fb-gh-sync-id: 88c78a3
fbshipit-source-id: 88c78a3

infer/annotations/com/facebook/infer/annotation/Expensive.java

@@ -15,5 +15,5 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 @Retention(RetentionPolicy.CLASS)
-@Target(ElementType.METHOD)
+@Target({ ElementType.METHOD, ElementType.TYPE })
 public @interface Expensive {}

infer/annotations/com/facebook/infer/annotation/PerformanceCritical.java

@@ -15,5 +15,5 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 @Retention(RetentionPolicy.CLASS)
-@Target(ElementType.METHOD)
+@Target(value={ElementType.METHOD, ElementType.TYPE})
 public @interface PerformanceCritical {}

infer/src/checkers/annotations.ml

@@ -44,6 +44,19 @@ let get_field_type_and_annotation fn = function
        with Not_found -> None)
   | _ -> None
 
+(** Return the annotations on the declaring class of [pname]. Only works for Java *)
+let get_declaring_class_annotations pname tenv =
+  match pname with
+  | Procname.Java pname_java ->
+      let receiver_typ_str = Procname.java_get_class_name pname_java in
+      begin
+        match Tenv.lookup_java_typ_from_string tenv receiver_typ_str with
+        | Sil.Tstruct { struct_annotations; } -> Some struct_annotations
+        | exception Tenv.Cannot_convert_string_to_typ _ -> None
+        | _ -> None
+      end
+  | _ -> None
+
 let ia_iter f =
   let ann_iter (a, _) = f a in
   IList.iter ann_iter

infer/src/checkers/annotations.mli

@@ -54,6 +54,9 @@ val get_annotated_signature : ProcAttributes.t -> annotated_signature
 val get_field_type_and_annotation :
   Ident.fieldname -> Sil.typ -> (Sil.typ * Sil.item_annotation) option
 
+(** Return the annotations on the declaring class of [pname]. Only works for Java *)
+val get_declaring_class_annotations : Procname.t -> Tenv.t -> Sil.item_annotation option
+
 val nullable : string
 
 val ia_contains : Sil.item_annotation -> string -> bool

infer/src/checkers/performanceCritical.ml

@@ -39,39 +39,41 @@ let is_modeled_expensive =
         false
 
 
-let check_attributes check attributes =
+let check_attributes check tenv pname =
+  let check_class_attributes check tenv pname =
+    match Annotations.get_declaring_class_annotations pname tenv with
+    | Some annotations -> check annotations
+    | None -> false in
+  let check_method_attributes check pname =
+    match Specs.proc_resolve_attributes pname with
+    | None -> false
+    | Some attributes ->
         let annotated_signature = Annotations.get_annotated_signature attributes in
         let ret_annotation, _ = annotated_signature.Annotations.ret in
-  check ret_annotation
-
-
-let is_expensive attributes =
-  check_attributes Annotations.ia_is_expensive attributes
+        check ret_annotation in
+  check_class_attributes check tenv pname || check_method_attributes check pname
 
 
-let check_method check pname =
-  match Specs.proc_resolve_attributes pname with
-  | None -> false
-  | Some attributes ->
-      check_attributes check attributes
+let is_expensive tenv pname =
+  check_attributes Annotations.ia_is_expensive tenv pname
 
 
-let method_is_performance_critical pname =
-  check_method Annotations.ia_is_performance_critical pname
+let method_is_performance_critical tenv pname =
+  check_attributes Annotations.ia_is_performance_critical tenv pname
 
 
-let method_is_no_allcation pname =
-  check_method Annotations.ia_is_no_allocation pname
+let method_is_no_allocation tenv pname =
+  check_attributes Annotations.ia_is_no_allocation tenv pname
 
 
 let method_overrides is_annotated tenv pname =
   let overrides () =
     let found = ref false in
     PatternMatch.proc_iter_overridden_methods
-      (fun pn -> found := is_annotated pn)
+      (fun pn -> found := is_annotated tenv pn)
       tenv pname;
     !found in
-  is_annotated pname
+  is_annotated tenv pname
   || overrides ()
 
 
@@ -80,12 +82,12 @@ let method_overrides_performance_critical tenv pname =
 
 
 let method_overrides_no_allocation tenv pname =
-  method_overrides method_is_no_allcation tenv pname
+  method_overrides method_is_no_allocation tenv pname
 
 
 let method_is_expensive tenv pname =
-  is_modeled_expensive tenv pname
-  || check_method Annotations.ia_is_expensive pname
+  is_modeled_expensive tenv pname ||
+  check_attributes Annotations.ia_is_expensive tenv pname
 
 
 let lookup_call_summary pname =
@@ -131,7 +133,7 @@ let is_allocator tenv pname = match pname with
 
 let method_allocates tenv pname =
   let annotated_ignore_allocation =
-    check_method Annotations.ia_is_ignore_allocations pname in
+    check_attributes Annotations.ia_is_ignore_allocations tenv pname in
   let allocates () =
     match lookup_call_summary pname with
     | Some { Specs.allocations } ->
@@ -265,10 +267,8 @@ let report_allocations pname pdesc loc calls =
 
 
 let check_one_procedure tenv pname pdesc =
-
   let loc = Cfg.Procdesc.get_loc pdesc in
-  let attributes = Cfg.Procdesc.get_attributes pdesc in
-  let expensive = is_expensive attributes
+  let expensive = is_expensive tenv pname
   and performance_critical =
     method_overrides_performance_critical tenv pname
   and no_allocation =
@@ -323,8 +323,3 @@ let callback_performance_checker
       check_one_procedure tenv proc_name proc_desc;
       Ondemand.unset_callbacks ()
     end
-
-(*
-let is_performance_critical attributes =
-  check_attributes Annotations.ia_is_performance_critical attributes
-*)

infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java

@@ -35,6 +35,66 @@ class Other {
     expensive();
   }
 
+  void inexpensiveMethod() {
+  }
+
+}
+
+@Expensive
+class ExpensiveClass {
+
+  void anExpensiveMethod() {
+  }
+
+}
+
+@PerformanceCritical
+class PerformanceCriticalClass {
+
+  void performanceCriticalMethod1(ExpensiveClass c) {
+    c.anExpensiveMethod(); // should report
+  }
+
+  void performanceCriticalMethod2(Other o) {
+    o.expensive(); // should report
+  }
+
+  void performanceCriticalMethod3(Other o) {
+    o.callsExpensive1(); // should report
+  }
+
+  void performanceCriticalMethod4(Other o) {
+    o.inexpensiveMethod(); // should not report
+  }
+
+
+}
+
+class ExpensiveSubclass extends ExpensiveClass {
+
+  void anotherExpensiveMethod() {
+  }
+
+}
+
+class PerformanceCriticalSubclass extends PerformanceCriticalClass {
+
+  void subclassPerformanceCriticalMethod1(ExpensiveClass c) {
+    c.anExpensiveMethod(); // should report
+  }
+
+  void subclassPerformanceCriticalMethod2(ExpensiveSubclass c) {
+    c.anotherExpensiveMethod(); // should report
+  }
+
+  void subclassPerformanceCriticalMethod3(Other o) {
+    o.callsExpensive1(); // should report;
+  }
+
+  void subclassPerformanceCriticalMethod4(Other o) {
+    o.inexpensiveMethod(); // should not report;
+  }
+
 }
 
 
@@ -93,6 +153,11 @@ public class ExpensiveCallExample implements AnnotatedInterface {
     return activity.findViewById(id);
   }
 
+  @PerformanceCritical
+  void callMethodOnExpensiveClass(ExpensiveClass c) {
+    c.anExpensiveMethod();
+  }
+
   public void annotatedPerformanceCriticalInInterface() {
     mOther.callsExpensive1();
   }

infer/tests/endtoend/java/checkers/ExpensiveCallTest.java

@@ -47,6 +47,14 @@ public class ExpensiveCallTest {
         "callsFindViewByIdFromView",
         "callsFindViewByIdFromActivity",
         "annotatedPerformanceCriticalInInterface",
+        "performanceCriticalMethod1",
+        "performanceCriticalMethod2",
+        "performanceCriticalMethod3",
+        // TODO: make subclassing work
+        //"subclassPerformanceCriticalMethod1",
+        //"subclassPerformanceCriticalMethod2",
+        //"subclassPerformanceCriticalMethod3",
+        "callMethodOnExpensiveClass"
     };
     assertThat(
         "Results should contain " + CALLS_EXPENSIVE_METHOD,