From c68dc542b72cbb5a7a2d63b766aa17c63eafb4f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Tue, 1 Oct 2019 06:03:57 -0700 Subject: [PATCH] [required-props] Refactor tests Summary: - Refactor tests for better modularity for future tests. - Add a positive check for child builder pattern. - Instead of returning MyComponent, return Component to mimic generated code better. Reviewed By: Katalune Differential Revision: D17684995 fbshipit-source-id: d0b851e34 --- .../tests/codetoanalyze/java/litho/Prop.java | 22 +++++++ .../java/litho/RequiredProps.java | 65 ++++++++----------- .../codetoanalyze/java/litho/ResType.java | 12 ++++ .../codetoanalyze/java/litho/TreeProp.java | 20 ++++++ .../tests/codetoanalyze/java/litho/issues.exp | 12 ++-- 5 files changed, 86 insertions(+), 45 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/litho/Prop.java create mode 100644 infer/tests/codetoanalyze/java/litho/ResType.java create mode 100644 infer/tests/codetoanalyze/java/litho/TreeProp.java diff --git a/infer/tests/codetoanalyze/java/litho/Prop.java b/infer/tests/codetoanalyze/java/litho/Prop.java new file mode 100644 index 000000000..cf49d01e3 --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho/Prop.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +package com.facebook.litho.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.PARAMETER, ElementType.FIELD}) +@Retention(RetentionPolicy.CLASS) +public @interface Prop { + ResType resType() default ResType.NONE; + + boolean optional() default false; + + String varArg() default ""; +} diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java index d77a64972..900558249 100644 --- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java @@ -9,36 +9,12 @@ package codetoanalyze.java.litho; import com.facebook.litho.Column; import com.facebook.litho.Component; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; +import com.facebook.litho.annotations.Prop; +import com.facebook.litho.annotations.ResType; +import com.facebook.litho.annotations.TreeProp; import java.util.ArrayList; import java.util.List; -enum ResType { - SOME, - NONE -} - -@Target({ElementType.PARAMETER, ElementType.FIELD}) -@Retention(RetentionPolicy.CLASS) -@interface Prop { - ResType resType() default ResType.NONE; - - boolean optional() default false; - - String varArg() default ""; -} - -@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 @@ -227,7 +203,7 @@ public class RequiredProps { public ResPropComponent mResPropComponent; public VarArgPropComponent mVarArgPropComponent; - public MyComponent buildWithAllOk() { + public Component buildWithAllOk() { return mMyComponent .create() .prop1(new Object()) @@ -237,17 +213,17 @@ public class RequiredProps { } // prop 2 is optional - public MyComponent buildWithout2Ok() { + public Component buildWithout2Ok() { return mMyComponent.create().prop1(new Object()).prop3(new Object()).build(); } // prop 1 is required - public MyComponent buildWithout1Bad() { + public Component buildWithout1Bad() { return mMyComponent.create().prop2(new Object()).prop3(new Object()).build(); } // prop3 is required - public MyComponent buildWithout3Bad() { + public Component buildWithout3Bad() { return mMyComponent.create().prop1(new Object()).prop2(new Object()).build(); } @@ -259,19 +235,19 @@ public class RequiredProps { return builder.prop3(new Object()); } - public MyComponent setProp1InCalleeOk() { + public Component setProp1InCalleeOk() { return setProp1(mMyComponent.create().prop2(new Object())).prop3(new Object()).build(); } - public MyComponent setProp3InCalleeOk() { + public Component setProp3InCalleeOk() { return setProp3(mMyComponent.create().prop1(new Object()).prop2(new Object())).build(); } - public MyComponent setProp3InCalleeButForgetProp1Bad() { + public Component setProp3InCalleeButForgetProp1Bad() { return setProp3(mMyComponent.create()).prop2(new Object()).build(); } - public MyComponent setRequiredOnOneBranchBad(boolean b) { + public Component setRequiredOnOneBranchBad(boolean b) { MyComponent.Builder builder = mMyComponent.create(); if (b) { builder = builder.prop1(new Object()); @@ -279,7 +255,7 @@ public class RequiredProps { return builder.prop2(new Object()).prop3(new Object()).build(); } - public MyComponent FP_setRequiredOnBothBranchesOk(boolean b) { + public Component FP_setRequiredOnBothBranchesOk(boolean b) { MyComponent.Builder builder = mMyComponent.create(); if (b) { builder = builder.prop1(new Object()); @@ -295,15 +271,20 @@ public class RequiredProps { } // shouldn't report here; prop 1 passed - public MyComponent callBuildSuffixWithRequiredOk() { + public Component callBuildSuffixWithRequiredOk() { return buildSuffix(mMyComponent.create().prop1(new Object())); } // should report here; forgot prop 1 - public MyComponent callBuildSuffixWithoutRequiredBad() { + public Component callBuildSuffixWithoutRequiredBad() { return buildSuffix(mMyComponent.create()); } + public Object generalTypeWithout2Ok() { + Component.Builder builder = mMyComponent.create().prop1(new Object()).prop3(new Object()); + return builder.build(); + } + public Object generalTypeForgot3Bad() { MyComponent.Builder builder1 = mMyComponent.create(); Component.Builder builder2 = (Component.Builder) builder1.prop1(new Object()); @@ -314,11 +295,17 @@ public class RequiredProps { public void buildWithColumnChildBad() { Column.Builder builder = Column.create(); - MyComponent.Builder childBuilder = mMyComponent.create().prop1(new Object()); + Component.Builder childBuilder = mMyComponent.create().prop1(new Object()); // forgot prop 3, and builder.child() will invoke build() on childBuilder builder.child(childBuilder); } + public Component buildWithColumnChildOk() { + return Column.create() + .child(mMyComponent.create().prop1(new Object()).prop3(new Object())) + .build(); + } + public void buildPropResWithNormalOk() { mResPropComponent.create().prop(new Object()).build(); } diff --git a/infer/tests/codetoanalyze/java/litho/ResType.java b/infer/tests/codetoanalyze/java/litho/ResType.java new file mode 100644 index 000000000..200b1066b --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho/ResType.java @@ -0,0 +1,12 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +package com.facebook.litho.annotations; + +public enum ResType { + SOME, + NONE +} diff --git a/infer/tests/codetoanalyze/java/litho/TreeProp.java b/infer/tests/codetoanalyze/java/litho/TreeProp.java new file mode 100644 index 000000000..2560f427f --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho/TreeProp.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +package com.facebook.litho.annotations; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.PARAMETER, ElementType.FIELD}) +@Retention(RetentionPolicy.CLASS) +public @interface TreeProp { + ResType resType() default ResType.NONE; + + boolean optional() default false; +} diff --git a/infer/tests/codetoanalyze/java/litho/issues.exp b/infer/tests/codetoanalyze/java/litho/issues.exp index 5a3accddb..b1bc18ab3 100644 --- a/infer/tests/codetoanalyze/java/litho/issues.exp +++ b/infer/tests/codetoanalyze/java/litho/issues.exp @@ -1,13 +1,13 @@ -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.FP_setRequiredOnBothBranchesOk(boolean):codetoanalyze.java.litho.MyComponent, 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()] +codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.FP_setRequiredOnBothBranchesOk(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()] 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()] 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()] 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()] -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout1Bad():codetoanalyze.java.litho.MyComponent, 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()] -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout3Bad():codetoanalyze.java.litho.MyComponent, 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()] -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.callBuildSuffixWithoutRequiredBad():codetoanalyze.java.litho.MyComponent, 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()] +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()] +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()] +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()] 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()] -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setProp3InCalleeButForgetProp1Bad():codetoanalyze.java.litho.MyComponent, 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()] -codetoanalyze/java/litho/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchBad(boolean):codetoanalyze.java.litho.MyComponent, 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()] +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()] +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()] 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(...)]