From df712bc629442b915b7ca85c2d5a1e599af6dc24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= <ezgi@fb.com>
Date: Tue, 24 Sep 2019 01:54:47 -0700
Subject: [PATCH] [required-props] Refine Required Props checker to only check
 @Prop

Summary: Before, we were mistakenly checking any annotation that ends with Prop such as TreeProp. This was wrong. Instead, we should only check Prop as adviced by the Litho team.

Reviewed By: ngorogiannis

Differential Revision: D17527769

fbshipit-source-id: b753dd87a
---
 infer/src/checkers/Litho.ml                   |  4 +-
 .../java/litho/RequiredProps.java             | 40 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/infer/src/checkers/Litho.ml b/infer/src/checkers/Litho.ml
index 4f9e69a26..d59eb4fa3 100644
--- a/infer/src/checkers/Litho.ml
+++ b/infer/src/checkers/Litho.ml
@@ -94,8 +94,8 @@ module RequiredProps = struct
   let get_required_props typename tenv =
     let is_required annot_list =
       List.exists
-        ~f:(fun ({Annot.class_name; parameters}, _) ->
-          String.is_suffix class_name ~suffix:Annotations.prop
+        ~f:(fun (({Annot.parameters} as annot), _) ->
+          Annotations.annot_ends_with annot Annotations.prop
           && (* Don't count as required if it's @Prop(optional = true) *)
              not (List.exists ~f:(fun annot_string -> String.equal annot_string "true") parameters)
           )
diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java
index c660e3d6e..74d1ef707 100644
--- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java
+++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java
@@ -27,6 +27,14 @@ enum ResType {
   boolean optional() default false;
 }
 
+@Target({ElementType.PARAMETER, ElementType.FIELD})
+@Retention(RetentionPolicy.CLASS)
+@interface TreeProp {
+  ResType resType() default ResType.NONE;
+
+  boolean optional() default false;
+}
+
 class MyComponent extends Component {
   @Prop Object prop1; // implicitly non-optional
 
@@ -66,6 +74,29 @@ class MyComponent extends Component {
   }
 }
 
+class MyTreeComponent extends Component {
+  @TreeProp Object prop1; // implicitly non-optional
+
+  Object nonProp;
+
+  public Builder create() {
+    return new Builder();
+  }
+
+  static class Builder extends Component.Builder {
+    MyTreeComponent mMyTreeComponent;
+
+    public Builder prop1(Object o) {
+      this.mMyTreeComponent.prop1 = o;
+      return this;
+    }
+
+    public MyTreeComponent build() {
+      return mMyTreeComponent;
+    }
+  }
+}
+
 /**
  * using @Prop(resType = ..) allows you to set the Prop with any of .propname, .propnameRes, or
  * .propnameAttr
@@ -216,4 +247,13 @@ public class RequiredProps {
   public void buildPropResMissingBad() {
     mResPropComponent.create().build();
   }
+
+  public class NonRequiredTreeProps {
+
+    public MyTreeComponent mMyTreeComponent;
+
+    public MyTreeComponent buildWithoutOk() {
+      return mMyTreeComponent.create().build();
+    }
+  }
 }