[required-props] Add call chain to trace

Summary:
Improve the trace by incorporating the callees and their locations in the call chain (i.e. chain of methods starting from `build()` call)

- extend the domain to contain the callee location
- replace the test results with the new traces

This makes our job much easier to debug FPs in a big codebase.

Reviewed By: skcho

Differential Revision: D17788996

fbshipit-source-id: 31938b5fe

infer/src/checkers/GraphQLFieldAccess.ml

@@ -43,7 +43,10 @@ module LithoContext = struct
   let report astate _tenv summary =
     let report_graphql_getter access_path call_chain =
       let call_strings =
-        List.map ~f:(Typ.Procname.to_simplified_string ~withclass:false) call_chain
+        List.map
+          ~f:(fun Domain.MethodCall.{procname} ->
+            Typ.Procname.to_simplified_string ~withclass:false procname )
+          call_chain
       in
       let call_string = String.concat ~sep:"." call_strings in
       let message = F.asprintf "%a.%s" AccessPath.pp access_path call_string in

infer/src/checkers/LithoDomain.ml

@@ -27,9 +27,10 @@ module LocalAccessPath = struct
 end
 
 module MethodCall = struct
-  type t = {receiver: LocalAccessPath.t; procname: Typ.Procname.t} [@@deriving compare]
+  type t = {receiver: LocalAccessPath.t; procname: Typ.Procname.t; location: Location.t}
+  [@@deriving compare]
 
-  let make receiver procname = {receiver; procname}
+  let make receiver procname location = {receiver; procname; location}
 
   let pp fmt {receiver; procname} =
     F.fprintf fmt "%a.%a" LocalAccessPath.pp receiver Typ.Procname.pp procname
@@ -50,11 +51,11 @@ let substitute ~(f_sub : LocalAccessPath.t -> LocalAccessPath.t option) astate =
       in
       let call_set' =
         CallSet.fold
-          (fun ({procname} as call) call_set_acc ->
+          (fun ({procname; location} as call) call_set_acc ->
             let receiver =
               match f_sub call.receiver with Some receiver' -> receiver' | None -> call.receiver
             in
-            CallSet.add {receiver; procname} call_set_acc )
+            CallSet.add {receiver; procname; location} call_set_acc )
           call_set CallSet.empty
       in
       add access_path' call_set' acc )
@@ -65,12 +66,12 @@ let substitute ~(f_sub : LocalAccessPath.t -> LocalAccessPath.t option) astate =
     maximal chain. For example, if the domain encodes the chains foo().bar().goo() and foo().baz(),
     [f] will be called once on foo().bar().goo() and once on foo().baz() *)
 let iter_call_chains_with_suffix ~f call_suffix astate =
-  let rec unroll_call_ ({receiver; procname} : MethodCall.t) (acc, visited) =
+  let rec unroll_call_ ({receiver; procname} as call : MethodCall.t) (acc, visited) =
     let is_cycle (call : MethodCall.t) =
       (* detect direct cycles and cycles due to mutual recursion *)
       LocalAccessPath.equal call.receiver receiver || Typ.Procname.Set.mem call.procname visited
     in
-    let acc' = procname :: acc in
+    let acc' = call :: acc in
     let visited' = Typ.Procname.Set.add procname visited in
     try
       let calls' = find receiver astate in

infer/src/checkers/LithoDomain.mli

@@ -19,11 +19,12 @@ module LocalAccessPath : sig
   val pp : F.formatter -> t -> unit
 end
 
-(** Called procedure + its receiver *)
+(** Called procedure & location + its receiver *)
 module MethodCall : sig
-  type t = private {receiver: LocalAccessPath.t; procname: Typ.Procname.t} [@@deriving compare]
+  type t = private {receiver: LocalAccessPath.t; procname: Typ.Procname.t; location: Location.t}
+  [@@deriving compare]
 
-  val make : LocalAccessPath.t -> Typ.Procname.t -> t
+  val make : LocalAccessPath.t -> Typ.Procname.t -> Location.t -> t
 
   val pp : F.formatter -> t -> unit
 end
@@ -36,5 +37,5 @@ val substitute : f_sub:(LocalAccessPath.t -> LocalAccessPath.t option) -> t -> t
 (** Substitute each access path in the domain using [f_sub]. If [f_sub] returns None, the
     original access path is retained; otherwise, the new one is used *)
 
-val iter_call_chains : f:(AccessPath.t -> Typ.Procname.t list -> unit) -> t -> unit
+val iter_call_chains : f:(AccessPath.t -> MethodCall.t list -> unit) -> t -> unit
 (** Apply [f] to each maximal call chain encoded in [t] *)

infer/src/checkers/LithoFramework.ml

@@ -103,7 +103,7 @@ struct
         , Direct (Typ.Procname.Java java_callee_procname as callee_pname)
         , (HilExp.AccessExpression receiver_ae :: _ as actuals)
         , _
-        , _ ) ->
+        , location ) ->
         let domain_summary = Payload.read ~caller_summary:summary ~callee_pname in
         let receiver =
           Domain.LocalAccessPath.make
@@ -122,7 +122,7 @@ struct
           let return_calls =
             ( try Domain.find return_access_path astate
               with Caml.Not_found -> Domain.CallSet.empty )
