From 2c6fb16636f75abd87b3a584428447767181aa07 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Tue, 23 Jan 2018 18:12:31 -0800 Subject: [PATCH] [litho] support optional Prop's Summary: Don't want to warn when `Prop(optional = true)`'s aren't provided to the `Builder`. Reviewed By: jeremydubreil Differential Revision: D6741579 fbshipit-source-id: b1ae3ed --- infer/src/checkers/Litho.ml | 20 ++++++++++++++++--- infer/src/checkers/annotations.mli | 2 ++ .../java/litho/RequiredProps.java | 12 +++++++---- .../tests/codetoanalyze/java/litho/issues.exp | 3 +-- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/infer/src/checkers/Litho.ml b/infer/src/checkers/Litho.ml index 336a26497..6041035d5 100644 --- a/infer/src/checkers/Litho.ml +++ b/infer/src/checkers/Litho.ml @@ -99,11 +99,24 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let get_required_props typename tenv = + let is_required_prop annotations = + List.exists + ~f:(fun ({Annot.class_name; parameters}, _) -> + String.is_suffix class_name ~suffix:Annotations.prop + && (* Don't count as required if it's @Prop(optional = true). Note: this is a hack. We + only translate boolean parameters at the moment, and we only translate the value of + the parameter (as a string, lol), not its name. In this case, the only boolean + parameter of @Prop is optional, and its default value is false. So it suffices to + do the "one parameter true" check *) + not (List.exists ~f:(fun annot_string -> String.equal annot_string "true") parameters) + ) + annotations + in match Tenv.lookup tenv typename with | Some {fields} -> List.filter_map - ~f:(fun (fieldname, _, annot) -> - if Annotations.ia_is_prop annot then Some (Typ.Fieldname.Java.get_field fieldname) + ~f:(fun (fieldname, _, annotation) -> + if is_required_prop annotation then Some (Typ.Fieldname.Java.get_field fieldname) else None ) fields | None -> @@ -139,7 +152,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let prop_set = List.fold prop_setter_calls ~f:(fun acc pname -> String.Set.add acc (Typ.Procname.get_method pname)) - ~init:String.Set.empty in + ~init:String.Set.empty + in List.iter ~f:(fun required_prop -> if not (String.Set.mem prop_set required_prop) then diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index c5addf393..b58a6e7c8 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -25,6 +25,8 @@ val performance_critical : string val present : string +val prop : string + val for_non_ui_thread : string val for_ui_thread : string diff --git a/infer/tests/codetoanalyze/java/litho/RequiredProps.java b/infer/tests/codetoanalyze/java/litho/RequiredProps.java index 557e2633f..50300d139 100644 --- a/infer/tests/codetoanalyze/java/litho/RequiredProps.java +++ b/infer/tests/codetoanalyze/java/litho/RequiredProps.java @@ -1,3 +1,5 @@ +package codetoanalyze.java.litho; + /* * Copyright (c) 2018 - present Facebook, Inc. * All rights reserved. @@ -80,8 +82,8 @@ public class RequiredProps { .build(); } - // need to parse optional boolean for this to work - public MyComponent FP_buildWithRequiredOk() { + // prop 2 is optional + public MyComponent buildWithout2Ok() { return mMyComponent .create() @@ -90,6 +92,7 @@ public class RequiredProps { .build(); } + // prop 1 is required public MyComponent buildWithout1Bad() { return mMyComponent @@ -99,12 +102,13 @@ public class RequiredProps { .build(); } - public MyComponent buildWithout2Bad() { + // prop3 is required + public MyComponent buildWithout3Bad() { return mMyComponent .create() .prop1(new Object()) - .prop3(new Object()) + .prop2(new Object()) .build(); } diff --git a/infer/tests/codetoanalyze/java/litho/issues.exp b/infer/tests/codetoanalyze/java/litho/issues.exp index 088889184..838331f16 100644 --- a/infer/tests/codetoanalyze/java/litho/issues.exp +++ b/infer/tests/codetoanalyze/java/litho/issues.exp @@ -1,8 +1,7 @@ codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.FP_buildSuffix(MyComponent$Builder), 1, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] -codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.FP_buildWithRequiredOk(), 6, MISSING_REQUIRED_PROP, [@Prop prop2 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.FP_setRequiredOnBothBranchesOk(boolean), 7, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.buildWithout1Bad(), 6, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] -codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.buildWithout2Bad(), 6, MISSING_REQUIRED_PROP, [@Prop prop2 is required, but not set before the call to build()] +codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.buildWithout3Bad(), 6, MISSING_REQUIRED_PROP, [@Prop prop3 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.setProp3InCalleeButForgetProp1Bad(), 4, MISSING_REQUIRED_PROP, [@Prop prop1 is required, but not set before the call to build()] codetoanalyze/java/litho/RequiredProps.java, MyComponent RequiredProps.setRequiredOnOneBranchBad(boolean), 5, MISSING_REQUIRED_PROP, [@Prop prop1 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(...)]