Look through aliases when checking typedefs

Summary:
Recursion checks are broken on newtypes because we don't model how the runtime resolves alias definitions, and in localization we conflate interpretation of values typed as aliases with alias resolution for well-formedness checks.

As a result, it is trivial to write definitions that pass `hh` but fatal the runtime, and we fail to properly distinguish recursive and non-recursive case types for gating.

This change attempts to bring us closer to what happens at runtime when the backing type structure for a typedef is resolved. When checking a typedef value, we fully expand all aliases. If a case type is reached and we started from an alias, we stop and treat it as opaque. If we started from a case type, we expand case types until we reach the root type. For some intuition, consider the type_structure models of the following two types
```
hphpd> case type A = shape('a' => ?B);
hphpd> case type B = shape('b' => ?A);
hphpd> =type_structure('A')
Dict
(
  [kind] => 31
  [union_types] => Vec
    (
      [0] => Dict
        (
          [kind] => 14
          [fields] => Dict
            (
              [a] => Dict
                (
                  [kind] => 31
                  [union_types] => Vec
                    (
                      [0] => Dict
                        (
                          [kind] => 14
                          [fields] => Dict
                            (
                              [b] => Dict
                                (
                                  [kind] => 32
                                  [case_type] => "A"
                                )
                            )
                        )
                    )
                  [case_type] => "B"
                  [nullable] => true
                )
            )
        )
    )
  [case_type] => "A"
)
hphpd> =type_structure('B')
Dict
(
  [kind] => 31
  [union_types] => Vec
    (
      [0] => Dict
        (
          [kind] => 14
          [fields] => Dict
            (
              [b] => Dict
                (
                  [kind] => 31
                  [union_types] => Vec
                    (
                      [0] => Dict
                        (
                          [kind] => 14
                          [fields] => Dict
                            (
                              [a] => Dict
                                (
                                  [kind] => 32
                                  [case_type] => "B"
                                )
                            )
                        )
                    )
                  [case_type] => "A"
                  [nullable] => true
                )
            )
        )
    )
  [case_type] => "B"
)
hphpd>
```
Lift out the alias constraint checking so that cycle detection occurs regardless of whether we have resolved to a `Rhs` value. This fixes a case where you can launder well-formedness errors through newtype.

Reviewed By: viratyosin

Differential Revision: D91863233

fbshipit-source-id: 00ab9e78bc8f80d7a454dc558a421fa349b4aa55

Author: vassilmladenov

hphp/hack/src/oxidized/copy_types.txt

@@ -109,7 +109,6 @@ reason_collector::LoggedType
 type_counter::Category
 type_counter::LoggedType
 typing_defs::ClassConstKind
-typing_defs::VisibilityBehavior
 typing_defs_core::ConsistentKind
 typing_defs_core::DestructureKind
 typing_defs_core::Enforcement

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

@@ -3,12 +3,11 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<9b313221c57bf31108da347472a57cab>>
+// @generated SignedSource<<e269113b4bb46a8bb1439e75a1b4a68b>>
 //
 // To regenerate this file, run:
 //   buck run @fbcode//mode/dev-nosan-lg fbcode//hphp/hack/src:oxidized_regen
 
-use arena_trait::TrivialDrop;
 use eq_modulo_pos::EqModuloPos;
 use no_pos_hash::NoPosHash;
 use ocamlrep::FromOcamlRep;
@@ -602,13 +601,11 @@ pub enum DeserializationError {
 
 #[derive(
     Clone,
-    Copy,
     Debug,
     Deserialize,
     Eq,
     EqModuloPos,
     FromOcamlRep,
-    FromOcamlRepIn,
     Hash,
     NoPosHash,
     Ord,
@@ -618,14 +615,14 @@ pub enum DeserializationError {
     ToOcamlRep
 )]
 #[rust_to_ocaml(attr = "deriving show { with_path = false }")]
-#[repr(u8)]
+#[repr(C, u8)]
 pub enum VisibilityBehavior {
     #[rust_to_ocaml(name = "Always_expand_newtype")]
     AlwaysExpandNewtype,
     #[rust_to_ocaml(name = "Expand_visible_newtype_only")]
     ExpandVisibleNewtypeOnly,
     #[rust_to_ocaml(name = "Never_expand_newtype")]
     NeverExpandNewtype,
+    #[rust_to_ocaml(name = "Resolve_type_structure")]
+    ResolveTypeStructure(Option<String>),
 }
-impl TrivialDrop for VisibilityBehavior {}
-arena_deserializer::impl_deserialize_in_arena!(VisibilityBehavior);

hphp/hack/src/typing/env/typing_env.ml

@@ -818,6 +818,8 @@ module M = struct
     | CaseType _ ->
       (match visibility_behavior with
       | Always_expand_newtype -> true
+      | Resolve_type_structure None -> false
+      | Resolve_type_structure (Some t) -> String.(t <> name)
       | Expand_visible_newtype_only
       | Never_expand_newtype ->
         false)
