diff --git a/infer/src/checkers/Litho.ml b/infer/src/checkers/Litho.ml index 5698f0507..90b7ea63f 100644 --- a/infer/src/checkers/Litho.ml +++ b/infer/src/checkers/Litho.ml @@ -153,6 +153,14 @@ module RequiredProps = struct && Procdesc.get_access proc_desc <> PredSymb.Private + let has_prop prop_set prop = + String.Set.mem prop_set prop + (* @Prop(resType = ...) myProp can also be set via myProp(), myPropAttr(), or myPropRes(). + Our annotation parameter parsing is too primitive to identify resType, so just assume + that all @Prop's can be set any of these 3 ways. *) + || String.Set.mem prop_set (prop ^ "Attr") || String.Set.mem prop_set (prop ^ "Res") + + let report astate tenv summary = let check_required_prop_chain _ call_chain = let rev_chain = List.rev call_chain in @@ -169,7 +177,7 @@ module RequiredProps = struct let prop_set = List.map ~f:Typ.Procname.get_method call_chain |> String.Set.of_list in List.iter ~f:(fun required_prop -> - if not (String.Set.mem prop_set required_prop) then + if not (has_prop prop_set required_prop) then report_missing_required_prop summary required_prop (Specs.get_loc summary) ) required_props | _ -> diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java index 01068ea1a..4deeef35d 100644 --- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java @@ -24,19 +24,19 @@ enum ResType { @Target({ ElementType.PARAMETER, ElementType.FIELD }) @Retention(RetentionPolicy.CLASS) @interface Prop { - ResType resType(); + ResType resType() default ResType.NONE; boolean optional() default false; } class MyComponent extends Component { - @Prop(resType = ResType.NONE, optional = false) - Object prop1; + @Prop + Object prop1; // implicitly non-optional - @Prop(resType = ResType.NONE, optional = true) - Object prop2; + @Prop(optional = true) + Object prop2; // explicitly optional - @Prop(resType = ResType.SOME, optional = false) - Object prop3; + @Prop(optional = false) + Object prop3; // explicitly non-optional Object nonProp; @@ -70,9 +70,49 @@ class MyComponent extends Component { } +/** 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 { + + 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 ResPropComponent build() { + return mResPropComponent; + } + + } + +} + public class RequiredProps { public MyComponent mMyComponent; + public ResPropComponent mResPropComponent; public MyComponent buildWithAllOk() { return @@ -198,4 +238,31 @@ public class RequiredProps { builder.child(childBuilder); } + public void buildPropResWithNormalOk() { + mResPropComponent + .create() + .prop(new Object()) + .build(); + } + + public void buildPropResWithResOk() { + mResPropComponent + .create() + .propRes(new Object()) + .build(); + } + + public void buildPropResWithAttrOk() { + mResPropComponent + .create() + .propAttr(new Object()) + .build(); + } + + public void buildPropResMissingBad() { + mResPropComponent + .create() + .build(); + } + } diff --git a/infer/tests/codetoanalyze/java/litho/issues.exp b/infer/tests/codetoanalyze/java/litho/issues.exp index 505f09dd4..c9d7866be 100644 --- a/infer/tests/codetoanalyze/java/litho/issues.exp +++ b/infer/tests/codetoanalyze/java/litho/issues.exp @@ -5,6 +5,7 @@ codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.callBuild codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.setProp3InCalleeButForgetProp1Bad(), 0, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.setRequiredOnOneBranchBad(boolean), 0, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, Object RequiredProps.generalTypeForgot3Bad(), 0, MISSING_REQUIRED_PROP, [@Prop prop3 is required, but not set before the call to build()] +codetoanalyze/java/litho/RequiredProps.java, void RequiredProps.buildPropResMissingBad(), 0, MISSING_REQUIRED_PROP, [@Prop prop is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, void RequiredProps.buildWithColumnChildBad(), 0, MISSING_REQUIRED_PROP, [@Prop prop3 is required, but not set before the call to build()] codetoanalyze/java/litho/ShouldUpdate.java, void LithoTest.onCreateLayout(), 0, GRAPHQL_FIELD_ACCESS, [&story.getActors().get(...)] codetoanalyze/java/litho/ShouldUpdate.java, void LithoTest.onCreateLayout(), 0, GRAPHQL_FIELD_ACCESS, [&story.getActors().get(...).toString()]