[litho] Make MethodCall comparison oblivious to prop suffixes

Summary: Current method call comparison is too strong. As exemplified with the new test, one can also set the required prop by calling a version which contains the suffixes. The domain should take care of such cases now.

Reviewed By: skcho

Differential Revision: D18808869

fbshipit-source-id: 9f7672e75

infer/src/checkers/LithoDomain.ml

@@ -30,6 +30,8 @@ module LocalAccessPath = struct
   let pp fmt t = AccessPath.pp fmt t.access_path
 end
 
+let suffixes = String.Set.of_list ["Attr"; "Dip"; "Px"; "Res"; "Sp"]
+
 module MethodCall = struct
   type t = {receiver: LocalAccessPath.t; procname: Typ.Procname.t; location: Location.t}
   [@@deriving compare]
@@ -68,9 +70,39 @@ module NewDomain = struct
     module S = AbstractDomain.InvertedSet (struct
       include MethodCall
 
+      let compare_procname p1 p2 =
+        let chopped_opt pname =
+          let method_name = Typ.Procname.get_method pname in
+          String.Set.find_map suffixes ~f:(fun suffix -> String.chop_suffix method_name ~suffix)
+        in
+        let replace_both s p =
+          match p with
+          | Typ.Procname.Java java_pname ->
+              (* This is a bit of a hack to quickly compare based on only
+                 method names *)
+              Typ.Procname.Java
+                ( Typ.Procname.Java.replace_method_name s java_pname
+                |> Typ.Procname.Java.replace_parameters [] )
+          | _ ->
+              p
+        in
+        let p1, p2 =
+          match (chopped_opt p1, chopped_opt p2) with
+          | Some chopped1, Some chopped2 ->
+              (replace_both chopped1 p1, replace_both chopped2 p2)
+          | Some chopped1, None ->
+              (replace_both chopped1 p1, Typ.Procname.replace_parameters [] p2)
+          | None, Some chopped2 ->
+              (Typ.Procname.replace_parameters [] p1, replace_both chopped2 p2)
+          | _ ->
+              (p1, p2)
+        in
+        Typ.Procname.compare p1 p2
+
+
       let compare x y =
         let r = LocalAccessPath.compare x.receiver y.receiver in
-        if r <> 0 then r else Typ.Procname.compare x.procname y.procname
+        if r <> 0 then r else compare_procname x.procname y.procname
     end)
 
     (* TODO: add return type && and do not add return method to the set *)

infer/src/checkers/LithoDomain.mli

@@ -21,6 +21,8 @@ module LocalAccessPath : sig
   val pp : F.formatter -> t -> unit
 end
 
+val suffixes : String.Set.t
+
 (** Called procedure & location + its receiver *)
 module MethodCall : sig
   type t = private {receiver: LocalAccessPath.t; procname: Typ.Procname.t; location: Location.t}

infer/src/checkers/RequiredProps.ml

@@ -92,8 +92,6 @@ let find_client_component_type call_chain =
     call_chain
 
 
-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
@@ -102,7 +100,7 @@ let has_prop prop_set prop =
           that all @Prop's can be set any of these 6 ways. *)
     String.Set.exists prop_set ~f:(fun el ->
         String.chop_prefix el ~prefix:prop
-        |> Option.exists ~f:(fun suffix -> String.Set.mem suffixes suffix) )
+        |> Option.exists ~f:(fun suffix -> String.Set.mem LithoDomain.suffixes suffix) )
   in
   match prop with
   | Prop prop ->

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

@@ -230,6 +230,26 @@ public class RequiredProps {
     mResPropComponent.create().build();
   }
 
+  public void buildPropResInCondOk_FP(boolean b) {
+    ResPropComponent.Builder builder = mResPropComponent.create();
+    if (b) {
+      builder.propAttr(new Object());
+    } else {
+      builder.propDip(new Object());
+    }
+    builder.build();
+  }
+
+  public void buildPropResInCondOneNormalOk_FP(boolean b) {
+    ResPropComponent.Builder builder = mResPropComponent.create();
+    if (b) {
+      builder.propAttr(new Object());
+    } else {
+      builder.prop(new Object());
+    }
+    builder.build();
+  }
+
   public void buildPropVarArgNormalOk() {
     mVarArgPropComponent.create().props(new ArrayList<Object>()).build();
   }

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

@@ -14,7 +14,7 @@ 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 {
+public class ResPropComponent extends Component {
 
   @Prop(resType = ResType.SOME)
   Object prop; // implicitly non-optional with resType
@@ -23,7 +23,7 @@ class ResPropComponent extends Component {
     return new Builder();
   }
 
-  static class Builder extends Component.Builder<Builder> {
+  public static class Builder extends Component.Builder<Builder> {
 
     ResPropComponent mResPropComponent;
 

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

@@ -2,6 +2,8 @@ codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredPr
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropLithoMissingBothBad():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$Builder MyLithoComponent.create(),calls MyLithoComponent MyLithoComponent$Builder.build()]
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropLithoMissingOneBad():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(),calls MyLithoComponent$Builder MyLithoComponent.create(),calls MyLithoComponent$Builder MyLithoComponent$Builder.prop1(Object),calls Component$Builder Component$Builder.commonProp(Object),calls Component Component$Builder.build()]
 codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropLithoMissingOneInLoopBad(int):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(),calls MyLithoComponent$Builder MyLithoComponent.create(),calls MyLithoComponent$Builder MyLithoComponent$Builder.prop1(Object),calls Component$Builder Component$Builder.commonProp(Object),calls Component Component$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResInCondOk_FP(boolean):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$Builder ResPropComponent.create(),calls ResPropComponent ResPropComponent$Builder.build()]
+codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResInCondOneNormalOk_FP(boolean):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$Builder ResPropComponent.create(),calls ResPropComponent ResPropComponent$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$Builder ResPropComponent.create(),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$Builder VarArgPropComponent.create(),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.create(),calls MyComponent$Builder MyComponent$Builder.prop1(Object),calls Component Component$Builder.build()]