Skip to content

Commit 0c9f901

Browse files
Mistral Contrastinfacebook-github-bot
authored andcommitted
Warn against static property redeclaration
Summary: Static property redeclaration is a footgun because it causes a separate static property be generated. In that sense it is not a (re)declaration but just a separate declaration. ``` <?hh class C { public static vec<int> $p = vec[]; } class D extends C { public static vec<int> $p = vec[]; } <<__EntryPoint>> function main(): void { C::$p[] = 1; D::$p[] = 2; var_dump(C::$p); // vec[1] var_dump(D::$p); // vec[2] } ``` This diff warns against redeclaration. We'd like this to be an error eventually. Reviewed By: francesco-zappa-nardelli Differential Revision: D77366104 fbshipit-source-id: ad28a6c1da574af4afd5e268bf17922e396b6aaa
1 parent 2ddfd54 commit 0c9f901

19 files changed

+191
-3
lines changed

hphp/hack/src/errors/error_codes.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,7 @@ module Warning = struct
814814
| SafeAbstractConstAccess [@value 12018]
815815
| SwitchRedundancy [@value 12019]
816816
| StaticCallOnTrait [@value 12020]
817+
| StaticPropertyOverride [@value 12021]
817818
[@@deriving enum, ord, show { with_path = false }]
818819
end
819820

hphp/hack/src/oxidized/gen/error_codes.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// This source code is licensed under the MIT license found in the
44
// LICENSE file in the "hack" directory of this source tree.
55
//
6-
// @generated SignedSource<<d3b733ec198c93107656515825474fc5>>
6+
// @generated SignedSource<<b2809815fefe191e19d540522aeb0dd9>>
77
//
88
// To regenerate this file, run:
99
// hphp/hack/src/oxidized_regen.sh
@@ -652,6 +652,7 @@ pub enum Warning {
652652
SafeAbstractConstAccess = 12018,
653653
SwitchRedundancy = 12019,
654654
StaticCallOnTrait = 12020,
655+
StaticPropertyOverride = 12021,
655656
}
656657
impl TrivialDrop for Warning {}
657658
arena_deserializer::impl_deserialize_in_arena!(Warning);

hphp/hack/src/typing/typing_class.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,6 +1786,7 @@ let class_wellformedness_checks env c tc (parents : class_parents) =
17861786
- __Disposable attribute w.r.t. parents
17871787
- individual member checks:
17881788
- subtyping parent members
1789+
- not redeclaring class properties
17891790
- __Override attribute check
17901791
- enum inclusions
17911792
- abstract members in concrete class

hphp/hack/src/typing/typing_extends.ml

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,22 @@ let check_override_final_method env parent_class_elt class_elt on_error =
492492
Typing_error.(
493493
apply_reasons ~on_error @@ Secondary.Override_final { pos; parent_pos })
494494

495+
(** Checks that we're not overriding a concrete static property. *)
496+
let check_override_concrete_static_prop
497+
~parent_pos env member_name parent_class_elt class_elt =
498+
if
499+
(not (get_ce_abstract parent_class_elt))
500+
&& not (get_ce_const parent_class_elt)
501+
then
502+
let child_prop_pos = Lazy.force class_elt.ce_pos in
503+
Typing_warning_utils.add
504+
env
505+
Typing_warning.
506+
( parent_pos,
507+
Static_property_override,
508+
Static_property_override.{ prop_name = member_name; child_prop_pos }
509+
)
510+
495511
(** Checks that methods annotated with __DynamicallyCallable are only overridden with
496512
dynamically callable method. *)
497513
let check_dynamically_callable
@@ -981,6 +997,7 @@ let check_override
981997
~class_
982998
~parent_class
983999
~(parent_type : decl_ty)
1000+
~parent_pos
9841001
~(class_elt : class_elt)
9851002
~parent_class_elt
9861003
on_error =
@@ -994,6 +1011,18 @@ let check_override
9941011
on_error
9951012
in
9961013

1014+
begin
1015+
match member_kind with
1016+
| MemberKind.Static_property ->
1017+
check_override_concrete_static_prop
1018+
~parent_pos
1019+
env
1020+
member_name
1021+
parent_class_elt
1022+
class_elt
1023+
| _ -> ()
1024+
end;
1025+
9971026
if MemberKind.is_method member_kind then begin
9981027
(* We first verify that we aren't overriding a final method. We only check
9991028
* for final overrides on methods, not properties. Constructors have their
@@ -1548,6 +1577,7 @@ let check_class_against_parent_class_elt
15481577
~parent_class
15491578
~parent_class_elt
15501579
~parent_type
1580+
~parent_pos
15511581
~class_elt
15521582
(on_error (parent_pos, Cls.name parent_class)) )
15531583
| None ->
@@ -1736,7 +1766,8 @@ let default_constructor_ce class_ =
17361766
}
17371767

17381768
(* When an interface defines a constructor, we check that they are compatible *)
1739-
let check_constructors env (parent_class, parent_type) class_ psubst on_error =
1769+
let check_constructors
1770+
~parent_pos env (parent_class, parent_type) class_ psubst on_error =
17401771
let parent_is_interface = Ast_defs.is_c_interface (Cls.kind parent_class) in
17411772
let parent_is_consistent =
17421773
constructor_is_consistent (snd (Cls.construct parent_class))
@@ -1772,6 +1803,7 @@ let check_constructors env (parent_class, parent_type) class_ psubst on_error =
17721803
~class_
17731804
~parent_class
17741805
~parent_type
1806+
~parent_pos
17751807
~parent_class_elt:parent_cstr
17761808
~class_elt:cstr
17771809
on_error
@@ -2127,14 +2159,21 @@ let check_typeconst_override
21272159
* message pointing at the class being checked.
21282160
*)
21292161
let check_class_extends_parent_constructors
2162+
~parent_pos
21302163
env
21312164
(parent_class : (Pos.t * string) * decl_ty * decl_ty list * Cls.t)
21322165
class_
21332166
on_error =
21342167
let (_, parent_type, parent_tparaml, parent_class) = parent_class in
21352168
let psubst = Inst.make_subst (Cls.tparams parent_class) parent_tparaml in
21362169
let env =
2137-
check_constructors env (parent_class, parent_type) class_ psubst on_error
2170+
check_constructors
2171+
~parent_pos
2172+
env
2173+
(parent_class, parent_type)
2174+
class_
2175+
psubst
2176+
on_error
21382177
in
21392178
env
21402179

