Permalink
Browse files

Handle __LSB attribute in typechecker

Summary:
Currently, the runtime handles the __LSB attribute, but the type-checker does not recognize it.

After this change, Hack will:
1) Recognize the __LSB attribute
2) Check that __LSB is only used on static properties
3) Check that __LSB properties are not overridden (even private ones!)
4) Check that __LSB properties are only accessed via static::$prop, ClassName::$prop, or $class_name::$prop (never via self:: or parent::)
5) Allow "this" inside of type hints for __LSB static properties
6) Recognize that a private __LSB property can be accessed using static:: (but only in the class that defined it)

Reviewed By: kmeht

Differential Revision: D10049830

fbshipit-source-id: 8500cb2050675625f32728cda78b4109e10ab652
  • Loading branch information...
pdox authored and hhvm-bot committed Oct 2, 2018
1 parent 8ad33f6 commit 94e7324ec591f075dd8a7843d516d2fdbe76d76e
Showing with 387 additions and 16 deletions.
  1. +5 −0 hphp/hack/src/decl/decl.ml
  2. +2 −0 hphp/hack/src/decl/decl_class.ml
  3. +1 −0 hphp/hack/src/decl/decl_defs.ml
  4. +1 −0 hphp/hack/src/decl/decl_inherit.ml
  5. +2 −0 hphp/hack/src/errors/error_codes.ml
  6. +9 −0 hphp/hack/src/errors/errors.ml
  7. +2 −0 hphp/hack/src/errors/errors_sig.ml
  8. +8 −0 hphp/hack/src/naming/attributes.ml
  9. +11 −2 hphp/hack/src/naming/naming.ml
  10. +2 −0 hphp/hack/src/naming/naming_special_names.ml
  11. +1 −1 hphp/hack/src/server/autocompleteService.ml
  12. +5 −0 hphp/hack/src/typing/pp_type.ml
  13. +1 −1 hphp/hack/src/typing/tast_env.mli
  14. +4 −4 hphp/hack/src/typing/typing.ml
  15. +3 −0 hphp/hack/src/typing/typing_defs.ml
  16. +22 −1 hphp/hack/src/typing/typing_extends.ml
  17. +34 −5 hphp/hack/src/typing/typing_visibility.ml
  18. +2 −2 hphp/hack/src/typing/typing_visibility.mli
  19. +2 −0 hphp/hack/test/errors/error_map.ml
  20. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_private.php
  21. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_private.php.exp
  22. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_protected.php
  23. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_protected.php.exp
  24. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_public.php
  25. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_public.php.exp
  26. +12 −0 hphp/hack/test/typecheck/lsb_disallow_override_subclass.php
  27. +9 −0 hphp/hack/test/typecheck/lsb_disallow_override_subclass.php.exp
  28. +16 −0 hphp/hack/test/typecheck/lsb_dynamic_access.php
  29. +1 −0 hphp/hack/test/typecheck/lsb_dynamic_access.php.exp
  30. +9 −0 hphp/hack/test/typecheck/lsb_nonstatic_disallowed.php
  31. +2 −0 hphp/hack/test/typecheck/lsb_nonstatic_disallowed.php.exp
  32. +11 −0 hphp/hack/test/typecheck/lsb_parent_disallowed.php
  33. +4 −0 hphp/hack/test/typecheck/lsb_parent_disallowed.php.exp
  34. +11 −0 hphp/hack/test/typecheck/lsb_private_disallow.php
  35. +4 −0 hphp/hack/test/typecheck/lsb_private_disallow.php.exp
  36. +11 −0 hphp/hack/test/typecheck/lsb_protected_disallow_direct.php
  37. +4 −0 hphp/hack/test/typecheck/lsb_protected_disallow_direct.php.exp
  38. +14 −0 hphp/hack/test/typecheck/lsb_protected_disallow_subclass.php
  39. +4 −0 hphp/hack/test/typecheck/lsb_protected_disallow_subclass.php.exp
  40. +15 −0 hphp/hack/test/typecheck/lsb_protected_visibility.php
  41. +1 −0 hphp/hack/test/typecheck/lsb_protected_visibility.php.exp
  42. +25 −0 hphp/hack/test/typecheck/lsb_public_visibility.php
  43. +1 −0 hphp/hack/test/typecheck/lsb_public_visibility.php.exp
  44. +9 −0 hphp/hack/test/typecheck/lsb_self_disallowed.php
  45. +4 −0 hphp/hack/test/typecheck/lsb_self_disallowed.php.exp
  46. +19 −0 hphp/hack/test/typecheck/lsb_this.php
  47. +1 −0 hphp/hack/test/typecheck/lsb_this.php.exp
  48. +28 −0 hphp/hack/test/typecheck/lsb_this_in_generic.php
  49. +1 −0 hphp/hack/test/typecheck/lsb_this_in_generic.php.exp
