[litho] Add prop check in the new domain

Summary: This diff checks litho condition on the new abstract value. This is triggered with `--new-litho-domain`, but it is intra-procedural as of now.

Reviewed By: ezgicicek

Differential Revision: D18783203

fbshipit-source-id: 98570104e

infer/man/man1/infer-full.txt

@@ -1539,6 +1539,10 @@ INTERNAL OPTIONS
        --never-returning-null json
            Matcher or list of matchers for functions that never return null.
 
+       --new-litho-domain
+           Activates: [EXPERIMENTAL] Use new litho domain (Conversely:
+           --no-new-litho-domain)
+
        --nullable-annotation-name string
            Specify custom nullable annotation name
 

infer/src/base/Config.ml

@@ -1740,6 +1740,8 @@ and monitor_prop_size =
 
 and nelseg = CLOpt.mk_bool ~deprecated:["nelseg"] ~long:"nelseg" "Use only nonempty lsegs"
 
+and new_litho_domain = CLOpt.mk_bool ~long:"new-litho-domain" "[EXPERIMENTAL] Use new litho domain"
+
 and nullable_annotation =
   CLOpt.mk_string_opt ~long:"nullable-annotation-name" "Specify custom nullable annotation name"
 
@@ -3019,6 +3021,8 @@ and monitor_prop_size = !monitor_prop_size
 
 and nelseg = !nelseg
 
+and new_litho_domain = !new_litho_domain
+
 and nullable_annotation = !nullable_annotation
 
 and nullsafe_optimistic_third_party_params_in_non_strict =

infer/src/base/Config.mli

@@ -492,6 +492,8 @@ val monitor_prop_size : bool
 
 val nelseg : bool
 
+val new_litho_domain : bool
+
 val no_translate_libs : bool
 
 val nullable_annotation : string option

infer/src/checkers/LithoDomain.ml

@@ -15,6 +15,10 @@ module LocalAccessPath = struct
 
   let make access_path parent = {access_path; parent}
 
+  let make_from_access_expression ae parent =
+    make (HilExp.AccessExpression.to_access_path ae) parent
+
+
   let to_formal_option {access_path= ((_, base_typ) as base), accesses; parent} formal_map =
     match FormalMap.get_formal_index base formal_map with
     | Some formal_index ->
@@ -34,6 +38,9 @@ module MethodCall = struct
 
   let pp fmt {receiver; procname} =
     F.fprintf fmt "%a.%a" LocalAccessPath.pp receiver Typ.Procname.pp procname
+
+
+  let procname_to_string {procname} = Typ.Procname.get_method procname
 end
 
 module CallSet = AbstractDomain.FiniteSet (MethodCall)
@@ -65,6 +72,7 @@ module NewDomain = struct
         if r <> 0 then r else Typ.Procname.compare x.procname y.procname
     end)
 
+    (* TODO: add return type && and do not add return method to the set *)
     type t = {is_build_method_called: IsBuildMethodCalled.t; method_calls: S.t}
 
     let pp fmt {is_build_method_called; method_calls} =
@@ -98,7 +106,43 @@ module NewDomain = struct
       if is_build_method_called then x else {x with method_calls= S.add e method_calls}
 
 
-    let set_build_method_called x = {x with is_build_method_called= true}
+    (* TODO do not add callee to the set *)
+    let set_build_method_called callee x =
+      let x = add callee x in
+      {x with is_build_method_called= true}
+
+
+    let find_client_component_type method_calls =
+      let exception Found of Typ.name in
+      let f 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)
+            |> Option.iter ~f:(fun typ -> raise (Found typ))
+        | _ ->
+            ()
+      in
+      match S.iter f method_calls with () -> None | exception Found typ -> Some typ
+
+
+    let to_string_set method_calls =
+      let accum_as_string method_call acc =
+        String.Set.add acc (MethodCall.procname_to_string method_call)
+      in
+      S.fold accum_as_string method_calls String.Set.empty
+
+
+    let get_call_chain method_calls =
+      (* TODO: sort chain by inserted order *)
+      S.elements method_calls
+
+
+    let check_required_props ~check_on_string_set {is_build_method_called; method_calls} =
+      if is_build_method_called then
+        Option.iter (find_client_component_type method_calls) ~f:(fun parent_typename ->
+            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 )
   end
 
   module MethodCalled = struct
@@ -120,16 +164,21 @@ module NewDomain = struct
         created_locations x
 
 
