[litho] Moved is_build_called and added is_return_called

Summary:
In order to handle the example added:

changed domain of `MethodCalled`
from `CreatedLocation -> (IsBuildCalled X IsChecked X Set(MethodCall))`
to `(CreatedLocation X IsBuildCalled) -> (IsChecked X Set(MethodCall))`

This avoids joining of two method calls where one is build-called and the other is not, e.g.,

```
if(b) {
  o.build();
} else {
  // no build call
}
```

changed domain of `NewDomain`
from `Created X MethodCalled`
to `(Created X MethodCalled) X (Created X MethodCalled)`
One is for no returned memory and the other is returned memory.  This keeps precision some join
points of branches, e.g.,

```
if(b) {
  return;
} else {
  // no return
}
```

Reviewed By: ezgicicek

Differential Revision: D18909768

fbshipit-source-id: c39d1a1ef

infer/src/IR/HilExp.ml

@@ -371,6 +371,9 @@ module AccessExpression = struct
     to_accesses y |> snd
     |> List.fold ~init:(Some onto) ~f:(fun acc access ->
            match acc with None -> acc | Some exp -> add_access exp access )
+
+
+  let is_return_var = function Base (var, _) -> Var.is_return var | _ -> false
 end
 
 let rec get_typ tenv = function

infer/src/IR/HilExp.mli

@@ -69,6 +69,8 @@ module AccessExpression : sig
 
   val is_base : access_expression -> bool
 
+  val is_return_var : access_expression -> bool
+
   val get_typ : access_expression -> Tenv.t -> Typ.t option
 
   val pp : Format.formatter -> access_expression -> unit

infer/src/IR/Typ.ml

@@ -576,6 +576,8 @@ let is_pointer_to_cpp_class typ = match typ.desc with Tptr (t, _) -> is_cpp_clas
 
 let is_pointer_to_void typ = match typ.desc with Tptr ({desc= Tvoid}, _) -> true | _ -> false
 
+let is_void typ = match typ.desc with Tvoid -> true | _ -> false
+
 let is_pointer_to_int typ = match typ.desc with Tptr ({desc= Tint _}, _) -> true | _ -> false
 
 let is_int typ = match typ.desc with Tint _ -> true | _ -> false

infer/src/IR/Typ.mli

@@ -303,6 +303,8 @@ val is_pointer_to_cpp_class : t -> bool
 
 val is_pointer_to_void : t -> bool
 
+val is_void : t -> bool
+
 val is_pointer_to_int : t -> bool
 
 val is_pointer : t -> bool

infer/src/backend/Payloads.ml

@@ -16,7 +16,7 @@ type t =
   ; class_loads: ClassLoadsDomain.summary option
   ; cost: CostDomain.summary option
   ; lab_resource_leaks: ResourceLeakDomain.summary option
-  ; litho_required_props: LithoDomain.t option
+  ; litho_required_props: LithoDomain.summary option
   ; pulse: PulseSummary.t option
   ; purity: PurityDomain.summary option
   ; quandary: QuandarySummary.t option
@@ -43,7 +43,7 @@ let fields =
     ~buffer_overrun_checker:(fun f -> mk f "BufferOverrunChecker" BufferOverrunCheckerSummary.pp)
     ~class_loads:(fun f -> mk f "ClassLoads" ClassLoadsDomain.pp_summary)
     ~cost:(fun f -> mk f "Cost" CostDomain.pp_summary)
-    ~litho_required_props:(fun f -> mk f "Litho Required Props" LithoDomain.pp)
+    ~litho_required_props:(fun f -> mk f "Litho Required Props" LithoDomain.pp_summary)
     ~pulse:(fun f -> mk f "Pulse" PulseSummary.pp)
     ~purity:(fun f -> mk f "Purity" PurityDomain.pp_summary)
     ~quandary:(fun f -> mk f "Quandary" QuandarySummary.pp)

infer/src/backend/Payloads.mli

@@ -20,7 +20,7 @@ include sig
     ; class_loads: ClassLoadsDomain.summary option
     ; cost: CostDomain.summary option
     ; lab_resource_leaks: ResourceLeakDomain.summary option
-    ; litho_required_props: LithoDomain.t option
+    ; litho_required_props: LithoDomain.summary option
     ; pulse: PulseSummary.t option
     ; purity: PurityDomain.summary option
     ; quandary: QuandarySummary.t option

infer/src/checkers/LithoDomain.ml

@@ -145,61 +145,42 @@ module NewDomain = struct
   end
 
   module MethodCalls = struct
-    module IsBuildMethodCalled = AbstractDomain.BooleanOr
-    (** if the build method has been called on the builder object *)
-
     module IsChecked = AbstractDomain.BooleanOr
     (** if the method calls are checked and reported *)
 
     module S = AbstractDomain.InvertedSet (MethodCallPrefix)
 
-    type t =
-      {is_build_method_called: IsBuildMethodCalled.t; is_checked: IsChecked.t; method_calls: S.t}
+    type t = {is_checked: IsChecked.t; method_calls: S.t}
 
