[required-props] Add more tests showing the ineffectiveness of callee heuristic

Summary: As a heuristic, litho library calls on non-litho callers are not tracked. This is very imprecise and results in FPs and FNs as exemplified by newly added tests. Instead, we should check to see if the summary contains a `build()` method as will be done in the next diff. This diff adds these tests and refactors the test code.

Reviewed By: skcho

Differential Revision: D17809536

fbshipit-source-id: 6dff1868c

infer/tests/codetoanalyze/java/litho/MyComponent.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.litho;
+
+import com.facebook.litho.Component;
+import com.facebook.litho.annotations.Prop;
+
+public class MyComponent extends Component {
+  @Prop Object prop1; // implicitly non-optional
+
+  @Prop(optional = true)
+  Object prop2; // explicitly optional
+
+  @Prop(optional = false)
+  Object prop3; // explicitly non-optional
+
+  Object nonProp;
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  public static class Builder extends Component.Builder<Builder> {
+    MyComponent mMyComponent;
+
+    public Builder prop1(Object o) {
+      this.mMyComponent.prop1 = o;
+      return this;
+    }
+
+    public Builder prop2(Object o) {
+      this.mMyComponent.prop2 = o;
+      return this;
+    }
+
+    public Builder prop3(Object o) {
+      this.mMyComponent.prop3 = o;
+      return this;
+    }
+
+    public MyComponent build() {
+      return mMyComponent;
+    }
+
+    @Override
+    public Builder getThis() {
+      return this;
+    }
+  }
+}

infer/tests/codetoanalyze/java/litho/MyLithoComponent.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package com.facebook.litho;
+
+import com.facebook.litho.annotations.Prop;
+
+public class MyLithoComponent extends Component {
+  @Prop Object prop1; // implicitly non-optional
+
+  @Prop(optional = false)
+  Object prop2; // explicitly non-optional
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  public static class Builder extends Component.Builder<Builder> {
+    MyLithoComponent mMyLithoComponent;
+
+    public Builder prop1(Object o) {
+      this.mMyLithoComponent.prop1 = o;
+      return this;
+    }
+
+    public Builder prop2(Object o) {
+      this.mMyLithoComponent.prop2 = o;
+      return this;
+    }
+
+    public MyLithoComponent build() {
+      return mMyLithoComponent;
+    }
+
+    @Override
+    public Builder getThis() {
+      return this;
+    }
+  }
+}

infer/tests/codetoanalyze/java/litho/MyTreeComponent.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.litho;
+
+import com.facebook.litho.Component;
+import com.facebook.litho.annotations.TreeProp;
+
+class MyTreeComponent extends Component {
+  @TreeProp Object prop1; // implicitly non-optional
+
+  Object nonProp;
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  static class Builder extends Component.Builder<Builder> {
+    MyTreeComponent mMyTreeComponent;
+
+    public Builder prop1(Object o) {
+      this.mMyTreeComponent.prop1 = o;
+      return this;
+    }
+
+    public MyTreeComponent build() {
+      return mMyTreeComponent;
+    }
+
+    @Override
+    public Builder getThis() {
+      return this;
+    }
+  }
+}

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

@@ -9,217 +9,13 @@ package codetoanalyze.java.litho;
 
 import com.facebook.litho.Column;
 import com.facebook.litho.Component;
-import com.facebook.litho.annotations.Prop;
+import com.facebook.litho.MyLithoComponent;
-import com.facebook.litho.annotations.ResType;
-import com.facebook.litho.annotations.TreeProp;
 import java.util.ArrayList;