-    let build_method_called_one created_location x =
+    let build_method_called_one callee created_location x =
       let f v =
         let method_calls = Option.value v ~default:MethodCalls.empty in
-        Some (MethodCalls.set_build_method_called method_calls)
+        Some (MethodCalls.set_build_method_called callee method_calls)
       in
       update created_location f x
 
 
-    let build_method_called created_locations x =
-      CreatedLocations.fold build_method_called_one created_locations x
+    let build_method_called created_locations callee x =
+      CreatedLocations.fold (build_method_called_one callee) created_locations x
+
+
+    let check_required_props ~check_on_string_set x =
+      let f _ method_calls = MethodCalls.check_required_props ~check_on_string_set method_calls in
+      iter f x
   end
 
   type t = {created: Created.t; method_called: MethodCalled.t}
@@ -171,10 +220,14 @@ module NewDomain = struct
     ; method_called= MethodCalled.add_all created_locations callee method_called }
 
 
-  let call_build_method ~ret ~receiver {created; method_called} =
+  let call_build_method ~ret ~receiver callee {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 }
+    ; method_called= MethodCalled.build_method_called created_locations callee method_called }
+
+
+  let check_required_props ~check_on_string_set {method_called} =
+    MethodCalled.check_required_props ~check_on_string_set method_called
 end
 
 include struct
@@ -182,6 +235,8 @@ include struct
 
   let lift_old f (o, _) = f o
 
+  let lift_new f (_, n) = f n
+
   let map_old f (o, n) = (f o, n)
 
   let map_new f (o, n) = (o, f n)
@@ -208,30 +263,39 @@ include struct
 
   let call_builder ~ret ~receiver callee = map_new (NewDomain.call_builder ~ret ~receiver callee)
 
-  let call_build_method ~ret ~receiver = map_new (NewDomain.call_build_method ~ret ~receiver)
+  let call_build_method ~ret ~receiver callee =
+    map_new (NewDomain.call_build_method ~ret ~receiver callee)
+
+
+  let check_required_props ~check_on_string_set =
+    lift_new (NewDomain.check_required_props ~check_on_string_set)
 end
 