@@ -706,6 +706,7 @@ and build_constructor env class_ method_ =
elt_is_xhp_attr = false;
elt_const = false;
elt_lateinit = false;
elt_lsb = false;
elt_override = consist_override;
elt_memoizelsb = false;
elt_synthesized = false;
@@ -802,6 +803,7 @@ and class_var_decl env c acc cv =
elt_is_xhp_attr = cv.cv_is_xhp;
elt_const = const;
elt_lateinit = lateinit;
elt_lsb = false;
elt_synthesized = false;
elt_override = false;
elt_memoizelsb = false;
@@ -824,10 +826,12 @@ and static_class_var_decl env c acc cv =
let id = "$" ^ cv_name in
let vis = visibility (snd c.c_name) cv.cv_visibility in
let lateinit = Attrs.mem SN.UserAttributes.uaLateInit cv.cv_user_attributes in
let lsb = Attrs.mem SN.UserAttributes.uaLSB cv.cv_user_attributes in
let elt = {
elt_final = true;
elt_const = false; (* unsupported for static properties *)
elt_lateinit = lateinit;
elt_lsb = lsb;
elt_is_xhp_attr = cv.cv_is_xhp;
elt_override = false;
elt_memoizelsb = false;
@@ -988,6 +992,7 @@ and method_decl_acc ~is_static env c (acc, condition_types) m =
elt_is_xhp_attr = false;
elt_const = false;
elt_lateinit = false;
elt_lsb = false;
elt_abstract = ft.ft_abstract;
elt_override = check_override;
elt_memoizelsb = has_memoizelsb;
@@ -33,6 +33,7 @@ let element_to_class_elt ce_type {
elt_final = ce_final;
elt_synthesized = ce_synthesized;
elt_override = ce_override;
elt_lsb = ce_lsb;
elt_memoizelsb = ce_memoizelsb;
elt_abstract = _;
elt_is_xhp_attr = ce_is_xhp_attr;
@@ -48,6 +49,7 @@ let element_to_class_elt ce_type {
ce_const;
ce_lateinit;
ce_override;
ce_lsb;
ce_memoizelsb;
ce_synthesized;
ce_visibility;
@@ -110,6 +110,7 @@ and element = {
elt_is_xhp_attr : bool;
elt_const: bool;
elt_lateinit: bool;
elt_lsb: bool;
elt_origin : string;
elt_visibility : visibility;
@@ -228,6 +228,7 @@ let mark_as_synthesized inh =
let filter_privates class_type =
let is_not_private _ elt = match elt.elt_visibility with
| Vprivate _ when elt.elt_lsb -> true
| Vprivate _ -> false
| Vpublic | Vprotected _ -> true
in
@@ -141,6 +141,7 @@ module Naming = struct
| RxOfScopeAndExplicitRx
| UnsupportedFeature
| TraitInterfaceConstructorPromo
| NonstaticPropertyWithLSB
(* EXTEND HERE WITH NEW VALUES IF NEEDED *)
[@@ deriving enum, show { with_path = false } ]
let err_code = to_enum
@@ -514,6 +515,7 @@ module Typing = struct
| StringCast
| BadLateInitOverride
| EscapingMutableObject
| OverrideLSB
(* EXTEND HERE WITH NEW VALUES IF NEEDED *)
[@@ deriving enum, show { with_path = false } ]
let err_code = to_enum
@@ -932,6 +932,10 @@ let this_type_forbidden pos =
"The type \"this\" cannot be used as a constraint on a class' generic, \
or as the type of a static member variable"
let nonstatic_property_with_lsb pos =
add (Naming.err_code Naming.NonstaticPropertyWithLSB) pos
"__LSB attribute may only be used on static properties"
let lowercase_this pos type_ =
add (Naming.err_code Naming.LowercaseThis) pos (
"Invalid Hack type \""^type_^"\". Use \"this\" instead"
@@ -2904,6 +2908,11 @@ let override_memoizelsb ~parent ~child =
child, "__MemoizeLSB method may not be an override (temporary due to HHVM bug)";
parent, "This method is being overridden"]
let override_lsb ~member_name ~parent ~child =
add_list (Typing.err_code Typing.OverrideLSB) [
child, "Member " ^ member_name ^ " may not override __LSB member of parent";
parent, "This is being overridden"]
let should_be_override pos class_id id =
add (Typing.err_code Typing.ShouldBeOverride) pos
((Utils.strip_ns class_id)^"::"^id^"() is marked as override; \
@@ -102,6 +102,7 @@ module type S = sig
val this_no_argument : Pos.t -> unit
val this_hint_outside_class : Pos.t -> unit
val this_type_forbidden : Pos.t -> unit
val nonstatic_property_with_lsb : Pos.t -> unit
val lowercase_this : Pos.t -> string -> unit
val classname_param : Pos.t -> unit
val invalid_instanceof : Pos.t -> unit
@@ -277,6 +278,7 @@ module type S = sig
string -> Pos.t -> Pos.t -> string -> unit
val override_final : parent:Pos.t -> child:Pos.t -> unit
val override_memoizelsb : parent:Pos.t -> child:Pos.t -> unit
val override_lsb : member_name:string -> parent:Pos.t -> child:Pos.t -> unit
val should_be_override : Pos.t -> string -> string -> unit
val override_per_trait : Pos.t * string -> string -> Pos.t -> unit
val missing_assign : Pos.t -> unit
@@ -25,6 +25,14 @@ let find x xs =
let find2 x1 x2 xs =
List.find xs (fun { ua_name = (_, n); _ } -> x1 = n || x2 = n)
let mem_pos x xs =
let attr = find x xs in
match attr with
| Some { ua_name = (pos, _); _ } ->
Some pos
| None ->
None
(* TODO: generalize the arity check / argument check here to handle attributes
* in general, not just __Deprecated *)
let deprecated ~kind (_, name) attrs =
@@ -1507,13 +1507,15 @@ module Make (GetLocals : GetLocals) = struct
| ClassVars
{ cv_kinds = kl; cv_hint = h; cv_names = cvl; cv_user_attributes = ua; _ }
when List.mem kl Static ~equal:(=) ->
(* Static variables are shared for all classes in the hierarchy.
(* Non-LSB Static variables are shared for all classes in the hierarchy.
* This makes the 'this' type completely unsafe as a type for a
* static variable. See test/typecheck/this_tparam_static.php as
* an example of what can occur.
*)
let h = Option.map h (hint ~forbid_this:true env) in
let attrs = user_attributes env ua in
let lsb = Attributes.mem SN.UserAttributes.uaLSB attrs in
let forbid_this = not lsb in
let h = Option.map h (hint ~forbid_this env) in
let cvl = List.map cvl (fun cv ->
let cv = class_prop_ env cv in
let cv = fill_prop kl h cv in
@@ -1543,6 +1545,13 @@ module Make (GetLocals : GetLocals) = struct
let cvl = List.map cv_names (class_prop_ env) in
let cvl = List.map cvl (fill_prop cv_kinds h) in
let attrs = user_attributes env cv_user_attributes in
let lsb_pos = Attributes.mem_pos SN.UserAttributes.uaLSB attrs in
(* Non-static properties cannot have attribute __LSB *)
let () = (match lsb_pos with
| Some pos ->
Errors.nonstatic_property_with_lsb pos
| None -> ()
) in
(* if class is __Const, make all member fields __Const *)
let attrs = match const with
| Some c -> if not (Attributes.mem SN.UserAttributes.uaConst attrs)
@@ -132,6 +132,7 @@ module UserAttributes = struct
let uaOptionalDestruct = "__OptionalDestruct"
let uaOnlyRxIfImpl = "__OnlyRxIfImpl"
let uaProbabilisticModel = "__PPL"
let uaLSB = "__LSB"
(* DEPRECATED: remove after codemodding www *)
let uaOnlyRxIfRxFunc_do_not_use = "__OnlyRxIfRxFunc"
let uaAtMostRxAsFunc = "__AtMostRxAsFunc"
@@ -166,6 +167,7 @@ module UserAttributes = struct
uaOptionalDestruct;
uaOnlyRxIfImpl;
uaProbabilisticModel;
uaLSB;
uaOnlyRxIfRxFunc_do_not_use;
uaOnlyRxIfArgs_do_not_use;
uaSealed;
@@ -129,7 +129,7 @@ let autocomplete_new cid env =
let get_class_elt_types env class_ cid elts =
let elts = SMap.filter elts begin fun _ x ->
Tast_env.is_visible env x.ce_visibility cid class_
Tast_env.is_visible env (x.ce_visibility, x.ce_lsb) cid class_
end in
SMap.map elts (fun { ce_type = lazy ty; _ } -> ty)
@@ -597,6 +597,11 @@ and pp_class_elt : Format.formatter -> class_elt -> unit = fun fmt x ->
Format.fprintf fmt "@]";
Format.fprintf fmt ";@ ";
Format.fprintf fmt "@[%s =@ " "ce_lsb";
Format.fprintf fmt "%B" x.ce_lsb;
Format.fprintf fmt "@]";
Format.fprintf fmt ";@ ";
Format.fprintf fmt "@[%s =@ " "ce_memoizelsb";
Format.fprintf fmt "%B" x.ce_memoizelsb;
Format.fprintf fmt "@]";
@@ -94,7 +94,7 @@ val get_concrete_supertypes : env -> Tast.ty -> env * Tast.ty list
val is_visible :
env ->
Typing_defs.visibility ->
Typing_defs.visibility * bool ->
Nast.class_id_ option ->
Typing_defs.class_type ->
bool
@@ -4781,9 +4781,9 @@ and class_get_ ~is_method ~is_const ~ety_env ?(explicit_tparams=[])
| None ->
smember_not_found p ~is_const ~is_method class_ mid;
env, (Reason.Rnone, Typing_utils.terr env), None
| Some {ce_visibility = vis; ce_type = lazy (r, Tfun ft); _} ->
| Some {ce_visibility = vis; ce_lsb = lsb; ce_type = lazy (r, Tfun ft); _} ->
let p_vis = Reason.to_pos r in
TVis.check_class_access p env (p_vis, vis) cid class_;
TVis.check_class_access p env (p_vis, vis, lsb) cid class_;
let env, ft =
Phase.localize_ft ~use_pos:p ~ety_env ~explicit_tparams:explicit_tparams env ft in
let arity_pos = match ft.ft_params with
@@ -4796,9 +4796,9 @@ and class_get_ ~is_method ~is_const ~ety_env ?(explicit_tparams=[])
} in
env, (r, Tfun ft), None
| _ -> assert false)
| Some { ce_visibility = vis; ce_type = lazy method_; _ } ->
| Some { ce_visibility = vis; ce_lsb = lsb; ce_type = lazy method_; _ } ->
let p_vis = Reason.to_pos (fst method_) in
TVis.check_class_access p env (p_vis, vis) cid class_;
TVis.check_class_access p env (p_vis, vis, lsb) cid class_;
let env, method_ =
begin match method_ with
(* We special case Tfun here to allow passing in explicit tparams to localize_ft. *)
@@ -441,6 +441,9 @@ and class_elt = {
ce_final : bool;
ce_is_xhp_attr : bool;
ce_override : bool;
(* true if this static property has attribute __LSB *)
ce_lsb : bool;
(* true if this method has attribute __MemoizeLSB *)
ce_memoizelsb : bool;
(* true if this elt arose from require-extends or other mechanisms
of hack "synthesizing" methods that were not written by the
@@ -31,6 +31,10 @@ let is_private = function
| { ce_visibility = Vprivate _; _ } -> true
| _ -> false
let is_lsb = function
| { ce_lsb = true; _ } -> true
| _ -> false
(*****************************************************************************)
(* Given a map of members, check that the overriding is correct.
* Please note that 'members' has a very general meaning here.
@@ -167,6 +171,20 @@ let check_memoizelsb_method member_source parent_class_elt class_elt =
let pos = Reason.to_pos elt_pos in
Errors.override_memoizelsb parent_pos pos
let check_lsb_overrides member_source member_name parent_class_elt class_elt =
let is_sprop = match member_source with
| `FromSProp -> true
| _ -> false in
let parent_is_lsb = parent_class_elt.ce_lsb in
let is_override = parent_class_elt.ce_origin <> class_elt.ce_origin in
if is_sprop && parent_is_lsb && is_override then
(* __LSB attribute is being overridden *)
let lazy (parent_pos, _) = parent_class_elt.ce_type in
let lazy (elt_pos, _) = class_elt.ce_type in
let parent_pos = Reason.to_pos parent_pos in
let pos = Reason.to_pos elt_pos in
Errors.override_lsb member_name parent_pos pos
let check_lateinit parent_class_elt class_elt =
let is_override = parent_class_elt.ce_origin <> class_elt.ce_origin in
let lateinit_diff = parent_class_elt.ce_lateinit <> class_elt.ce_lateinit in
@@ -183,6 +201,8 @@ let check_override env member_name mem_source ?(ignore_fun_return = false)
(* We first verify that we aren't overriding a final method *)
check_final_method mem_source parent_class_elt class_elt;
check_memoizelsb_method mem_source parent_class_elt class_elt;
(* Verify that we are not overriding an __LSB property *)
check_lsb_overrides mem_source member_name parent_class_elt class_elt;
check_lateinit parent_class_elt class_elt;
let class_known, check_params = should_check_params parent_class class_ in
let check_vis = class_known || check_partially_known_method_visibility in
@@ -240,7 +260,7 @@ let check_const_override env
(* Privates are only visible in the parent, we don't need to check them *)
let filter_privates members =
SMap.fold begin fun name class_elt acc ->
if is_private class_elt
if is_private class_elt && not (is_lsb class_elt)
then acc
else SMap.add name class_elt acc
end members SMap.empty
@@ -316,6 +336,7 @@ let default_constructor_ce class_ =
ce_const = false;
ce_lateinit = false;
ce_override = false;
ce_lsb = false;
ce_memoizelsb = false;
ce_synthesized = true;
ce_visibility = Vpublic;
@@ -73,7 +73,36 @@ let is_visible_for_obj env vis =
| Vprotected x ->
is_protected_visible env x self_id
let is_visible_for_class env vis cid cty =
(* The only permitted way to access an LSB property is via
static::, ClassName::, or $class_name:: *)
let is_lsb_permitted cid =
match cid with
| CIself -> Some "__LSB properties cannot be accessed with self::"
| CIparent -> Some "__LSB properties cannot be accessed with parent::"
| _ -> None
(* LSB property accessibility is relative to the defining class *)
let is_lsb_accessible env vis =
let self_id = Env.get_self_id env in
match vis with
| Vpublic -> None
| (Vprivate _ | Vprotected _) when Env.is_outside_class env ->
Some "You cannot access this member"
| Vprivate x ->
if x = self_id then None
else Some "You cannot access this member"
| Vprotected x ->
is_protected_visible env x self_id
let is_lsb_visible_for_class env vis cid =
match is_lsb_permitted cid with
| Some x -> Some x
| None -> is_lsb_accessible env vis
let is_visible_for_class env (vis, lsb) cid cty =
if lsb then is_lsb_visible_for_class env vis cid
else
let self_id = Env.get_self_id env in
match vis with
| Vpublic -> None
@@ -88,9 +117,9 @@ let is_visible_for_class env vis cid cty =
(did you mean to use static:: or self::?)"
| _ -> is_protected_visible env x self_id)
let is_visible env vis cid class_ =
let is_visible env (vis, lsb) cid class_ =
let msg_opt = match cid with
| Some cid -> is_visible_for_class env vis cid class_
| Some cid -> is_visible_for_class env (vis, lsb) cid class_
| None -> is_visible_for_obj env vis
in
Option.is_none msg_opt
@@ -106,8 +135,8 @@ let check_obj_access p env (p_vis, vis) =
| Some msg ->
visibility_error p msg (p_vis, vis)
let check_class_access p env (p_vis, vis) cid class_ =
match is_visible_for_class env vis cid class_ with
let check_class_access p env (p_vis, vis, lsb) cid class_ =
match is_visible_for_class env (vis, lsb) cid class_ with
| None -> ()
| Some msg ->
visibility_error p msg (p_vis, vis)
@@ -10,14 +10,14 @@
open Typing_defs
val check_class_access:
Pos.t -> Typing_env.env -> (Pos.t * visibility) -> Nast.class_id_ ->
Pos.t -> Typing_env.env -> (Pos.t * visibility * bool) -> Nast.class_id_ ->
class_type -> unit
val check_obj_access:
Pos.t -> Typing_env.env -> (Pos.t * visibility) -> unit
val is_visible:
Typing_env.env -> visibility -> Nast.class_id_ option -> class_type -> bool
Typing_env.env -> (visibility * bool) -> Nast.class_id_ option -> class_type -> bool
val min_vis_opt:
(Pos.t * visibility) option -> (Pos.t * visibility) option ->
Oops, something went wrong.

0 comments on commit 94e7324

Please sign in to comment.