Cyclic base types for enums are not checked

Vincent Siles · facebook-github-bot · commit 6c95bdbe3b1c · 2022-08-31T04:13:37.000-07:00
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
diff --git a/hphp/hack/src/typing/tast_check/enum_check.ml b/hphp/hack/src/typing/tast_check/enum_check.ml
@@ -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
diff --git a/hphp/hack/src/typing/tast_check/tast_check.ml b/hphp/hack/src/typing/tast_check/tast_check.ml
@@ -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 =
diff --git a/hphp/hack/test/typecheck/enum_base_cycle.php b/hphp/hack/test/typecheck/enum_base_cycle.php
@@ -0,0 +1,11 @@
+<?hh
+
+enum E : E {}
+
+enum E0 : E1 {}
+
+enum E1 : E2 {}
+
+enum E2 : E0 {}
+
+/* enum XXX : self {} // not allowed */
diff --git a/hphp/hack/test/typecheck/enum_base_cycle.php.exp b/hphp/hack/test/typecheck/enum_base_cycle.php.exp
@@ -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])
diff --git a/hphp/hack/test/unit/server_tests.ml b/hphp/hack/test/unit/server_tests.ml
@@ -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. *)
