From 776d4bf97d40d094f45f358d5936320dc24dcc51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Tue, 10 Dec 2019 02:46:28 -0800 Subject: [PATCH] [litho] Change the domain Summary: Making `MethodCalled` an inverted map from created location to method calls results in not being able to track a builder that is created in two different branches of a conditional with different types. Instead, we can make `MethodCalled` simply a map and also change `Created` to be a map from access paths to a set of created locations. To deal with the case of setting a prop only in one branch, we need to ensure that whenever we call a create method, we add a binding to `MethodCalled` with an empty list of methods so that its intersection with a non-empty one is empty. Reviewed By: skcho Differential Revision: D18883097 fbshipit-source-id: b3464ca20 --- infer/src/checkers/LithoDomain.ml | 10 +++--- .../litho-required-props/RequiredProps.java | 32 +++++++++++++++++++ .../java/litho-required-props/issues.exp | 4 +++ .../java/litho/RequiredProps.java | 32 +++++++++++++++++++ .../tests/codetoanalyze/java/litho/issues.exp | 4 +++ 5 files changed, 78 insertions(+), 4 deletions(-) diff --git a/infer/src/checkers/LithoDomain.ml b/infer/src/checkers/LithoDomain.ml index b5d1858a8..9166be6a8 100644 --- a/infer/src/checkers/LithoDomain.ml +++ b/infer/src/checkers/LithoDomain.ml @@ -84,10 +84,10 @@ module NewDomain = struct F.fprintf fmt "Created at %a with type %a" Location.pp location Typ.Name.pp typ_name end - module CreatedLocations = AbstractDomain.InvertedSet (CreatedLocation) + module CreatedLocations = AbstractDomain.FiniteSet (CreatedLocation) module Created = struct - include AbstractDomain.InvertedMap (LocalAccessPath) (CreatedLocations) + include AbstractDomain.Map (LocalAccessPath) (CreatedLocations) let lookup k x = Option.value (find_opt k x) ~default:CreatedLocations.empty end @@ -152,7 +152,7 @@ module NewDomain = struct end module MethodCalled = struct - include AbstractDomain.InvertedMap (CreatedLocation) (MethodCalls) + include AbstractDomain.Map (CreatedLocation) (MethodCalls) let add_one k v x = let f = function @@ -225,7 +225,9 @@ module NewDomain = struct let call_create lhs typ_name location ({created} as x) = - {x with created= Created.add lhs (CreatedLocations.singleton {location; typ_name}) created} + let created_location = CreatedLocation.{location; typ_name} in + { created= Created.add lhs (CreatedLocations.singleton created_location) created + ; method_called= MethodCalled.add created_location MethodCalls.empty x.method_called } let call_builder ~ret ~receiver callee {created; method_called} = diff --git a/infer/tests/codetoanalyze/java/litho-required-props/RequiredProps.java b/infer/tests/codetoanalyze/java/litho-required-props/RequiredProps.java index 8583976c7..d6dfa2349 100644 --- a/infer/tests/codetoanalyze/java/litho-required-props/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho-required-props/RequiredProps.java @@ -379,4 +379,36 @@ public class RequiredProps { } return builder.prop2(new Object()).prop3(new Object()).build(); } + + public Component missingProp3InOneBranchBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()) + : mMyLithoComponent.create().prop1(new Object()).prop2(new Object()); + return builder.build(); + } + + public Component missingProp2InOneBranchBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()).prop3(new Object()) + : mMyLithoComponent.create().prop1(new Object()); + return builder.build(); + } + + public Component missingProp1InBothBranchesBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop3(new Object()) + : mMyLithoComponent.create().prop2(new Object()); + return builder.build(); + } + + public Component createDiffferentInBranchesBeforeBuildOk(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()).prop3(new Object()) + : mMyLithoComponent.create().prop1(new Object()).prop2(new Object()); + return builder.build(); + } } diff --git a/infer/tests/codetoanalyze/java/litho-required-props/issues.exp b/infer/tests/codetoanalyze/java/litho-required-props/issues.exp index 2bbd3da17..1822d0826 100644 --- a/infer/tests/codetoanalyze/java/litho-required-props/issues.exp +++ b/infer/tests/codetoanalyze/java/litho-required-props/issues.exp @@ -15,6 +15,10 @@ codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.l codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castImpossibleOk_FP(java.lang.Object):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()] codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.doubleSetMissingBad():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 Component$Builder Component$Builder.commonProp(Object),calls MyComponent$Builder MyComponent$Builder.prop3(Object)] codetoanalyze/java/litho-required-props/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)] +codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp1InBothBranchesBeforeBuildBad(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.prop3(Object)] +codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp1InBothBranchesBeforeBuildBad(boolean):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$Builder.prop2(Object)] +codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp2InOneBranchBeforeBuildBad(boolean):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$Builder MyLithoComponent$Builder.prop1(Object)] +codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp3InOneBranchBeforeBuildBad(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$Builder.prop1(Object)] codetoanalyze/java/litho-required-props/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$Builder.prop1(Object),calls MyComponent$Builder MyComponent$Builder.prop2(Object)] codetoanalyze/java/litho-required-props/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)] codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchEffectfulBad(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)] diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java index bae8d09cd..5dd4f3ab5 100644 --- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java @@ -382,4 +382,36 @@ public class RequiredProps { } return builder.prop2(new Object()).prop3(new Object()).build(); } + + public Component missingProp3InOneBranchBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()) + : mMyLithoComponent.create().prop1(new Object()).prop2(new Object()); + return builder.build(); + } + + public Component missingProp2InOneBranchBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()).prop3(new Object()) + : mMyLithoComponent.create().prop1(new Object()); + return builder.build(); + } + + public Component missingProp1InBothBranchesBeforeBuildBad(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop3(new Object()) + : mMyLithoComponent.create().prop2(new Object()); + return builder.build(); + } + + public Component createDiffferentInBranchesBeforeBuildOk(boolean b) { + Component.Builder builder = + b + ? mMyComponent.create().prop1(new Object()).prop3(new Object()) + : mMyLithoComponent.create().prop1(new Object()).prop2(new Object()); + return builder.build(); + } } diff --git a/infer/tests/codetoanalyze/java/litho/issues.exp b/infer/tests/codetoanalyze/java/litho/issues.exp index f975a6b9d..125c25e85 100644 --- a/infer/tests/codetoanalyze/java/litho/issues.exp +++ b/infer/tests/codetoanalyze/java/litho/issues.exp @@ -21,6 +21,10 @@ codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredPr codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castImpossibleOk_FP(java.lang.Object):void, 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 MyLithoComponent$Builder MyLithoComponent.create(),calls MyComponent MyComponent$Builder.build()] codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.castMissingOneBad(java.lang.Object):void, 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$Builder MyLithoComponent$Builder.prop2(Object),calls MyLithoComponent MyLithoComponent$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.missingProp1InBothBranchesBeforeBuildBad(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.prop3(Object),calls Component MyLithoComponent$Builder.build()] +codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp1InBothBranchesBeforeBuildBad(boolean):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$Builder MyLithoComponent$Builder.prop2(Object),calls Component MyLithoComponent$Builder.build()] +codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp2InOneBranchBeforeBuildBad(boolean):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$Builder MyLithoComponent.create(),calls MyLithoComponent$Builder MyLithoComponent$Builder.prop1(Object),calls Component MyLithoComponent$Builder.build()] +codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.missingProp3InOneBranchBeforeBuildBad(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.prop1(Object),calls Component MyLithoComponent$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.setRequiredEffectful_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()]