@@ -826,7 +828,9 @@ module M = struct
       (match td_vis with
       | Aast.Opaque ->
         (match visibility_behavior with
-        | Always_expand_newtype -> true
+        | Resolve_type_structure _
+        | Always_expand_newtype ->
+          true
         | Never_expand_newtype -> false
         | Expand_visible_newtype_only ->
           let td_path = Naming_provider.get_typedef_path (get_ctx env) name in
@@ -835,7 +839,9 @@ module M = struct
           | None -> (* Not the right place to raise an error *) false))
       | Aast.OpaqueModule ->
         (match visibility_behavior with
-        | Always_expand_newtype -> true
+        | Resolve_type_structure _
+        | Always_expand_newtype ->
+          true
         | Never_expand_newtype -> false
         | Expand_visible_newtype_only ->
           Option.equal

hphp/hack/src/typing/typing_defs.ml

@@ -292,6 +292,7 @@ type visibility_behavior =
   | Always_expand_newtype
   | Expand_visible_newtype_only
   | Never_expand_newtype
+  | Resolve_type_structure of string option
 [@@deriving show { with_path = false }]
 
 let is_default_visibility_behaviour = function

hphp/hack/src/typing/typing_defs.mli

@@ -212,6 +212,11 @@ type visibility_behavior =
   | Always_expand_newtype
   | Expand_visible_newtype_only
   | Never_expand_newtype
+  (* Approximate HHVM's resolution of type structures, where types and newtypes are
+   * fully expanded, but recursive references to the current type become opaque leaf
+   * nodes i.e. TypeStructureKind::T_recursiveUnion. Where this differs is, for regular
+   * type alias roots, we do not expand case types. *)
+  | Resolve_type_structure of string option
 [@@deriving show]
 
 val is_default_visibility_behaviour : visibility_behavior -> bool

hphp/hack/src/typing/typing_phase.ml

@@ -831,12 +831,7 @@ and localize_class_instantiation
     else
       let tparams = Cls.tparams class_info in
       let ((env, err, cycles), tyl) =
-        localize_targs_constrain_wildcards
-          ~ety_env:
-            { ety_env with visibility_behavior = default_visibility_behaviour }
-          env
-          tyargs
-          tparams
+        localize_targs_constrain_wildcards ~ety_env env tyargs tparams
       in
       (* Hide the class type if its internal and outside of the module *)
       if
@@ -1498,6 +1493,26 @@ let localize_hint_no_subst_report_cycles_ env ~ignore_errors ?report_cycle h =
     ?report_cycle
     h
 
+let localize_typedef_structure env ~ignore_errors ~report_cycle name h =
+  let (pos, _) = h in
+  let ty = Decl_hint.hint env.decl_env h in
+  let ety_env =
+    {
+      empty_expand_env with
+      visibility_behavior = Resolve_type_structure name;
+      type_expansions =
+        Type_expansions.empty_w_cycle_report ~report_cycle:(Some report_cycle);
+      on_error =
+        (if ignore_errors then
+          None
+        else
+          Some (Typing_error.Reasons_callback.invalid_type_hint pos));
+      wildcard_action = Wildcard_illegal;
+      ish_weakening = false;
+    }
+  in
+  localize env ty ~ety_env
+
 let localize_hint_no_subst env ~ignore_errors h =
   let ((env, err, _cycles), ty) =
     localize_hint_no_subst_report_cycles_ env ~ignore_errors h

hphp/hack/src/typing/typing_phase.mli

@@ -110,6 +110,14 @@ val localize_hint_no_subst_report_cycles :
   Aast.hint ->
   (env * Typing_error.t option * Type_expansions.cycle_reporter list) * locl_ty
 
+val localize_typedef_structure :
+  env ->
+  ignore_errors:bool ->
+  report_cycle:Pos.t * Type_expansions.Expandable.t ->
+  string option ->
+  Aast.hint ->
+  (env * Typing_error.t option * Type_expansions.cycle_reporter list) * locl_ty
+
 val localize_hint_for_refinement :
   env -> Aast.hint -> (env * Typing_error.t option) * locl_ty
 

hphp/hack/src/typing/typing_tdef.ml

@@ -20,10 +20,11 @@ module MakeType = Typing_make_type
 
 type expansion =
   | Rhs of decl_ty
-  | Opaque of (* constraint *) decl_ty
+  | Opaque
 
 type expand_result = {
   tparams: decl_tparam list;
+  cstr: decl_ty;
   expansion: expansion;
 }
 
@@ -54,6 +55,11 @@ let expand_typedef_decl
     force_expand
     || Env.should_expand_type_alias ~visibility_behavior env ~name:x td
   in
+  let cstr =
+    match td_as_constraint with
+    | None -> MakeType.mixed (Reason.implicit_upper_bound (td_pos, "?nonnull"))
+    | Some cstr -> cstr
+  in
   let expansion =
     if should_expand then
       let td_type =
@@ -65,12 +71,8 @@ let expand_typedef_decl
       Rhs td_type
     else
       Opaque
-        (match td_as_constraint with
-        | None ->
-          MakeType.mixed (Reason.implicit_upper_bound (td_pos, "?nonnull"))
-        | Some cstr -> cstr)
   in
-  { tparams = td_tparams; expansion }
+  { tparams = td_tparams; cstr; expansion }
 
 (** [expand_typedef_ ~force_expand ety_env env r name ty_args] looks up the type
   alias [name] in the decls and returns the value of the type alias as a locl_ty, with
@@ -111,6 +113,7 @@ let expand_typedef_ ~force_expand ety_env env r (x : string) argl :
          able to make proper conclusions about the cycle. *)
       match ety_env.visibility_behavior with
       | Always_expand_newtype -> mk (r, Tnewtype (x, argl, mixed))
+      | Resolve_type_structure _
       | Never_expand_newtype
       | Expand_visible_newtype_only ->
         mixed
@@ -119,7 +122,7 @@ let expand_typedef_ ~force_expand ety_env env r (x : string) argl :
         { env; ty_err_opt = None; cycles = [cycle]; ty; bound = mixed },
       ety_env )
   | Ok ety_env ->