@@ -2890,6 +2929,7 @@ let check_implements_extends_uses
28902929
parents
28912930
~f:(fun env ((parent_name, _, _, _) as parent) ->
28922931
check_class_extends_parent_constructors
2932+
~parent_pos:(fst parent_name)
28932933
env
28942934
parent
28952935
class_

hphp/hack/src/typing/typing_warning.ml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,13 @@ module Static_call_on_trait = struct
161161
}
162162
end
163163

164+
module Static_property_override = struct
165+
type t = {
166+
prop_name: string;
167+
child_prop_pos: Pos_or_decl.t;
168+
}
169+
end
170+
164171
type (_, _) kind =
165172
| Sketchy_equality : (Sketchy_equality.t, warn) kind
166173
| Safe_abstract : (Safe_abstract.t, warn) kind
@@ -175,5 +182,6 @@ type (_, _) kind =
175182
| No_disjoint_union_check : (No_disjoint_union_check.t, warn) kind
176183
| Switch_redundancy : (Switch_redundancy.t, warn) kind
177184
| Static_call_on_trait : (Static_call_on_trait.t, warn) kind
185+
| Static_property_override : (Static_property_override.t, warn) kind
178186

179187
type ('x, 'a) t = Pos.t * ('x, 'a) kind * 'x

hphp/hack/src/typing/typing_warning_utils.ml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,28 @@ module Static_call_on_trait = struct
587587
let quickfixes _ = []
588588
end
589589

590+
module Static_property_override = struct
591+
open Typing_warning.Static_property_override
592+
593+
type t = Typing_warning.Static_property_override.t
594+
595+
let code = Codes.StaticPropertyOverride
596+
597+
let codes = [code]
598+
599+
let code _ = code
600+
601+
let claim { prop_name; _ } =
602+
Printf.sprintf
603+
"Static property is overridden: %s"
604+
(Markdown_lite.md_codify prop_name)
605+
606+
let reasons { child_prop_pos; _ } =
607+
[(child_prop_pos, "Overriding static property is here")]
608+
609+
let quickfixes _ = []
610+
end
611+
590612
let module_of (type a x) (kind : (x, a) Typing_warning.kind) :
591613
(module Warning with type t = x) =
592614
match kind with
@@ -603,6 +625,7 @@ let module_of (type a x) (kind : (x, a) Typing_warning.kind) :
603625
| Typing_warning.No_disjoint_union_check -> (module No_disjoint_union_check)
604626
| Typing_warning.Switch_redundancy -> (module Switch_redundancy)
605627
| Typing_warning.Static_call_on_trait -> (module Static_call_on_trait)
628+
| Typing_warning.Static_property_override -> (module Static_property_override)
606629

607630
let module_of_migrated
608631
(type x) (kind : (x, Typing_warning.migrated) Typing_warning.kind) :
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
WARN: File "03-using-a-trait-03.hack", line 20, characters 7-7:
22
Duplicated property `x` in `C` (defined in `T1` and `T2`): all instances will be aliased at runtime (Warn[12008])
3+
WARN: File "03-using-a-trait-03.hack", line 21, characters 7-8:
4+
Static property is overridden: `$x` (Warn[12021])
5+
File "03-using-a-trait-03.hack", line 13, characters 17-19:
6+
Overriding static property is here

hphp/hack/test/typecheck/const_attribute/static/redeclare_to_const.php.exp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
WARN: File "redeclare_to_const.php", line 5, characters 17-17:
2+
Static property is overridden: `$p` (Warn[12021])
3+
File "redeclare_to_const.php", line 5, characters 47-49:
4+
Overriding static property is here
15
ERROR: File "redeclare_to_const.php", line 5, characters 47-49:
26
This property is `__Const` (Typing[4204])
37
File "redeclare_to_const.php", line 4, characters 28-30:

hphp/hack/test/typecheck/inheritance/non_const_prop_override_static.php.exp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
WARN: File "non_const_prop_override_static.php", line 8, characters 17-17:
2+
Static property is overridden: `$p` (Warn[12021])
3+
File "non_const_prop_override_static.php", line 9, characters 17-19:
4+
Overriding static property is here
15
ERROR: File "non_const_prop_override_static.php", line 9, characters 17-19:
26
The property `$p` has the wrong type (Typing[4341])
37
File "non_const_prop_override_static.php", line 9, characters 17-19:

hphp/hack/test/typecheck/lsb_disallow_override_private.php.exp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
WARN: File "lsb_disallow_override_private.php", line 7, characters 17-17:
2+
Static property is overridden: `$x` (Warn[12021])
3+
File "lsb_disallow_override_private.php", line 8, characters 18-20:
4+
Overriding static property is here
15
ERROR: File "lsb_disallow_override_private.php", line 8, characters 18-20:
26
Member `$x` may not override `__LSB` member of parent (Typing[4284])
37
File "lsb_disallow_override_private.php", line 4, characters 28-30:

0 commit comments

Comments
 (0)