-import java.util.List;
-
-class MyComponent extends Component {
-  @Prop Object prop1; // implicitly non-optional
-
-  @Prop(optional = true)
-  Object prop2; // explicitly optional
-
-  @Prop(optional = false)
-  Object prop3; // explicitly non-optional
-
-  Object nonProp;
-
-
-  public Builder create() {
-    return new Builder();
-  }
-
-  static class Builder extends Component.Builder<Builder> {
-    MyComponent mMyComponent;
-
-    public Builder prop1(Object o) {
-      this.mMyComponent.prop1 = o;
-      return this;
-    }
-
-    public Builder prop2(Object o) {
-      this.mMyComponent.prop2 = o;
-      return this;
-    }
-
-    public Builder prop3(Object o) {
-      this.mMyComponent.prop3 = o;
-      return this;
-    }
-
-    public MyComponent build() {
-      return mMyComponent;
-    }
-
-    @Override
-    public Builder getThis() {
-      return this;
-    }
-  }
-}
-
-class MyTreeComponent extends Component {
-  @TreeProp Object prop1; // implicitly non-optional
-
-  Object nonProp;
-
-  public Builder create() {
-    return new Builder();
-  }
-
-  static class Builder extends Component.Builder<Builder> {
-    MyTreeComponent mMyTreeComponent;
-
-    public Builder prop1(Object o) {
-      this.mMyTreeComponent.prop1 = o;
-      return this;
-    }
-
-    public MyTreeComponent build() {
-      return mMyTreeComponent;
-    }
-
-    @Override
-    public Builder getThis() {
-      return this;
-    }
-  }
-}
-
-/**
- * using @Prop(resType = ..) allows you to set the Prop with any of .propname, .propnameRes, or
- * .propnameAttr
- */
-class ResPropComponent extends Component {
-
-  @Prop(resType = ResType.SOME)
-  Object prop; // implicitly non-optional with resType
-
-  public Builder create() {
-    return new Builder();
-  }
-
-  static class Builder extends Component.Builder<Builder> {
-
-    ResPropComponent mResPropComponent;
-
-    public Builder prop(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public Builder propRes(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public Builder propAttr(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public Builder propDip(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public Builder propPx(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public Builder propSp(Object o) {
-      this.mResPropComponent.prop = o;
-      return this;
-    }
-
-    public ResPropComponent build() {
-      return mResPropComponent;
-    }
-
-    @Override
-    public Builder getThis() {
-      return this;
-    }
-  }
-}
-
-/** varArg test */
-class VarArgPropComponent extends Component {
-
-  @Prop(varArg = "prop")
-  List<Object> props;
-
-  public Builder create() {
-    return new Builder();
-  }
-
-  static class Builder extends Component.Builder<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;
-    }
-
-    @Override
-    public Builder getThis() {
-      return this;
-    }
-  }
-}
 
 public class RequiredProps {
 
   public MyComponent mMyComponent;
+  public MyLithoComponent mMyLithoComponent;
   public ResPropComponent mResPropComponent;
   public VarArgPropComponent mVarArgPropComponent;
 
@@ -383,6 +179,27 @@ public class RequiredProps {
     mVarArgPropComponent.create().build();
   }
 
+  // can't track MyLithoComponent.build() because it is on litho, hence we miss the error
+  public Component buildPropLithoMissingBothBad_FN() {
+    return mMyLithoComponent.create().build();
+  }
+
+  // we pick up Component.build() rather than
+  // MyLithoComponent.build(). Hence, we can't detect that it has
+  // missing props at all.
+  public void buildPropLithoMissingOneBad_FN() {
+    Column.create()
+        .child(mMyLithoComponent.create().prop1(new Object()).commonProp(new Object()))
+        .build();
+  }
+
+  // We can't track prop1 and prop2 because they are on litho
+  public Component buildPropLithoOK_FP() {
+    Component.Builder layoutBuilder =
+        mMyLithoComponent.create().prop1(new Object()).prop2(new Object());
+    return layoutBuilder.build();
+  }
+
   public class NonRequiredTreeProps {
 
     public MyTreeComponent mMyTreeComponent;

infer/tests/codetoanalyze/java/litho/ResPropComponent.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.litho;
+
+import com.facebook.litho.Component;
+import com.facebook.litho.annotations.Prop;
+import com.facebook.litho.annotations.ResType;
+
+/**
+ * using @Prop(resType = ..) allows you to set the Prop with any of .propname, .propnameRes, or
+ * .propnameAttr
+ */
+class ResPropComponent extends Component {
+
+  @Prop(resType = ResType.SOME)
+  Object prop; // implicitly non-optional with resType
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  static class Builder extends Component.Builder<Builder> {
+
+    ResPropComponent mResPropComponent;
+
+    public Builder prop(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public Builder propRes(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public Builder propAttr(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public Builder propDip(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public Builder propPx(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public Builder propSp(Object o) {
+      this.mResPropComponent.prop = o;
+      return this;
+    }
+
+    public ResPropComponent build() {
+      return mResPropComponent;
+    }
+
+    @Override
+    public Builder getThis() {
+      return this;
+    }
+  }
+}

infer/tests/codetoanalyze/java/litho/VarArgPropComponent.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.litho;
+
+import com.facebook.litho.Component;
+import com.facebook.litho.annotations.Prop;
+import java.util.ArrayList;
+import java.util.List;
+
+/** varArg test */
+class VarArgPropComponent extends Component {
+
+  @Prop(varArg = "prop")
+  List<Object> props;
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  static class Builder extends Component.Builder<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;
+    }
+
+    @Override
+    public Builder getThis() {
+      return this;
+    }
+  }
+}

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

@@ -1,11 +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(),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.buildPropLithoOK_FP():com.facebook.litho.Component, 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 MyLithoComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropLithoOK_FP():com.facebook.litho.Component, 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(),calls MyLithoComponent MyLithoComponent$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.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 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$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()]