-    let { tparams; expansion } =
+    let { tparams; cstr; expansion } =
       expand_typedef_decl
         ~force_expand
         ~visibility_behavior:ety_env.visibility_behavior
@@ -144,22 +147,25 @@ let expand_typedef_ ~force_expand ety_env env r (x : string) argl :
     else
       let substs = Subst.make_locl tparams argl in
       let ety_env = { ety_env with substs } in
+      let ((env, cstr_err, cstr_cycles), cstr) =
+        (* Special case for supportdyn<T> defined with "as T" in order to
+         * avoid supportdynamic.hhi appearing in reason *)
+        if String.equal x SN.Classes.cSupportDyn then
+          ((env, None, []), List.hd_exn argl)
+        else
+          Phase.localize_rec ~ety_env env cstr
+      in
       let ((env, err, cycles), expanded_ty, bound) =
         match expansion with
         | Rhs ty ->
-          let ((env, err, cycles), ty) = Phase.localize_rec ~ety_env env ty in
-          ((env, err, cycles), ty, ty)
-        | Opaque cstr_ty ->
-          let ((env, err, cycles), cstr_ty) =
-            (* Special case for supportdyn<T> defined with "as T" in order to
-             * avoid supportdynamic.hhi appearing in reason *)
-            if String.equal x SN.Classes.cSupportDyn then
-              ((env, None, []), List.hd_exn argl)
-            else
-              Phase.localize_rec ~ety_env env cstr_ty
+          let ((env, rhs_err, rhs_cycles), ty) =
+            Phase.localize_rec ~ety_env env ty
           in
-          let ty = mk (r, Tnewtype (x, argl, cstr_ty)) in
-          ((env, err, cycles), ty, cstr_ty)
+          let err = Option.merge cstr_err rhs_err ~f:Typing_error.both in
+          ((env, err, rhs_cycles @ cstr_cycles), ty, ty)
+        | Opaque ->
+          let ty = mk (r, Tnewtype (x, argl, cstr)) in
+          ((env, cstr_err, cstr_cycles), ty, cstr)
       in
       ( { env; ty_err_opt = err; cycles; ty = with_reason expanded_ty r; bound },
         ety_env )

hphp/hack/src/typing/typing_typedef.ml

@@ -208,13 +208,19 @@ let casetype_def env typedef =
     env
 
 let check_cycles env (t_pos, t_name) t_assignment hints =
+  let case_type_root =
+    match t_assignment with
+    | SimpleTypeDef _ -> None
+    | CaseType _ -> Some t_name
+  in
   let (env, cycles) =
     List.fold_left hints ~init:(env, []) ~f:(fun (env, cycles) hint ->
         let ((env, _ty_err_opt, new_cycles), _ty) =
-          Phase.localize_hint_no_subst_report_cycles
+          Phase.localize_typedef_structure
             env
             ~ignore_errors:true
             ~report_cycle:(t_pos, Type_expansions.Expandable.Type_alias t_name)
+            case_type_root
             hint
         in
         (env, new_cycles @ cycles))

hphp/hack/test/deps/invoking_funs.php.exp

@@ -4,3 +4,4 @@ Type HH\Contexts\Unsafe\globals -> Fun B
 Type HH\Contexts\Unsafe\read_globals -> Fun A
 Type HH\Contexts\globals -> Fun B
 Type HH\Contexts\read_globals -> Fun A, Fun B
+Type HH\supportdyn -> Fun B