Skip to content

Commit

Permalink
fix fanout bug when making an attribute required
Browse files Browse the repository at this point in the history
Reviewed By: bobrenjc93

Differential Revision: D37379772

fbshipit-source-id: 423b9ba68666d8867b713787a4636e5ae203c8e5
  • Loading branch information
CatherineGasnier authored and facebook-github-bot committed Jun 23, 2022
1 parent 543ec0a commit 59f6ba8
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 8 deletions.
44 changes: 36 additions & 8 deletions hphp/hack/src/decl/shallow_class_diff.ml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ module type Member_S = sig
(** Whether adding this member implies that the constructor should be considered modified.
This is the case for example for required XHP attributes. *)
val constructor_is_modified_when_added : t -> bool

(** Whether modifying this member implies that the constructor should be considered modified.
This is the case for example for required XHP attributes. *)
val constructor_is_modified_when_modified : old:t -> new_:t -> bool
end

(** Returns the diff of two member lists, plus a diff on the constructor if the member changes
Expand All @@ -67,18 +71,34 @@ let diff_members
| (Some _, None) ->
(SMap.add diff ~key:name ~data:Removed, constructor_change)
| (None, Some m) ->
( SMap.add diff ~key:name ~data:Added,
let constructor_change =
max_constructor_change
constructor_change
(if Member.constructor_is_modified_when_added m then
Some Modified
else
None) )
None)
in
(SMap.add diff ~key:name ~data:Added, constructor_change)
| (Some old_member, Some new_member) ->
( Member.diff old_member new_member
let member_changes =
Member.diff old_member new_member
|> Option.value_map ~default:diff ~f:(fun ch ->
SMap.add diff ~key:name ~data:ch),
constructor_change ))
SMap.add diff ~key:name ~data:ch)
in
let constructor_change =
max_constructor_change
constructor_change
(if
Member.constructor_is_modified_when_modified
~old:old_member
~new_:new_member
then
Some Modified
else
None)
in
(member_changes, constructor_change))

module ClassConst : Member_S with type t = shallow_class_const = struct
type t = shallow_class_const
Expand All @@ -98,6 +118,8 @@ module ClassConst : Member_S with type t = shallow_class_const = struct
let is_private _ = false

let constructor_is_modified_when_added _ = false

let constructor_is_modified_when_modified ~old:_ ~new_:_ = false
end

module TypeConst : Member_S with type t = shallow_typeconst = struct
Expand All @@ -119,6 +141,8 @@ module TypeConst : Member_S with type t = shallow_typeconst = struct
let is_private _ = false

let constructor_is_modified_when_added _ = false

let constructor_is_modified_when_modified ~old:_ ~new_:_ = false
end

module Prop : Member_S with type t = shallow_prop = struct
Expand All @@ -141,9 +165,11 @@ module Prop : Member_S with type t = shallow_prop = struct
Aast_defs.equal_visibility p.sp_visibility Aast_defs.Private

let constructor_is_modified_when_added p =
match p.sp_xhp_attr with
| None -> false
| Some attr -> Xhp_attribute.is_required attr
Xhp_attribute.opt_is_required p.sp_xhp_attr

let constructor_is_modified_when_modified ~old ~new_ =
Xhp_attribute.opt_is_required new_.sp_xhp_attr
&& (not @@ Xhp_attribute.opt_is_required old.sp_xhp_attr)
end

module Method : Member_S with type t = shallow_method = struct
Expand All @@ -166,6 +192,8 @@ module Method : Member_S with type t = shallow_method = struct
Aast_defs.equal_visibility m.sm_visibility Aast_defs.Private

let constructor_is_modified_when_added _ = false

let constructor_is_modified_when_modified ~old:_ ~new_:_ = false
end

let diff_constructor old_cls new_cls old_cstr new_cstr : member_change option =
Expand Down
5 changes: 5 additions & 0 deletions hphp/hack/src/typing/xhp_attribute.ml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ let is_required attr =
| Some LateInit ->
false
| Some Required -> true

let opt_is_required (attr : t option) : bool =
match attr with
| None -> false
| Some attr -> is_required attr
2 changes: 2 additions & 0 deletions hphp/hack/src/typing/xhp_attribute.mli
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ val init : t
val map_tag : t -> f:(tag option -> tag option) -> t

val is_required : t -> bool

val opt_is_required : t option -> bool
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ No errors!
No errors!
=== fanout ===
0x52d77445a3eff51b
Fun xhp_simple_attribute

0 comments on commit 59f6ba8

Please sign in to comment.