-    let pp fmt {is_build_method_called; is_checked; method_calls} =
-      F.fprintf fmt "%a%s" S.pp method_calls
-        ( if is_checked then " checked"
-        else if is_build_method_called then " then build() called"
-        else "" )
+    let pp fmt {is_checked; method_calls} =
+      F.fprintf fmt "%a%s" S.pp method_calls (if is_checked then " checked" else "")
 
 
     let leq ~lhs ~rhs =
-      IsBuildMethodCalled.leq ~lhs:lhs.is_build_method_called ~rhs:rhs.is_build_method_called
-      && IsChecked.leq ~lhs:lhs.is_checked ~rhs:rhs.is_checked
+      IsChecked.leq ~lhs:lhs.is_checked ~rhs:rhs.is_checked
       && S.leq ~lhs:lhs.method_calls ~rhs:rhs.method_calls
 
 
     let join x y =
-      { is_build_method_called=
-          IsBuildMethodCalled.join x.is_build_method_called y.is_build_method_called
-      ; is_checked= IsChecked.join x.is_checked y.is_checked
+      { is_checked= IsChecked.join x.is_checked y.is_checked
       ; method_calls= S.join x.method_calls y.method_calls }
 
 
     let widen ~prev ~next ~num_iters =
-      { is_build_method_called=
-          IsBuildMethodCalled.widen ~prev:prev.is_build_method_called
-            ~next:next.is_build_method_called ~num_iters
-      ; is_checked= IsChecked.widen ~prev:prev.is_checked ~next:next.is_checked ~num_iters
+      { is_checked= IsChecked.widen ~prev:prev.is_checked ~next:next.is_checked ~num_iters
       ; method_calls= S.widen ~prev:prev.method_calls ~next:next.method_calls ~num_iters }
 
 
-    let empty = {is_build_method_called= false; is_checked= false; method_calls= S.empty}
-
-    let singleton e = {is_build_method_called= false; is_checked= false; method_calls= S.singleton e}
+    let empty = {is_checked= false; method_calls= S.empty}
 
-    let add e ({is_build_method_called; method_calls} as x) =
-      if is_build_method_called then x else {x with method_calls= S.add e method_calls}
+    let singleton e = {is_checked= false; method_calls= S.singleton e}
 
+    let add e ({method_calls} as x) = {x with method_calls= S.add e method_calls}
 
     let merge x y =
-      { is_build_method_called= x.is_build_method_called || y.is_build_method_called
-      ; is_checked= x.is_checked || y.is_checked
-      ; method_calls= S.union x.method_calls y.method_calls }
+      {is_checked= x.is_checked || y.is_checked; method_calls= S.union x.method_calls y.method_calls}
 
 
-    let set_build_method_called x = {x with is_build_method_called= true}
-
     let to_string_set method_calls =
       let accum_as_string method_call acc =
         String.Set.add acc (MethodCallPrefix.procname_to_string method_call)
@@ -212,9 +193,9 @@ module NewDomain = struct
       S.elements method_calls
 
 
-    let check_required_props ~check_on_string_set parent_typename
-        ({is_build_method_called; is_checked; method_calls} as x) =
-      if is_build_method_called && not is_checked then (
+    let check_required_props ~check_on_string_set parent_typename ({is_checked; method_calls} as x)
+        =
+      if not is_checked then (
         let prop_set = to_string_set method_calls in
         let call_chain = get_call_chain method_calls in
         check_on_string_set parent_typename call_chain prop_set ;
@@ -223,16 +204,32 @@ module NewDomain = struct
   end
 
   module MethodCalled = struct
-    include AbstractDomain.Map (CreatedLocation) (MethodCalls)
+    module Key = struct
+      type t =
+        { created_location: CreatedLocation.t
+        ; is_build_called: bool  (** if the build method has been called on the builder object *) }
+      [@@deriving compare]
+
+      let pp fmt {created_location; is_build_called} =
+        F.fprintf fmt "%a%s" CreatedLocation.pp created_location
+          (if is_build_called then " with build() called" else "")
 
-    let add_one k v x =
+
+      let no_build_called created_location = {created_location; is_build_called= false}
+
+      let build_called created_location = {created_location; is_build_called= true}
+    end
+
+    include AbstractDomain.Map (Key) (MethodCalls)
+
+    let add_one created_location v x =
       let f = function
         | None ->
             Some (MethodCalls.singleton v)
         | Some method_calls ->
             Some (MethodCalls.add v method_calls)
       in
-      update k f x
+      update (Key.no_build_called created_location) f x
 
 
     let add_all created_locations callee x =
@@ -242,11 +239,18 @@ module NewDomain = struct
 
 
     let build_method_called_one created_location x =
-      let f v =
-        let method_calls = Option.value v ~default:MethodCalls.empty in
-        Some (MethodCalls.set_build_method_called method_calls)
+      let k_no_build_called = Key.no_build_called created_location in
+      let k_build_called = Key.build_called created_location in
+      let method_calls =
+        match (find_opt k_no_build_called x, find_opt k_build_called x) with
+        | None, None ->
+            MethodCalls.empty
+        | Some x, None | None, Some x ->
+            x
+        | Some method_calls_no_build_called, Some method_calls_build_called ->
+            MethodCalls.join method_calls_no_build_called method_calls_build_called
       in
-      update created_location f x
+      remove k_no_build_called x |> add k_build_called method_calls
 
 
     let build_method_called created_locations x =
@@ -254,153 +258,230 @@ module NewDomain = struct
 
 
     let check_required_props ~check_on_string_set x =
-      let f created_location method_calls =
-        match created_location with
-        | CreatedLocation.ByCreateMethod {typ_name} ->
-            MethodCalls.check_required_props ~check_on_string_set typ_name method_calls
-        | CreatedLocation.ByParameter _ ->
-            method_calls
+      let f {Key.created_location; is_build_called} method_calls =
+        if is_build_called then
+          match created_location with
+          | CreatedLocation.ByCreateMethod {typ_name} ->
+              MethodCalls.check_required_props ~check_on_string_set typ_name method_calls
+          | CreatedLocation.ByParameter _ ->
+              method_calls
+        else method_calls
       in
       mapi f x
 
 
     let subst ~is_reachable map ~find_caller_created ~caller ~callee =
-      let accum_substed created_location callee_method_calls acc =
-        let merge_method_calls caller_created acc =
-          let method_calls =
-            Option.value_map (find_opt caller_created caller) ~default:callee_method_calls
-              ~f:(fun caller_method_calls ->
-                MethodCalls.merge caller_method_calls callee_method_calls )
-          in
-          update caller_created
-            (function
-              | None ->
-                  Some method_calls
-              | Some acc_method_calls ->
-                  Some (MethodCalls.merge acc_method_calls method_calls) )
-            acc
+      let merge_method_calls ~callee_method_calls ({Key.created_location} as caller_key) acc =
+        let method_calls =
+          Option.value_map
+            (find_opt (Key.no_build_called created_location) caller)
+            ~default:callee_method_calls
+            ~f:(fun caller_method_calls -> MethodCalls.merge caller_method_calls callee_method_calls)
         in
+        update caller_key
+          (function
+            | None ->
+                Some method_calls
+            | Some acc_method_calls ->
+                Some (MethodCalls.merge acc_method_calls method_calls) )
+          acc
+      in
+      let merge_method_calls_on_substed ~callee_method_calls ~is_build_called caller_created acc =
+        CreatedLocations.fold
+          (fun created_location acc ->
+            merge_method_calls ~callee_method_calls {Key.created_location; is_build_called} acc )
+          caller_created acc
+      in
+      let accum_substed ({Key.created_location; is_build_called} as callee_key) callee_method_calls
+          acc =
         match created_location with
         | CreatedLocation.ByCreateMethod _ ->
-            if is_reachable created_location then merge_method_calls created_location acc else acc
+            if is_reachable created_location then
+              merge_method_calls ~callee_method_calls callee_key acc
+            else acc
         | CreatedLocation.ByParameter path ->
             Option.value_map (SubstPathMap.find_opt path map) ~default:acc ~f:(fun caller_path ->
                 Option.value_map (find_caller_created caller_path) ~default:acc
                   ~f:(fun caller_created ->
-                    CreatedLocations.fold merge_method_calls caller_created acc ) )
+                    merge_method_calls_on_substed ~callee_method_calls ~is_build_called
+                      caller_created acc ) )
       in
       let caller' = fold accum_substed callee empty in
       merge (fun _ v v' -> match v' with Some _ -> v' | None -> v) caller caller'
   end
 
-  type t = {created: Created.t; method_called: MethodCalled.t}
+  module Mem = struct
+    type t = {created: Created.t; method_called: MethodCalled.t}
+
+    let pp fmt {created; method_called} =
+      F.fprintf fmt "@[<v 0>@[Created:@;%a@]@,@[MethodCalled:@;%a@]@]" Created.pp created
+        MethodCalled.pp method_called
+
+
+    let leq ~lhs ~rhs =
+      Created.leq ~lhs:lhs.created ~rhs:rhs.created
+      && MethodCalled.leq ~lhs:lhs.method_called ~rhs:rhs.method_called
+
+
+    let join x y =
+      { created= Created.join x.created y.created
+      ; method_called= MethodCalled.join x.method_called y.method_called }
+
+
+    let widen ~prev ~next ~num_iters =
+      { created= Created.widen ~prev:prev.created ~next:next.created ~num_iters
+      ; method_called=
+          MethodCalled.widen ~prev:prev.method_called ~next:next.method_called ~num_iters }
+
+
+    let empty = {created= Created.empty; method_called= MethodCalled.empty}
+
+    let init tenv pname formals =
+      List.fold formals ~init:empty ~f:(fun ({created; method_called} as acc) (pvar, ptr_typ) ->
+          match ptr_typ with
+          | Typ.{desc= Tptr (typ, _)} -> (
+            match Typ.name typ with
+            | Some typ_name
+              when PatternMatch.is_subtype_of_str tenv typ_name
+                     "com.facebook.litho.Component$Builder" ->
+                let formal_ae = LocalAccessPath.make_from_pvar pvar ptr_typ pname in
+                let created_location = CreatedLocation.ByParameter formal_ae in
+                { created=
+                    Created.add formal_ae (CreatedLocations.singleton created_location) created
+                ; method_called=
+                    MethodCalled.add
+                      (MethodCalled.Key.no_build_called created_location)
+                      MethodCalls.empty method_called }
+            | _ ->
+                acc )
+          | _ ->
+              acc )
+
+
+    let assign ~lhs ~rhs ({created} as x) =
+      {x with created= Created.add lhs (Created.lookup rhs created) created}
+
+
+    let call_create lhs typ_name location ({created} as x) =
+      let created_location = CreatedLocation.ByCreateMethod {location; typ_name} in
+      { created= Created.add lhs (CreatedLocations.singleton created_location) created
+      ; method_called=
+          MethodCalled.add
+            (MethodCalled.Key.no_build_called created_location)
+            MethodCalls.empty x.method_called }
+
+
+    let call_builder ~ret ~receiver callee {created; method_called} =
+      let created_locations = Created.lookup receiver created in
+      { created= Created.add ret created_locations created
+      ; method_called= MethodCalled.add_all created_locations callee method_called }
+
+
+    let call_build_method ~ret ~receiver {created; method_called} =
+      let created_locations = Created.lookup receiver created in
+      { created= Created.add ret created_locations created
+      ; method_called= MethodCalled.build_method_called created_locations method_called }
+
+
+    let check_required_props ~check_on_string_set ({method_called} as x) =
+      {x with method_called= MethodCalled.check_required_props ~check_on_string_set method_called}
+
+
+    let subst ~formals ~actuals ~ret_id_typ:(ret_var, ret_typ) ~caller_pname ~callee_pname ~caller
+        ~callee =
+      let callee_return =
+        LocalAccessPath.make_from_pvar (Pvar.get_ret_pvar callee_pname) ret_typ callee_pname
+      in
+      let caller_return = LocalAccessPath.make (AccessPath.of_var ret_var ret_typ) caller_pname in
+      let formals =
+        List.map formals ~f:(fun (pvar, typ) -> LocalAccessPath.make_from_pvar pvar typ callee_pname)
+      in
+      let actuals =
+        List.map actuals ~f:(function
+          | HilExp.AccessExpression actual ->
+              Some (LocalAccessPath.make_from_access_expression actual caller_pname)
+          | _ ->
+              None )
+      in
+      let map = SubstPathMap.make ~formals ~actuals ~caller_return ~callee_return in
+      let created =
+        Created.subst map ~caller_return ~callee_return ~caller:caller.created
+          ~callee:callee.created
+      in
+      let is_reachable =
+        let reachable_paths =
+          LocalAccessPathSet.of_list formals |> LocalAccessPathSet.add callee_return
+        in
+        let reachable_locations =
+          let accum_reachable_location path locations acc =
+            if LocalAccessPathSet.mem path reachable_paths then CreatedLocations.union acc locations
+            else acc
+          in
+          Created.fold accum_reachable_location callee.created CreatedLocations.empty
+        in
+        fun created_location -> CreatedLocations.mem created_location reachable_locations
+      in
+      let method_called =
+        let find_caller_created path = Created.find_opt path caller.created in
+        MethodCalled.subst ~is_reachable map ~find_caller_created ~caller:caller.method_called
+          ~callee:callee.method_called
+      in
+      {created; method_called}
+  end
+
+  type t = {no_return_called: Mem.t; return_called: Mem.t}
+
+  let pp fmt {no_return_called; return_called} =
+    F.fprintf fmt "@[<v 0>@[NoReturnCalled:@;%a@]@,@[ReturnCalled:@;%a@]@]" Mem.pp no_return_called
+      Mem.pp return_called
 
-  let pp fmt {created; method_called} =
-    F.fprintf fmt "@[<v 0>@[Created:@;%a@]@,@[MethodCalled:@;%a@]@]" Created.pp created
-      MethodCalled.pp method_called
 
+  let get_summary ~is_void_func x = if is_void_func then x.no_return_called else x.return_called
 
   let leq ~lhs ~rhs =
-    Created.leq ~lhs:lhs.created ~rhs:rhs.created
-    && MethodCalled.leq ~lhs:lhs.method_called ~rhs:rhs.method_called
+    Mem.leq ~lhs:lhs.no_return_called ~rhs:rhs.no_return_called
+    && Mem.leq ~lhs:lhs.return_called ~rhs:rhs.return_called
 
 
   let join x y =
-    { created= Created.join x.created y.created
-    ; method_called= MethodCalled.join x.method_called y.method_called }
+    { no_return_called= Mem.join x.no_return_called y.no_return_called
+    ; return_called= Mem.join x.return_called y.return_called }
 
 
   let widen ~prev ~next ~num_iters =
-    { created= Created.widen ~prev:prev.created ~next:next.created ~num_iters
-    ; method_called= MethodCalled.widen ~prev:prev.method_called ~next:next.method_called ~num_iters
-    }
+    { no_return_called= Mem.widen ~prev:prev.no_return_called ~next:next.no_return_called ~num_iters
+    ; return_called= Mem.widen ~prev:prev.return_called ~next:next.return_called ~num_iters }
 
 
-  let empty = {created= Created.empty; method_called= MethodCalled.empty}
+  let empty = {no_return_called= Mem.empty; return_called= Mem.empty}
 
   let init tenv pname formals =
-    List.fold formals ~init:empty ~f:(fun ({created; method_called} as acc) (pvar, ptr_typ) ->
-        match ptr_typ with
-        | Typ.{desc= Tptr (typ, _)} -> (
-          match Typ.name typ with
-          | Some typ_name
-            when PatternMatch.is_subtype_of_str tenv typ_name "com.facebook.litho.Component$Builder"
-            ->
-              let formal_ae = LocalAccessPath.make_from_pvar pvar ptr_typ pname in
-              let created_location = CreatedLocation.ByParameter formal_ae in
-              { created= Created.add formal_ae (CreatedLocations.singleton created_location) created
-              ; method_called= MethodCalled.add created_location MethodCalls.empty method_called }
-          | _ ->
-              acc )
-        | _ ->
-            acc )
+    {no_return_called= Mem.init tenv pname formals; return_called= Mem.empty}
 
 
-  let assign ~lhs ~rhs ({created} as x) =
-    {x with created= Created.add lhs (Created.lookup rhs created) created}
+  let map_no_return_called f x = {x with no_return_called= f x.no_return_called}
 
+  let assign ~lhs ~rhs = map_no_return_called (Mem.assign ~lhs ~rhs)
 
-  let call_create lhs typ_name location ({created} as x) =
-    let created_location = CreatedLocation.ByCreateMethod {location; typ_name} in
-    { created= Created.add lhs (CreatedLocations.singleton created_location) created
-    ; method_called= MethodCalled.add created_location MethodCalls.empty x.method_called }
+  let call_create lhs typ_name location =
+    map_no_return_called (Mem.call_create lhs typ_name location)
 
 
-  let call_builder ~ret ~receiver callee {created; method_called} =
-    let created_locations = Created.lookup receiver created in
-    { created= Created.add ret created_locations created
-    ; method_called= MethodCalled.add_all created_locations callee method_called }
+  let call_builder ~ret ~receiver callee =
+    map_no_return_called (Mem.call_builder ~ret ~receiver callee)
 
 
-  let call_build_method ~ret ~receiver {created; method_called} =
-    let created_locations = Created.lookup receiver created in
-    { created= Created.add ret created_locations created
-    ; method_called= MethodCalled.build_method_called created_locations method_called }
+  let call_build_method ~ret ~receiver = map_no_return_called (Mem.call_build_method ~ret ~receiver)
 
+  let call_return {no_return_called; return_called} =
+    {no_return_called= Mem.empty; return_called= Mem.join no_return_called return_called}
 
-  let check_required_props ~check_on_string_set ({method_called} as x) =
-    {x with method_called= MethodCalled.check_required_props ~check_on_string_set method_called}
 
-
-  let subst ~formals ~actuals ~ret_id_typ:(ret_var, ret_typ) ~caller_pname ~callee_pname ~caller
-      ~callee =
-    let callee_return =
-      LocalAccessPath.make_from_pvar (Pvar.get_ret_pvar callee_pname) ret_typ callee_pname
-    in
-    let caller_return = LocalAccessPath.make (AccessPath.of_var ret_var ret_typ) caller_pname in
-    let formals =
-      List.map formals ~f:(fun (pvar, typ) -> LocalAccessPath.make_from_pvar pvar typ callee_pname)
-    in
-    let actuals =
-      List.map actuals ~f:(function
-        | HilExp.AccessExpression actual ->
-            Some (LocalAccessPath.make_from_access_expression actual caller_pname)
-        | _ ->
-            None )
-    in
-    let map = SubstPathMap.make ~formals ~actuals ~caller_return ~callee_return in
-    let created =
-      Created.subst map ~caller_return ~callee_return ~caller:caller.created ~callee:callee.created
-    in
-    let is_reachable =
-      let reachable_paths =
-        LocalAccessPathSet.of_list formals |> LocalAccessPathSet.add callee_return
-      in
-      let reachable_locations =
-        let accum_reachable_location path locations acc =
-          if LocalAccessPathSet.mem path reachable_paths then CreatedLocations.union acc locations
-          else acc
-        in
-        Created.fold accum_reachable_location callee.created CreatedLocations.empty
-      in
-      fun created_location -> CreatedLocations.mem created_location reachable_locations
-    in
-    let method_called =
-      let find_caller_created path = Created.find_opt path caller.created in
-      MethodCalled.subst ~is_reachable map ~find_caller_created ~caller:caller.method_called
-        ~callee:callee.method_called
-    in
-    {created; method_called}
+  let subst ~formals ~actuals ~ret_id_typ ~caller_pname ~callee_pname ~caller ~callee =
+    { caller with
+      no_return_called=
+        Mem.subst ~formals ~actuals ~ret_id_typ ~caller_pname ~callee_pname
+          ~caller:caller.no_return_called ~callee }
 end
 
 include struct
@@ -428,8 +509,6 @@ include struct
 
   let iter f = lift_old (OldDomain.iter f)
 
-  let fold f (o, _) init = OldDomain.fold f o init
-
   let assign ~lhs ~rhs = map_new (NewDomain.assign ~lhs ~rhs)
 
   let call_create ret typ_name location = map_new (NewDomain.call_create ret typ_name location)
@@ -438,35 +517,41 @@ include struct
 
   let call_build_method ~ret ~receiver = map_new (NewDomain.call_build_method ~ret ~receiver)
 
+  let call_return = map_new NewDomain.call_return
+
+  type summary = OldDomain.t * NewDomain.Mem.t
+
+  let pp_summary fmt (o, n) =
+    F.fprintf fmt "@[<v 2>@[Old:@;%a@]@,@[New:@;%a@]@]" OldDomain.pp o NewDomain.Mem.pp n
+
+
+  let get_summary ~is_void_func = map_new (NewDomain.get_summary ~is_void_func)
+
   let check_required_props ~check_on_string_set =
-    map_new (NewDomain.check_required_props ~check_on_string_set)
+    map_new (NewDomain.Mem.check_required_props ~check_on_string_set)
 end
 
-let substitute ~(f_sub : LocalAccessPath.t -> LocalAccessPath.t option) ((_, new_astate) as astate)
-    =
-  let old_astate, _ =
-    fold
-      (fun original_access_path call_set acc ->
-        let access_path' =
-          match f_sub original_access_path with
-          | Some access_path ->
-              access_path
-          | None ->
-              original_access_path
-        in
-        let call_set' =
-          CallSet.fold
-            (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; location} call_set_acc )
-            call_set CallSet.empty
-        in
-        add access_path' call_set' acc )
-      astate empty
-  in
-  (old_astate, new_astate)
+let substitute ~(f_sub : LocalAccessPath.t -> LocalAccessPath.t option) old_astate =
+  OldDomain.fold
+    (fun original_access_path call_set acc ->
+      let access_path' =
+        match f_sub original_access_path with
+        | Some access_path ->
+            access_path
+        | None ->
+            original_access_path
+      in
+      let call_set' =
+        CallSet.fold
+          (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; location} call_set_acc )
+          call_set CallSet.empty
+      in
+      OldDomain.add access_path' call_set' acc )
+    old_astate OldDomain.empty
 
 
 (** Unroll the domain to enumerate all the call chains ending in [call] and apply [f] to each

infer/src/checkers/LithoDomain.mli

@@ -54,6 +54,10 @@ module CallSet : module type of AbstractDomain.FiniteSet (MethodCall)
 module OldDomain : module type of AbstractDomain.Map (LocalAccessPath) (CallSet)
 
 module NewDomain : sig
+  module Mem : sig
+    type t
+  end
+
   include AbstractDomain.S
 
   val subst :
@@ -63,12 +67,15 @@ module NewDomain : sig
     -> caller_pname:Typ.Procname.t
     -> callee_pname:Typ.Procname.t
     -> caller:t
-    -> callee:t
+    -> callee:Mem.t
     -> t
 end
 
 include module type of AbstractDomain.Pair (OldDomain) (NewDomain)
 
+(** type for saving in summary payload *)
+type summary = OldDomain.t * NewDomain.Mem.t
+
 val empty : t
 
 val init : Tenv.t -> Typ.Procname.t -> (Pvar.t * Typ.t) list -> t
@@ -81,7 +88,7 @@ val mem : LocalAccessPath.t -> t -> bool
 
 val find : LocalAccessPath.t -> t -> CallSet.t
 
-val bindings : t -> (LocalAccessPath.t * CallSet.t) list
+val bindings : summary -> (LocalAccessPath.t * CallSet.t) list
 
 val assign : lhs:LocalAccessPath.t -> rhs:LocalAccessPath.t -> t -> t
 
@@ -95,12 +102,21 @@ val call_builder :
 val call_build_method : ret:LocalAccessPath.t -> receiver:LocalAccessPath.t -> t -> t
 (** Semantics of builder's final build method *)
 
+val call_return : t -> t
+(** Semantics of return method *)
+
+val pp_summary : Format.formatter -> summary -> unit
+
+val get_summary : is_void_func:bool -> t -> summary
+
 val check_required_props :
-  check_on_string_set:(Typ.name -> MethodCallPrefix.t list -> String.Set.t -> unit) -> t -> t
+     check_on_string_set:(Typ.name -> MethodCallPrefix.t list -> String.Set.t -> unit)
+  -> summary
+  -> summary
 
-val substitute : f_sub:(LocalAccessPath.t -> LocalAccessPath.t option) -> t -> t
+val substitute : f_sub:(LocalAccessPath.t -> LocalAccessPath.t option) -> OldDomain.t -> OldDomain.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 -> MethodCall.t list -> unit) -> t -> unit
+val iter_call_chains : f:(AccessPath.t -> MethodCall.t list -> unit) -> summary -> unit
 (** Apply [f] to each maximal call chain encoded in [t] *)

infer/src/checkers/LithoFramework.ml

@@ -71,35 +71,45 @@ let get_component_create_typ_opt procname tenv =
 module type LithoContext = sig
   type t
 
-  val field : (Payloads.t, t option) Field.t
+  type summary
 
-  val check_callee : callee_pname:Typ.Procname.t -> tenv:Tenv.t -> t option -> bool
+  val field : (Payloads.t, summary option) Field.t
+
+  val check_callee : callee_pname:Typ.Procname.t -> tenv:Tenv.t -> summary option -> bool
 
   val satisfies_heuristic :
-    callee_pname:Typ.Procname.t -> callee_summary_opt:t option -> Tenv.t -> bool
+    callee_pname:Typ.Procname.t -> callee_summary_opt:summary option -> Tenv.t -> bool
 
   val should_report : Procdesc.t -> Tenv.t -> bool
 
-  val report : t -> Tenv.t -> Summary.t -> t
+  val report : summary -> Tenv.t -> Summary.t -> summary
 
   val session_name : string
 end
 
-type get_proc_summary_and_formals = Typ.Procname.t -> (Domain.t * (Pvar.t * Typ.t) list) option
+type get_proc_summary_and_formals =
+  Typ.Procname.t -> (Domain.summary * (Pvar.t * Typ.t) list) option
 
 type extras = {get_proc_summary_and_formals: get_proc_summary_and_formals}
 
-module TransferFunctions (CFG : ProcCfg.S) (LithoContext : LithoContext with type t = Domain.t) =
+module TransferFunctions
+    (CFG : ProcCfg.S)
+    (LithoContext : LithoContext with type summary = Domain.summary) =
 struct
   module CFG = CFG
   module Domain = Domain
-  module Payload = SummaryPayload.Make (LithoContext)
+
+  module Payload = SummaryPayload.Make (struct
+    type t = LithoContext.summary
+
+    let field = LithoContext.field
+  end)
 
   type nonrec extras = extras
 
   let apply_callee_summary summary_opt ~caller_pname ~callee_pname ret_id_typ formals actuals
-      ((_, new_domain) as astate) =
-    Option.value_map summary_opt ~default:astate ~f:(fun summary ->
+      ((old_domain, new_domain) as astate) =
+    Option.value_map summary_opt ~default:astate ~f:(fun (old_callee, new_callee) ->
         (* TODO: append paths if the footprint access path is an actual path instead of a var *)
         let f_sub {Domain.LocalAccessPath.access_path= (var, _), _} =
           match Var.get_footprint_index var with
@@ -117,10 +127,10 @@ struct
                 Some (Domain.LocalAccessPath.make (ret_id_typ, []) caller_pname)
               else None
         in
-        let astate_old, _ = Domain.substitute ~f_sub summary |> Domain.join astate in
+        let astate_old = Domain.substitute ~f_sub old_callee |> Domain.OldDomain.join old_domain in
         let astate_new =
           Domain.NewDomain.subst ~formals ~actuals ~ret_id_typ ~caller_pname ~callee_pname
-            ~caller:new_domain ~callee:(snd summary)
+            ~caller:new_domain ~callee:new_callee
         in
         (astate_old, astate_new) )
 
@@ -175,23 +185,34 @@ struct
         Option.value_map callee_summary_and_formals_opt ~default:astate ~f:(fun (_, formals) ->
             apply_callee_summary callee_summary_opt ~caller_pname ~callee_pname ret_id_typ formals
               actuals astate )
-    | Assign (lhs_ae, HilExp.AccessExpression rhs_ae, _) ->
-        (* creating an alias for the rhs binding; assume all reads will now occur through the
-           alias. this helps us keep track of chains in cases like tmp = getFoo(); x = tmp;
-           tmp.getBar() *)
-        let lhs_access_path =
-          Domain.LocalAccessPath.make (HilExp.AccessExpression.to_access_path lhs_ae) caller_pname
-        in
-        let rhs_access_path =
-          Domain.LocalAccessPath.make (HilExp.AccessExpression.to_access_path rhs_ae) caller_pname
-        in
+    | Assign (lhs_ae, rhs, _) ->
         let astate =
-          try
-            let call_set = Domain.find rhs_access_path astate in
-            Domain.remove rhs_access_path astate |> Domain.add lhs_access_path call_set
-          with Caml.Not_found -> astate
+          match rhs with
+          | HilExp.AccessExpression rhs_ae ->
+              (* creating an alias for the rhs binding; assume all reads will now occur through the
+                 alias. this helps us keep track of chains in cases like tmp = getFoo(); x = tmp;
+                 tmp.getBar() *)
+              let lhs_access_path =
+                Domain.LocalAccessPath.make
+                  (HilExp.AccessExpression.to_access_path lhs_ae)
+                  caller_pname
+              in
+              let rhs_access_path =
+                Domain.LocalAccessPath.make
+                  (HilExp.AccessExpression.to_access_path rhs_ae)
+                  caller_pname
+              in
+              let astate =
+                try
+                  let call_set = Domain.find rhs_access_path astate in
+                  Domain.remove rhs_access_path astate |> Domain.add lhs_access_path call_set
+                with Caml.Not_found -> astate
+              in
+              Domain.assign ~lhs:lhs_access_path ~rhs:rhs_access_path astate
+          | _ ->
+              astate
         in
-        Domain.assign ~lhs:lhs_access_path ~rhs:rhs_access_path astate
+        if HilExp.AccessExpression.is_return_var lhs_ae then Domain.call_return astate else astate
     | _ ->
         astate
 
@@ -199,7 +220,7 @@ struct
   let pp_session_name _node fmt = F.pp_print_string fmt LithoContext.session_name
 end
 
-module MakeAnalyzer (LithoContext : LithoContext with type t = Domain.t) = struct
+module MakeAnalyzer (LithoContext : LithoContext with type summary = Domain.summary) = struct
   module TF = TransferFunctions (ProcCfg.Normal) (LithoContext)
   module A = LowerHil.MakeAbstractInterpreter (TF)
 
@@ -222,13 +243,15 @@ module MakeAnalyzer (LithoContext : LithoContext with type t = Domain.t) = struc
     let initial = Domain.init tenv proc_name (Procdesc.get_pvar_formals proc_desc) in
     match A.compute_post proc_data ~initial with
     | Some post ->
+        let is_void_func = Procdesc.get_ret_type proc_desc |> Typ.is_void in
+        let post = Domain.get_summary ~is_void_func post in
         let post =
           if LithoContext.should_report proc_desc tenv then LithoContext.report post tenv summary
           else post
         in
-        let postprocess astate formal_map : Domain.t =
+        let postprocess (old_astate, new_astate) formal_map : Domain.summary =
           let f_sub access_path = Domain.LocalAccessPath.to_formal_option access_path formal_map in
-          Domain.substitute ~f_sub astate
+          (Domain.substitute ~f_sub old_astate, new_astate)
         in
         let payload = postprocess post (FormalMap.make proc_desc) in
         TF.Payload.update_summary payload summary

infer/src/checkers/RequiredProps.ml

@@ -114,6 +114,8 @@ let has_prop prop_set prop =
 module LithoContext = struct
   type t = Domain.t
 
+  type summary = Domain.summary
+
   let check_callee ~callee_pname ~tenv _ =
     LithoFramework.is_component_builder callee_pname tenv
     || LithoFramework.is_component_create_method callee_pname tenv

infer/tests/codetoanalyze/java/litho-required-props/RequiredProps.java

@@ -411,4 +411,14 @@ public class RequiredProps {
             : mMyLithoComponent.create().prop1(new Object()).prop2(new Object());
     return builder.build();
   }
+
+  public Component buildJoinOk(boolean b, String s) {
+    MyComponent.Builder builder = mMyComponent.create().prop1(new Object());
+    if (b) {
+      builder.prop3(new Object());
+    } else {
+      return null;
+    }
+    return builder.build();
+  }
 }

infer/tests/codetoanalyze/java/litho-required-props/issues.exp

@@ -15,8 +15,7 @@ codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.l
 codetoanalyze/java/litho-required-props/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)]
 codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castImpossibleOk_FP(java.lang.Object):void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component com.facebook.litho.MyLithoComponent, but is not set before the call to build()]
 codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castImpossibleOk_FP(java.lang.Object):void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop2 is required for component com.facebook.litho.MyLithoComponent, but is not set before the call to build()]
-codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castMissingOneBad(java.lang.Object):void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop2 is required for component com.facebook.litho.MyLithoComponent, but is not set before the call to build()]
-codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castMissingOneBad(java.lang.Object):void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component com.facebook.litho.MyLithoComponent, but is not set before the call to build()]
+codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castMissingOneBad(java.lang.Object):void, 0, MISSING_REQUIRED_PROP, no_bucket, ERROR, [@Prop prop1 is required for component com.facebook.litho.MyLithoComponent, but is not set before the call to build(),calls MyLithoComponent$Builder MyLithoComponent$Builder.prop2(Object)]
 codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.doubleSetMissingBad():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 Component$Builder Component$Builder.commonProp(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object)]
 codetoanalyze/java/litho-required-props/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)]
 codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp1InBothBranchesBeforeBuildBad(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.prop3(Object)]