diff --git a/infer/src/checkers/LithoDomain.ml b/infer/src/checkers/LithoDomain.ml index 77cfb9620..4d1580786 100644 --- a/infer/src/checkers/LithoDomain.ml +++ b/infer/src/checkers/LithoDomain.ml @@ -8,6 +8,12 @@ open! IStd module F = Format +let is_component_or_section_builder class_typ_name tenv = + PatternMatch.is_subtype_of_str tenv class_typ_name "com.facebook.litho.Component$Builder" + || PatternMatch.is_subtype_of_str tenv class_typ_name + "com.facebook.litho.sections.Section$Builder" + + module LocalAccessPath = struct type t = {access_path: AccessPath.t; parent: Procname.t} [@@deriving compare] @@ -381,9 +387,7 @@ module Mem = struct match ptr_typ with | Typ.{desc= Tptr (typ, _)} -> ( match Typ.name typ with - | Some typ_name - when PatternMatch.is_subtype_of_str tenv typ_name "com.facebook.litho.Component$Builder" - -> + | Some typ_name when is_component_or_section_builder typ_name tenv -> let formal_ae = LocalAccessPath.make_from_pvar pvar ptr_typ pname in let created_location = CreatedLocation.ByParameter formal_ae in { created= Created.add formal_ae (CreatedLocations.singleton created_location) created diff --git a/infer/src/checkers/LithoDomain.mli b/infer/src/checkers/LithoDomain.mli index 1d67ed72b..698de491b 100644 --- a/infer/src/checkers/LithoDomain.mli +++ b/infer/src/checkers/LithoDomain.mli @@ -7,6 +7,8 @@ open! IStd +val is_component_or_section_builder : Typ.Name.t -> Tenv.t -> bool + (** Access path + its parent procedure *) module LocalAccessPath : sig type t = private {access_path: AccessPath.t; parent: Procname.t} [@@deriving compare] diff --git a/infer/src/checkers/RequiredProps.ml b/infer/src/checkers/RequiredProps.ml index 0ab3cc5aa..976ba2ffe 100644 --- a/infer/src/checkers/RequiredProps.ml +++ b/infer/src/checkers/RequiredProps.ml @@ -117,41 +117,40 @@ let is_litho_function = function false -let is_component_builder procname tenv = +let is_builder procname tenv = match procname with | Procname.Java java_procname -> - PatternMatch.is_subtype_of_str tenv - (Procname.Java.get_class_type_name java_procname) - "com.facebook.litho.Component$Builder" + let class_name = Procname.Java.get_class_type_name java_procname in + Domain.is_component_or_section_builder class_name tenv | _ -> false -let is_component procname tenv = +let is_component_or_section procname tenv = match procname with | Procname.Java java_procname -> - PatternMatch.is_subtype_of_str tenv - (Procname.Java.get_class_type_name java_procname) - "com.facebook.litho.Component" + let class_name = Procname.Java.get_class_type_name java_procname in + PatternMatch.is_subtype_of_str tenv class_name "com.facebook.litho.Component" + || PatternMatch.is_subtype_of_str tenv class_name "com.facebook.litho.sections.Section" | _ -> false -let is_component_build_method procname tenv = +let is_build_method procname tenv = + match Procname.get_method procname with "build" -> is_builder procname tenv | _ -> false + + +let is_create_method procname tenv = match Procname.get_method procname with - | "build" -> - is_component_builder procname tenv + | "create" -> + is_component_or_section procname tenv | _ -> false -let is_component_create_method procname tenv = - match Procname.get_method procname with "create" -> is_component procname tenv | _ -> false - - let get_component_create_typ_opt procname tenv = match procname with - | Procname.Java java_pname when is_component_create_method procname tenv -> + | Procname.Java java_pname when is_create_method procname tenv -> Some (Procname.Java.get_class_type_name java_pname) | _ -> None @@ -164,8 +163,8 @@ let satisfies_heuristic ~callee_pname ~callee_summary_opt tenv = Option.value_map ~default:false callee_summary_opt ~f:(fun sum -> LithoDomain.Mem.contains_build sum ) in - is_component_build_method callee_pname tenv - || is_component_create_method callee_pname tenv + is_build_method callee_pname tenv + || is_create_method callee_pname tenv || match callee_pname with | Procname.Java java_callee_procname -> @@ -174,7 +173,7 @@ let satisfies_heuristic ~callee_pname ~callee_summary_opt tenv = not build_exists_in_callees -let should_report pname tenv = not (is_litho_function pname || is_component_build_method pname tenv) +let should_report pname tenv = not (is_litho_function pname || is_build_method pname tenv) let report {InterproceduralAnalysis.proc_desc; tenv; err_log} astate = let check_on_string_set parent_typename create_loc call_chain prop_set = @@ -227,7 +226,7 @@ module TransferFunctions = struct Domain.LocalAccessPath.make_from_access_expression receiver_ae caller_pname in if - (is_component_builder callee_pname tenv || is_component_create_method callee_pname tenv) + (is_builder callee_pname tenv || is_create_method callee_pname tenv) (* track callee in order to report respective errors *) && satisfies_heuristic ~callee_pname ~callee_summary_opt tenv then @@ -236,9 +235,9 @@ module TransferFunctions = struct | Some create_typ -> Domain.call_create return_access_path create_typ location astate | None -> - if is_component_build_method callee_pname tenv then + if is_build_method callee_pname tenv then Domain.call_build_method ~ret:return_access_path ~receiver astate - else if is_component_builder callee_pname tenv then + else if is_builder callee_pname tenv then let callee_prefix = Domain.MethodCallPrefix.make callee_pname location in Domain.call_builder ~ret:return_access_path ~receiver callee_prefix astate else astate diff --git a/infer/tests/codetoanalyze/java/litho-required-props/MySection.java b/infer/tests/codetoanalyze/java/litho-required-props/MySection.java new file mode 100644 index 000000000..a6865071d --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho-required-props/MySection.java @@ -0,0 +1,44 @@ +/* + * 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 codetoanalyze.java.litho; + +import com.facebook.litho.annotations.Prop; +import com.facebook.litho.sections.Section; + +public class MySection extends Section { + @Prop Object prop1; // implicitly non-optional + + @Prop(optional = true) + Object prop2; // explicitly optional + + public Builder create() { + return new Builder(); + } + + public static class Builder extends Section.Builder { + MySection mMySection; + + public Builder prop1(Object o) { + this.mMySection.prop1 = o; + return this; + } + + public Builder prop2(Object o) { + this.mMySection.prop2 = o; + return this; + } + + public MySection build() { + return mMySection; + } + + @Override + public Builder getThis() { + return this; + } + } +} diff --git a/infer/tests/codetoanalyze/java/litho-required-props/RequiredPropsSection.java b/infer/tests/codetoanalyze/java/litho-required-props/RequiredPropsSection.java new file mode 100644 index 000000000..e768187db --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho-required-props/RequiredPropsSection.java @@ -0,0 +1,29 @@ +/* + * 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 codetoanalyze.java.litho; + +import com.facebook.litho.sections.Section; + +public class RequiredPropsSection { + + public MySection mMySection; + + public Section buildWithAllOk() { + return mMySection.create().prop1(new Object()).prop2(new Object()).build(); + } + + // prop 2 is optional + public Section buildWithout2Ok() { + return mMySection.create().prop1(new Object()).build(); + } + + // prop 1 is required + public Section buildWithout1Bad() { + return mMySection.create().prop2(new Object()).build(); + } +} diff --git a/infer/tests/codetoanalyze/java/litho-required-props/Section.java b/infer/tests/codetoanalyze/java/litho-required-props/Section.java new file mode 100644 index 000000000..5e7251717 --- /dev/null +++ b/infer/tests/codetoanalyze/java/litho-required-props/Section.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.sections; + +public class Section { + + public abstract static class Builder> { + + public abstract Section build(); + + public abstract T getThis(); + + public T commonProp(Object prop) { + return getThis(); + } + } +} diff --git a/infer/tests/codetoanalyze/java/litho-required-props/issues.exp b/infer/tests/codetoanalyze/java/litho-required-props/issues.exp index 1fdac41d0..a98236dc6 100644 --- a/infer/tests/codetoanalyze/java/litho-required-props/issues.exp +++ b/infer/tests/codetoanalyze/java/litho-required-props/issues.exp @@ -35,3 +35,4 @@ codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.l codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchBad(boolean):com.facebook.litho.Component, 1, 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 codetoanalyze.java.litho.MyComponent.create(...),calls prop2(...),calls prop3(...)] codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.setRequiredOnOneBranchEffectfulBad(boolean):com.facebook.litho.Component, 1, 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 codetoanalyze.java.litho.MyComponent.create(...),calls prop2(...),calls prop3(...)] codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.twoBuildersBad():void, -10, 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 codetoanalyze.java.litho.MyComponent.create(...),calls prop3(...)] +codetoanalyze/java/litho-required-props/RequiredPropsSection.java, codetoanalyze.java.litho.RequiredPropsSection.buildWithout1Bad():com.facebook.litho.sections.Section, 1, MISSING_REQUIRED_PROP, no_bucket, ERROR, [**@Prop prop1** is required for component codetoanalyze.java.litho.MySection, but is not set before the call to build(),calls codetoanalyze.java.litho.MySection.create(...),calls prop2(...)]