[required-props] Add support for checking varArgs

Summary: `Prop(varArg = myProp) List <?> myPropList` can also be set via `myPropList()` or `myProp()`. Add support for picking up the `varArg` and checking this form of required props.

Reviewed By: ngorogiannis

Differential Revision: D17571997

fbshipit-source-id: 7956cb972

infer/src/checkers/Litho.ml

@@ -91,6 +91,10 @@ module GraphQLGetters = struct
 end
 
 module RequiredProps = struct
+  (* VarProp is only for props that have a varArg parameter like
+     @Prop(varArg = "var_prop") whereas Prop is for everything except. *)
+  type required_prop = Prop of string | VarProp of {prop: string; var_prop: string}
+
   let get_required_props typename tenv =
     let is_required annot_list =
       List.exists
@@ -106,19 +110,47 @@ module RequiredProps = struct
                   parameters) )
         annot_list
     in
+    let get_var_args annot_list =
+      List.fold ~init:None
+        ~f:(fun acc (({Annot.parameters} as annot), _) ->
+          if Annotations.annot_ends_with annot Annotations.prop then
+            (* Pick up the parameter for varArg if it has the form
+               @Prop(varArg = myProp). *)
+            List.fold ~init:acc
+              ~f:(fun acc Annot.{name; value} ->
+                if Option.value_map name ~default:false ~f:(fun name -> String.equal "varArg" name)
+                then Some value
+                else acc )
+              parameters
+          else acc )
+        annot_list
+    in
     match Tenv.lookup tenv typename with
     | Some {fields} ->
         List.filter_map
           ~f:(fun (fieldname, _, annot) ->
-            if is_required annot then Some (Typ.Fieldname.Java.get_field fieldname) else None )
+            if is_required annot then
+              let prop = Typ.Fieldname.Java.get_field fieldname in
+              let var_prop_opt = get_var_args annot in
+              Some
+                (Option.value_map var_prop_opt ~default:(Prop prop) ~f:(fun var_prop ->
+                     VarProp {var_prop; prop} ))
+            else None )
           fields
     | None ->
         []
 
 
-  let report_missing_required_prop summary prop_string parent_typename loc =
+  let report_missing_required_prop summary prop parent_typename loc =
     let message =
-      F.asprintf "@Prop %s is required for component %s, but is not set before the call to build()"
+      let prop_string =
+        match prop with
+        | Prop prop ->
+            F.asprintf "@Prop %s" prop
+        | VarProp {var_prop; prop} ->
+            F.asprintf "Either @Prop %s or @Prop(varArg = %s)" prop var_prop
+      in
+      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
@@ -148,13 +180,22 @@ module RequiredProps = struct
   let suffixes = String.Set.of_list ["Attr"; "Dip"; "Px"; "Res"; "Sp"]
 
   let has_prop prop_set prop =
+    let check prop =
       String.Set.mem prop_set prop
-    (* @Prop(resType = ...) myProp can also be set via myProp(), myPropAttr(), myPropDip(), myPropPx(), myPropRes() or myPropSp().
+      || (* @Prop(resType = ...) myProp can also be set via myProp(), myPropAttr(), myPropDip(), myPropPx(), myPropRes() or myPropSp().
            Our annotation parameter parsing is too primitive to identify resType, so just assume
            that all @Prop's can be set any of these 6 ways. *)
-    || String.Set.exists prop_set ~f:(fun el ->
+         String.Set.exists prop_set ~f:(fun el ->
              String.chop_prefix el ~prefix:prop
              |> Option.exists ~f:(fun suffix -> String.Set.mem suffixes suffix) )
+    in
+    match prop with
+    | Prop prop ->
+        check prop
+    | VarProp {var_prop; prop} ->
+        (* @Prop(varArg = myProp) List <?> myPropList can also be set
+         via myPropList() or myProp().*)
+        check var_prop || check prop
 
 
   let report astate tenv summary =

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

@@ -13,6 +13,8 @@ import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.ArrayList;
+import java.util.List;
 
 enum ResType {
   SOME,
@@ -25,6 +27,8 @@ enum ResType {
   ResType resType() default ResType.NONE;
 
   boolean optional() default false;
+
+  String varArg() default "";
 }
 
 @Target({ElementType.PARAMETER, ElementType.FIELD})
@@ -46,6 +50,7 @@ class MyComponent extends Component {
 
   Object nonProp;
 
+
   public Builder create() {
     return new Builder();
   }
@@ -150,10 +155,77 @@ class ResPropComponent extends Component {
   }
 }
 
+/** varArg test */
+class VarArgPropComponent extends Component {
+
+  @Prop(varArg = "prop")
+  List<Object> props;
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  static class Builder extends Component.Builder {
+
+    VarArgPropComponent mVarArgPropComponent;
+
+    public Builder prop(Object prop) {
+      if (prop == null) {
+        return this;
+      }
+      if (this.mVarArgPropComponent.props == null) {
+        this.mVarArgPropComponent.props = new ArrayList<Object>();
+      }
+      this.mVarArgPropComponent.props.add(prop);
+      return this;
+    }
+
+    public Builder propAttr(Object prop) {
+      if (prop == null) {
+        return this;
+      }
+      if (this.mVarArgPropComponent.props == null) {
+        this.mVarArgPropComponent.props = new ArrayList<Object>();
+      }
+      this.mVarArgPropComponent.props.add(prop);
+      return this;
+    }
+
+    public Builder propsAttr(List<Object> props) {
+      if (props == null) {
+        return this;
+      }
+      if (this.mVarArgPropComponent.props == null || this.mVarArgPropComponent.props.isEmpty()) {
+        this.mVarArgPropComponent.props = props;
+      } else {
+        this.mVarArgPropComponent.props.addAll(props);
+      }
+      return this;
+    }
+
+    public Builder props(List<Object> props) {
+      if (props == null) {
+        return this;
+      }
+      if (this.mVarArgPropComponent.props == null || this.mVarArgPropComponent.props.isEmpty()) {
+        this.mVarArgPropComponent.props = props;
+      } else {
+        this.mVarArgPropComponent.props.addAll(props);
+      }
+      return this;
+    }
+
+    public VarArgPropComponent build() {
+      return mVarArgPropComponent;
+    }
+  }
+}
+
 public class RequiredProps {
 
   public MyComponent mMyComponent;
   public ResPropComponent mResPropComponent;
+  public VarArgPropComponent mVarArgPropComponent;
 
   public MyComponent buildWithAllOk() {
     return mMyComponent
@@ -275,6 +347,26 @@ public class RequiredProps {
     mResPropComponent.create().build();
   }
 
+  public void buildPropVarArgNormalOk() {
+    mVarArgPropComponent.create().props(new ArrayList<Object>()).build();
+  }
+
+  public void buildPropVarArgElementOk() {
+    mVarArgPropComponent.create().prop(new Object()).build();
+  }
+
+  public void buildPropVarArgAttrElementOk() {
+    mVarArgPropComponent.create().propAttr(new Object()).build();
+  }
+
+  public void buildPropVarArgNormalAttrElementOk() {
+    mVarArgPropComponent.create().propsAttr(new ArrayList<Object>()).build();
+  }
+
+  public void buildPropVarArgMissingBad() {
+    mVarArgPropComponent.create().build();
+  }
+
   public class NonRequiredTreeProps {
 
     public MyTreeComponent mMyTreeComponent;

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

@@ -1,5 +1,6 @@
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.FP_setRequiredOnBothBranchesOk(boolean):codetoanalyze.java.litho.MyComponent, 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():codetoanalyze.java.litho.MyComponent, 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():codetoanalyze.java.litho.MyComponent, 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()]