[litho] Fix FN caused by special treatment of ResType suffixes

Summary: We only added suffix-stripped prefixes of props but this causes FPS if a component declares props that actually contain suffixes since we strip them when we are adding builder calls. The fix is to add both normal and stripped versions.

Reviewed By: skcho, Katalune

Differential Revision: D23375405

fbshipit-source-id: e99cb4dd8
master
Ezgi Çiçek 4 years ago committed by Facebook GitHub Bot
parent fdb1640e12
commit 7cb8145ef6

@ -37,13 +37,15 @@ module MethodCallPrefix = struct
{prefix: string; procname: Procname.t [@compare.ignore]; location: Location.t [@compare.ignore]}
[@@deriving compare]
let make procname location =
let make_with_prefixes procname location =
let method_name = Procname.get_method procname in
let prefix_opt =
String.Set.find_map suffixes ~f:(fun suffix -> String.chop_suffix method_name ~suffix)
in
let prefix = Option.value prefix_opt ~default:method_name in
{prefix; procname; location}
let default = [{prefix= method_name; procname; location}] in
Option.value_map prefix_opt ~default ~f:(fun prefix ->
(* We have to add the default as well as the stripped prefix since there could be a required prop which actually includes the suffix. *)
{prefix; procname; location} :: default )
let pp fmt {procname} = Procname.pp fmt procname

@ -26,7 +26,7 @@ val suffixes : String.Set.t
module MethodCallPrefix : sig
type t = private {prefix: string; procname: Procname.t; location: Location.t}
val make : Procname.t -> Location.t -> t
val make_with_prefixes : Procname.t -> Location.t -> t list
end
module Mem : sig

@ -240,8 +240,11 @@ module TransferFunctions = struct
if is_build_method callee_pname tenv then
Domain.call_build_method ~ret:return_access_path ~receiver astate
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
let callee_prefixes =
Domain.MethodCallPrefix.make_with_prefixes callee_pname location
in
List.fold ~init:astate callee_prefixes ~f:(fun acc callee_prefix ->
Domain.call_builder ~ret:return_access_path ~receiver callee_prefix acc )
else astate
else
(* treat it like a normal call *)

@ -17,6 +17,7 @@ public class RequiredProps {
public MyComponent mMyComponent;
public MyLithoComponent mMyLithoComponent;
public ResPropComponent mResPropComponent;
public ResPropDoubleComponent mResPropDoubleComponent;
public VarArgPropComponent mVarArgPropComponent;
public Component buildWithAllOk() {
@ -253,6 +254,18 @@ public class RequiredProps {
mResPropComponent.create().propPx(new Object()).build();
}
public void buildPropResWithPxDoubleOk() {
mResPropDoubleComponent.create().propPx(new Object()).propPx(0).build();
}
public void buildPropResWithPxDoubleAlsoOk() {
mResPropDoubleComponent.create().prop(new Object()).propPx(0).build();
}
public void buildPropResWithPxDoubleBad() {
mResPropDoubleComponent.create().prop(new Object()).build();
}
public void buildPropResWithSpOk() {
mResPropComponent.create().propSp(new Object()).build();
}

@ -0,0 +1,59 @@
/*
* 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.Component;
import com.facebook.litho.annotations.Prop;
import com.facebook.litho.annotations.ResType;
/**
* using @Prop(resType = ..) allows you to set the Prop with any of .propname, .propnameRes, or
* .propnameAttr
*/
public class ResPropDoubleComponent extends Component {
@Prop(resType = ResType.SOME)
Object prop; // implicitly non-optional with resType
@Prop
Integer
propPx; // note that setter for propPx(Integer) is not same as propPx(Object) corresponding to
// prop
public Builder create() {
return new Builder();
}
public static class Builder extends Component.Builder<Builder> {
ResPropDoubleComponent mResPropDoubleComponent;
public Builder prop(Object o) {
this.mResPropDoubleComponent.prop = o;
return this;
}
public Builder propPx(Integer o) {
this.mResPropDoubleComponent.propPx = o;
return this;
}
public Builder propPx(Object o) {
this.mResPropDoubleComponent.prop = o;
return this;
}
public ResPropDoubleComponent build() {
return mResPropDoubleComponent;
}
@Override
public Builder getThis() {
return this;
}
}
}

@ -8,6 +8,7 @@ codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.l
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropLithoMissingOneInLoopBad(int):void, 4, 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(). Either set the missing @Prop or make @Prop(optional = true).,calls com.facebook.litho.MyLithoComponent.create(...),calls commonProp(...),calls prop1(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropMissingInConditionalBad(boolean):void, 1, 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(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.MyComponent.create(...),calls prop1(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResMissingBad():void, 1, 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(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.ResPropComponent.create(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropResWithPxDoubleBad():void, 1, MISSING_REQUIRED_PROP, no_bucket, ERROR, [**@Prop propPx** is required for component codetoanalyze.java.litho.ResPropDoubleComponent, but is not set before the call to build(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.ResPropDoubleComponent.create(...),calls prop(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildPropVarArgMissingBad():void, 1, 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(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.VarArgPropComponent.create(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithColumnChildBad():void, 2, 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(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.MyComponent.create(...),calls prop1(...)]
codetoanalyze/java/litho-required-props/RequiredProps.java, codetoanalyze.java.litho.RequiredProps.buildWithout1Bad():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(). Either set the missing @Prop or make @Prop(optional = true).,calls codetoanalyze.java.litho.MyComponent.create(...),calls prop2(...),calls prop3(...)]

Loading…
Cancel
Save