-            |> Domain.CallSet.add (Domain.MethodCall.make receiver callee_pname)
+            |> Domain.CallSet.add (Domain.MethodCall.make receiver callee_pname location)
           in
           Domain.add return_access_path return_calls astate
         else

infer/src/checkers/RequiredProps.ml

@@ -58,7 +58,7 @@ let get_required_props typename tenv =
       []
 
 
-let report_missing_required_prop summary prop parent_typename loc =
+let report_missing_required_prop summary prop parent_typename loc call_chain =
   let message =
     let prop_string =
       match prop with
@@ -70,7 +70,12 @@ let report_missing_required_prop summary prop parent_typename loc =
     F.asprintf "%s is required for component %s, but is not set before the call to build()"
       prop_string (Typ.Name.name parent_typename)
   in
-  let ltr = [Errlog.make_trace_element 0 loc message []] in
+  let ltr =
+    Errlog.make_trace_element 0 loc message []
+    :: List.map call_chain ~f:(fun Domain.MethodCall.{procname; location} ->
+           let call_msg = F.asprintf "calls %a" Typ.Procname.pp procname in
+           Errlog.make_trace_element 0 location call_msg [] )
+  in
   Reporting.log_error summary ~loc ~ltr IssueType.missing_required_prop message
 
 
@@ -78,8 +83,8 @@ let report_missing_required_prop summary prop parent_typename loc =
      the Litho framework (i.e., is client code) *)
 let find_client_component_type call_chain =
   List.find_map
-    ~f:(fun pname ->
-      match pname with
+    ~f:(fun Domain.MethodCall.{procname} ->
+      match procname with
       | Typ.Procname.Java java_pname ->
           Typ.Name.Java.get_outer_class (Typ.Procname.Java.get_class_type_name java_pname)
       | _ ->
@@ -131,19 +136,25 @@ module LithoContext = struct
     let check_required_prop_chain _ call_chain =
       let rev_chain = List.rev call_chain in
       match rev_chain with
-      | pname :: _ when LithoFramework.is_component_build_method pname tenv -> (
+      | Domain.MethodCall.{procname} :: _
+        when LithoFramework.is_component_build_method procname tenv -> (
         (* Here, we'll have a type name like MyComponent$Builder in hand. Truncate the $Builder
                part from the typename, then look at the fields of MyComponent to figure out which
                ones are annotated with @Prop *)
         match find_client_component_type call_chain with
         | Some parent_typename ->
             let required_props = get_required_props parent_typename tenv in
-            let prop_set = List.map ~f:Typ.Procname.get_method call_chain |> String.Set.of_list in
+            let prop_set =
+              List.map
+                ~f:(fun Domain.MethodCall.{procname} -> Typ.Procname.get_method procname)
+                call_chain
+              |> String.Set.of_list
+            in
             List.iter
               ~f:(fun required_prop ->
                 if not (has_prop prop_set required_prop) then
                   report_missing_required_prop summary required_prop parent_typename
-                    (Summary.get_loc summary) )
+                    (Summary.get_loc summary) call_chain )
               required_props
         | _ ->
             () )

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

@@ -1,13 +1,13 @@
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.FP_setRequiredOnBothBranchesOk(boolean):com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResMissingBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop is required for component codetoanalyze.java.litho.ResPropComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropVarArgMissingBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [Either @Prop props or @Prop(varArg = prop) is required for component codetoanalyze.java.litho.VarArgPropComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithColumnChildBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout1Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout3Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.callBuildSuffixWithoutRequiredBad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.generalTypeForgot3Bad():java.lang.Object, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setProp3InCalleeButForgetProp1Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
-codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchBad(boolean):com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.FP_setRequiredOnBothBranchesOk(boolean):com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResMissingBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop is required for component codetoanalyze.java.litho.ResPropComponent, but is not set before the call to build(),calls ResPropComponent ResPropComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropVarArgMissingBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [Either @Prop props or @Prop(varArg = prop) is required for component codetoanalyze.java.litho.VarArgPropComponent, but is not set before the call to build(),calls VarArgPropComponent VarArgPropComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithColumnChildBad():void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop1(Object),calls Component Component$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout1Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout3Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop1(Object),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.callBuildSuffixWithoutRequiredBad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.generalTypeForgot3Bad():java.lang.Object, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop3 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop1(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setProp3InCalleeButForgetProp1Bad():com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop3(Object),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchBad(boolean):com.facebook.litho.Component, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component codetoanalyze.java.litho.MyComponent, but is not set before the call to build(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object),calls MyComponent MyComponent$Builder.build()]
 codetoanalyze/java/litho/ShouldUpdate.java, com.facebook.graphql.model.LithoTest.onCreateLayout():void, 0, GRAPHQL_FIELD_ACCESS, no_bucket, ERROR, [story.getActors()]
 codetoanalyze/java/litho/ShouldUpdate.java, com.facebook.graphql.model.LithoTest.onCreateLayout():void, 0, GRAPHQL_FIELD_ACCESS, no_bucket, ERROR, [story.getActors().get(...).toString()]
 codetoanalyze/java/litho/ShouldUpdate.java, com.facebook.graphql.model.LithoTest.onCreateLayout():void, 0, GRAPHQL_FIELD_ACCESS, no_bucket, ERROR, [story.getActors().get(...)]