Skip to content

Commit

Permalink
Cyclic base types for enums are not checked
Browse files Browse the repository at this point in the history
Summary:
We currently only perform cycle detection on enum constants, but the
following code would make the runtime fatal

```
enum E : E {}
```

See T128241203 for the full report.
This diff implements this detection for enum (and enum class while we're
at it, even if the runtime would not fatal in that case).

Reviewed By: chenmela

Differential Revision: D39129381

fbshipit-source-id: 9b586e1f5b942d514ddd3a3bd1f07428616413b7
  • Loading branch information
Vincent Siles authored and facebook-github-bot committed Aug 31, 2022
1 parent 8201ae8 commit 6c95bdb
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 1 deletion.
64 changes: 64 additions & 0 deletions hphp/hack/src/typing/tast_check/enum_check.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
(*
* Copyright (c) 2021, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the "hack" directory of this source tree.
*
*)

open Hh_prelude
open Aast
open Typing_defs
module Env = Tast_env
module Cls = Decl_provider.Class

let get_name dty =
match get_node dty with
| Tapply ((_, name), []) -> Some name
| _ -> None

(* Check that the base type of an enum or (enum class) is not an alias
* to the enum being defined:
*
* enum Foo : Foo {}
* enum Bar0 : Bar1 {}
* enum Bar1 : Bar0 {}
*
* Such code would make HHVM fatal.
*
* Note that we have a similar check for enum/class constants themselves in
* Cyclic_class_constant but it doesn't take into account the empty enums.
*)
let find_cycle env class_name =
(* Note w.r.t. Cyclic_class_constant:
* Since `self` is not allowed (parsing error) in this position, we just
* keep track of the hints we see.
*)
let rec spot_target seen current =
let open Option in
let enum_def = Env.get_enum env current in
let enum_info = enum_def >>= Cls.enum_type in
let te_base = enum_info >>= fun info -> get_name info.te_base in
match te_base with
| None -> None
| Some base ->
if SSet.mem base seen then
Some seen
else
spot_target (SSet.add base seen) base
in
spot_target SSet.empty class_name

let handler =
object
inherit Tast_visitor.handler_base

method! at_class_ env c =
let (pos, c_name) = c.c_name in
match find_cycle env c_name with
| Some stack ->
Errors.add_typing_error
Typing_error.(primary @@ Primary.Cyclic_class_def { pos; stack })
| None -> ()
end
4 changes: 4 additions & 0 deletions hphp/hack/src/typing/tast_check/tast_check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ let visitor ctx =
Some Global_write_check.handler
else
None);
(if skip_hierarchy_checks then
None
else
Some Enum_check.handler);
]
in
let handlers =
Expand Down
11 changes: 11 additions & 0 deletions hphp/hack/test/typecheck/enum_base_cycle.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?hh

enum E : E {}

enum E0 : E1 {}

enum E1 : E2 {}

enum E2 : E0 {}

/* enum XXX : self {} // not allowed */
8 changes: 8 additions & 0 deletions hphp/hack/test/typecheck/enum_base_cycle.php.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
File "enum_base_cycle.php", line 3, characters 6-6:
Cyclic class definition : `E` (Typing[4013])
File "enum_base_cycle.php", line 5, characters 6-7:
Cyclic class definition : `E2` `E1` `E0` (Typing[4013])
File "enum_base_cycle.php", line 7, characters 6-7:
Cyclic class definition : `E2` `E1` `E0` (Typing[4013])
File "enum_base_cycle.php", line 9, characters 6-7:
Cyclic class definition : `E2` `E1` `E0` (Typing[4013])
2 changes: 1 addition & 1 deletion hphp/hack/test/unit/server_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ let test_process_file_deferring () =

true

let expected_decling_count = 65
let expected_decling_count = 66

(* This test verifies that the deferral/counting machinery works for
ProviderUtils.compute_tast_and_errors_unquarantined. *)
Expand Down

0 comments on commit 6c95bdb

Please sign in to comment.