-let substitute ~(f_sub : LocalAccessPath.t -> LocalAccessPath.t option) 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
+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)
 
 
 (** Unroll the domain to enumerate all the call chains ending in [call] and apply [f] to each

infer/src/checkers/LithoDomain.mli

@@ -14,6 +14,8 @@ module LocalAccessPath : sig
 
   val make : AccessPath.t -> Typ.Procname.t -> t
 
+  val make_from_access_expression : HilExp.AccessExpression.t -> Typ.Procname.t -> t
+
   val to_formal_option : t -> FormalMap.t -> t option
 
   val pp : F.formatter -> t -> unit
@@ -27,6 +29,8 @@ module MethodCall : sig
   val make : LocalAccessPath.t -> Typ.Procname.t -> Location.t -> t
 
   val pp : F.formatter -> t -> unit
+
+  val procname_to_string : t -> string
 end
 
 module CallSet : module type of AbstractDomain.FiniteSet (MethodCall)
@@ -52,10 +56,17 @@ val bindings : t -> (LocalAccessPath.t * CallSet.t) list
 val assign : lhs:LocalAccessPath.t -> rhs:LocalAccessPath.t -> t -> t
 
 val call_create : LocalAccessPath.t -> Location.t -> t -> t
+(** Semantics of builder creation method *)
 
 val call_builder : ret:LocalAccessPath.t -> receiver:LocalAccessPath.t -> MethodCall.t -> t -> t
+(** Semantics of builder's methods, e.g. [prop] *)
+
+val call_build_method :
+  ret:LocalAccessPath.t -> receiver:LocalAccessPath.t -> MethodCall.t -> t -> t
+(** Semantics of builder's final build method *)
 
-val call_build_method : ret:LocalAccessPath.t -> receiver:LocalAccessPath.t -> t -> t
+val check_required_props :
+  check_on_string_set:(Typ.name -> MethodCall.t list -> String.Set.t -> unit) -> t -> unit
 
 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

infer/src/checkers/LithoFramework.ml

@@ -41,6 +41,14 @@ let is_component procname tenv =
       false
 
 
+let is_call_build_inside procname tenv =
+  match Typ.Procname.get_method procname with
+  | "child" ->
+      is_component_builder procname tenv
+  | _ ->
+      false
+
+
 let is_component_build_method procname tenv =
   match Typ.Procname.get_method procname with
   | "build" ->
@@ -121,9 +129,7 @@ struct
         , location ) ->
         let callee_summary_opt = Payload.read ~caller_summary:summary ~callee_pname in
         let receiver =
-          Domain.LocalAccessPath.make
-            (HilExp.AccessExpression.to_access_path receiver_ae)
-            caller_pname
+          Domain.LocalAccessPath.make_from_access_expression receiver_ae caller_pname
         in
         if
           ( LithoContext.check_callee ~callee_pname ~tenv callee_summary_opt
@@ -142,7 +148,14 @@ struct
           if is_component_create_method callee_pname tenv then
             Domain.call_create return_access_path location astate
           else if is_component_build_method callee_pname tenv then
-            Domain.call_build_method ~ret:return_access_path ~receiver astate
+            Domain.call_build_method ~ret:return_access_path ~receiver callee astate
+          else if is_call_build_inside callee_pname tenv then
+            match actuals with
+            | _ :: HilExp.AccessExpression ae :: _ ->
+                let receiver = Domain.LocalAccessPath.make_from_access_expression ae caller_pname in
+                Domain.call_build_method ~ret:return_access_path ~receiver callee astate
+            | _ ->
+                astate
           else if is_component_builder callee_pname tenv then
             Domain.call_builder ~ret:return_access_path ~receiver callee astate
           else astate
@@ -179,7 +192,7 @@ struct
 end
 
 module MakeAnalyzer (LithoContext : LithoContext with type t = Domain.t) = struct
-  module TF = TransferFunctions (ProcCfg.Exceptional) (LithoContext)
+  module TF = TransferFunctions (ProcCfg.Normal) (LithoContext)
   module A = LowerHil.MakeAbstractInterpreter (TF)
 
   let checker {Callbacks.summary; exe_env} =

infer/src/checkers/RequiredProps.ml

@@ -154,6 +154,15 @@ module LithoContext = struct
 
 
   let report astate tenv summary =
+    let check_on_string_set parent_typename call_chain prop_set =
+      let required_props = get_required_props parent_typename tenv 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) call_chain )
+        required_props
+    in
     let check_required_prop_chain _ call_chain =
       let call_chain =
         List.drop_while call_chain ~f:(fun Domain.MethodCall.{procname} ->
@@ -168,25 +177,17 @@ module LithoContext = struct
            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:(fun Domain.MethodCall.{procname} -> Typ.Procname.get_method procname)
-                call_chain
-              |> String.Set.of_list
+              List.map ~f:Domain.MethodCall.procname_to_string 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) call_chain )
-              required_props
+            check_on_string_set parent_typename call_chain prop_set
         | _ ->
             () )
       | _ ->
           ()
     in
-    Domain.iter_call_chains ~f:check_required_prop_chain astate
+    if Config.new_litho_domain then Domain.check_required_props ~check_on_string_set astate
+    else Domain.iter_call_chains ~f:check_required_prop_chain astate
 
 
   let session_name = "litho required props"

infer/tests/codetoanalyze/java/litho/RequiredProps.java

@@ -80,7 +80,7 @@ public class RequiredProps {
     return builder.prop2(new Object()).prop3(new Object()).build();
   }
 
-  public Component setRequiredOnBothBranchesOk_FP(boolean b) {
+  public Component setRequiredOnBothBranchesOk(boolean b) {
     MyComponent.Builder builder = mMyComponent.create();
     if (b) {
       builder = builder.prop1(new Object());
@@ -102,7 +102,7 @@ public class RequiredProps {
   }
 
   // gets confused at cyclic dependency to builder when setting prop1
-  public Component setRequiredOk_FP(boolean b) {
+  public Component setRequiredOk(boolean b) {
     MyComponent.Builder builder = mMyComponent.create();
     builder = builder.prop1(new Object());
     return builder.prop2(new Object()).prop3(new Object()).build();

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

@@ -14,11 +14,9 @@ codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredPr
 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.create(),calls MyComponent$Builder MyComponent$Builder.prop1(Object),calls Component 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.create(),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.setRequiredEffectful_FP(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.create(),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.setRequiredOk_FP(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.create(),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.setRequiredOnBothBranchesMissingProp3Bad(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.create(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()]
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnBothBranchesMissingProp3Bad(boolean):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.create(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()]
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnBothBranchesNoAssignOk_FP(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.create(),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.setRequiredOnBothBranchesOk_FP(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.create(),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.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.create(),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.setRequiredOnOneBranchEffectfulBad(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.create(),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()]