From ddf6254b6f4c968c6611704f58e7148ffb8259a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Thu, 14 Nov 2019 06:42:29 -0800 Subject: [PATCH] [required-props] Only report when there is a create Summary: Due to the weakness of the analysis which can't detect side-effecting prop setting (e.g. as in `builder.prop1(..)`), we currently have many broken chains that do do have any `create` method in their prefixes. Let's not report on these broken chains. Reviewed By: skcho Differential Revision: D18503523 fbshipit-source-id: 7506e34b7 --- infer/src/checkers/RequiredProps.ml | 4 ++++ infer/tests/codetoanalyze/java/litho/RequiredProps.java | 9 ++++++++- infer/tests/codetoanalyze/java/litho/issues.exp | 7 +++---- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/infer/src/checkers/RequiredProps.ml b/infer/src/checkers/RequiredProps.ml index e04c191de..e68a96087 100644 --- a/infer/src/checkers/RequiredProps.ml +++ b/infer/src/checkers/RequiredProps.ml @@ -155,6 +155,10 @@ module LithoContext = struct let report astate tenv summary = let check_required_prop_chain _ call_chain = + let call_chain = + List.drop_while call_chain ~f:(fun Domain.MethodCall.{procname} -> + not (LithoFramework.is_component_create_method procname tenv) ) + in let rev_chain = List.rev call_chain in match rev_chain with | Domain.MethodCall.{procname} :: _ diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java index 65b81fb8e..b1c981c6e 100644 --- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java @@ -109,7 +109,14 @@ public class RequiredProps { } // due to mutual recursion check, we break cycle at seen props - public Component doubleSetOk_FP() { + public Component doubleSetMissingBad_FN() { + Component.Builder builder = + mMyComponent.create().commonProp(new Object()).prop3(new Object()).commonProp(new Object()); + return builder.build(); + } + + // due to mutual recursion check, we break cycle at seen props + public Component doubleSetCommonOk() { Component.Builder builder = mMyComponent .create() diff --git a/infer/tests/codetoanalyze/java/litho/issues.exp b/infer/tests/codetoanalyze/java/litho/issues.exp index a8764f3ef..a2115cd10 100644 --- a/infer/tests/codetoanalyze/java/litho/issues.exp +++ b/infer/tests/codetoanalyze/java/litho/issues.exp @@ -7,15 +7,14 @@ codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredPr 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.create(),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.create(),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.create(),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.doubleSetOk_FP():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 Component$Builder Component$Builder.commonProp(Object),calls Component Component$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.create(),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.create(),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.setRequiredOk_FP(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.setRequiredOk_FP(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.create(),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.setRequiredOnBothBranchesMissingProp3Bad(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.create(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()] codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnBothBranchesMissingProp3Bad(boolean):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.create(),calls MyComponent$Builder MyComponent$Builder.prop2(Object),calls MyComponent MyComponent$Builder.build()] codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnBothBranchesNoAssignOk_FP(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.create(),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.setRequiredOnBothBranchesOk_FP(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.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/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnBothBranchesOk_FP(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.create(),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.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.create(),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()] codetoanalyze/java/litho/ShouldUpdate.java, com.facebook.graphql.model.LithoTest.onCreateLayout():void, 0, GRAPHQL_FIELD_ACCESS, no_bucket, ERROR, [story.getActors().get(...).toString()] codetoanalyze/java/litho/ShouldUpdate.java, com.facebook.graphql.model.LithoTest.onCreateLayout():void, 0, GRAPHQL_FIELD_ACCESS, no_bucket, ERROR, [story.getActors().get(...)]