Preserve Opaque Types when evaluating ReactDRO destructor

Summary:
The ReactDRO destructor adds a flag to all object types that make that object deeply-read-only for local analysis. This is unique among destructors in that it isn't really *transforming* the type as it is *marking* the type.

Typically, we handle OpaqueTs in destructors by using their underlying type or otherwise falling back on behavior in flow_js.ml's larger pattern match when the underlying type is not exposable. For destructors that transform this makes sense: there's no sense in keeping around an opaque type because the opaque type is undergoing a transformation that would make it fundamentally different than its original definition. There's no sense in making that type opaque.

But for ReactDRO, we don't actually want to lose the opaque type, we just want to mark the underlying_t and super_t with the ReactDRO marker. It would make sense to preserve the opacity, and a DRO OpaqueT should be compatible with its non-DRO equivalent, since ReactDRO is just for local analysis.

This diff solves the issue by preserving opacity when evaluating ReactDRO on an opaque type. We have to go one extra step of forcing the evaluation of ReactDRO applied to the underlying_t and super_t because our refinement machinery expects to be able to inspect the underlying_t and super_t of opaque types.

Without the change to eagerly evaluate we get ~50 errors matching the conditional.js test case added.

Changelog: [fix] Fixed a behavior where component syntax would transform opaque types into their underlying types.

Reviewed By: SamChou19815

Differential Revision: D55575945

fbshipit-source-id: 64a6ca0febf717f6aa4e047e20a8a052f520abf1

src/typing/flow_js.ml

@@ -7477,6 +7477,19 @@ struct
         let void = VoidT.make reason in
         destruct_union ?f reason [t; void] upper
       in
+      let destruct_and_preserve_opaque_t r ({ underlying_t; super_t; _ } as opaquetype) =
+        let eval_t t =
+          let tvar = Tvar.mk_no_wrap cx reason in
+          (* We have to eagerly evaluate these destructors when possible because
+           * various other systems, like type_filter, expect OpaqueT underlying_t and
+           * super_t to be inspectable *)
+          eagerly_eval_destructor_if_resolved cx ~trace use_op reason t d tvar
+        in
+        let underlying_t = Base.Option.map ~f:eval_t underlying_t in
+        let super_t = Base.Option.map ~f:eval_t super_t in
+        let opaque_t = OpaqueT (r, { opaquetype with underlying_t; super_t }) in
+        rec_flow_t cx trace ~use_op (opaque_t, OpenT tout)
+      in
       let should_destruct_union () =
         match d with
         | ConditionalType { distributive_tparam_name; _ } -> Option.is_some distributive_tparam_name
@@ -7490,9 +7503,11 @@ struct
           | _ -> true)
         | _ -> true
       in
-      (match t with
-      | GenericT
-          { bound = OpaqueT (_, { underlying_t = Some t; _ }); reason = r; id; name; no_infer }
+      (match (t, d) with
+      | ( GenericT
+            { bound = OpaqueT (_, { underlying_t = Some t; _ }); reason = r; id; name; no_infer },
+          _
+        )
         when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
         eval_destructor
           cx
@@ -7502,21 +7517,25 @@ struct
           (GenericT { bound = t; reason = r; id; name; no_infer })
           d
           tout
-      | OpaqueT (r, { underlying_t = Some t; _ })
+      | (OpaqueT (r, opaquetype), ReactDRO _) -> destruct_and_preserve_opaque_t r opaquetype
+      | (OpaqueT (r, { underlying_t = Some t; _ }), _)
         when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
         eval_destructor cx ~trace use_op reason t d tout
       (* Specialize TypeAppTs before evaluating them so that we can handle special
          cases. Like the union case below. mk_typeapp_instance will return an AnnotT
          which will be fully resolved using the AnnotT case above. *)
-      | GenericT
-          {
-            bound =
-              TypeAppT { reason = _; use_op = use_op_tapp; type_; targs; from_value; use_desc = _ };
-            reason = reason_tapp;
-            id;
-            name;
-            no_infer;
-          } ->
+      | ( GenericT
+            {
+              bound =
+                TypeAppT
+                  { reason = _; use_op = use_op_tapp; type_; targs; from_value; use_desc = _ };
+              reason = reason_tapp;
+              id;
+              name;
+              no_infer;
+            },
+          _
+        ) ->
         let destructor = TypeDestructorT (use_op, reason, d) in
         let t =
           mk_typeapp_instance_annot
@@ -7538,8 +7557,10 @@ struct
               destructor,
             UseT (use_op, OpenT tout)
           )
-      | TypeAppT
-          { reason = reason_tapp; use_op = use_op_tapp; type_; targs; from_value; use_desc = _ } ->
+      | ( TypeAppT
+            { reason = reason_tapp; use_op = use_op_tapp; type_; targs; from_value; use_desc = _ },
+          _
+        ) ->
         let destructor = TypeDestructorT (use_op, reason, d) in
         let t =
           mk_typeapp_instance_annot
@@ -7559,39 +7580,47 @@ struct
          Instead, we preserve the union by pushing down the destructor onto the
          branches of the unions.
       *)
-      | UnionT (r, rep) when should_destruct_union () ->
+      | (UnionT (r, rep), _) when should_destruct_union () ->
         destruct_union r (UnionRep.members rep) (UseT (unknown_use, OpenT tout))
-      | GenericT { reason; bound = UnionT (_, rep); id; name; no_infer }
+      | (GenericT { reason; bound = UnionT (_, rep); id; name; no_infer }, _)
         when should_destruct_union () ->
         destruct_union
           ~f:(fun bound -> GenericT { reason = reason_of_t bound; bound; id; name; no_infer })
           reason
           (UnionRep.members rep)
           (UseT (use_op, OpenT tout))
-      | MaybeT (r, t) when should_destruct_union () ->
+      | (MaybeT (r, t), _) when should_destruct_union () ->
         destruct_maybe r t (UseT (unknown_use, OpenT tout))
-      | GenericT { reason; bound = MaybeT (_, t); id; name; no_infer } when should_destruct_union ()
-        ->
+      | (GenericT { reason; bound = MaybeT (_, t); id; name; no_infer }, _)
+        when should_destruct_union () ->
         destruct_maybe
           ~f:(fun bound -> GenericT { reason = reason_of_t bound; bound; id; name; no_infer })
           reason
           t
           (UseT (use_op, OpenT tout))
-      | OptionalT { reason = r; type_ = t; use_desc = _ } when should_destruct_union () ->
+      | (OptionalT { reason = r; type_ = t; use_desc = _ }, _) when should_destruct_union () ->
         destruct_optional r t (UseT (unknown_use, OpenT tout))
-      | GenericT
-          { reason; bound = OptionalT { reason = _; type_ = t; use_desc = _ }; id; name; no_infer }
+      | ( GenericT
+            {
+              reason;
+              bound = OptionalT { reason = _; type_ = t; use_desc = _ };
+              id;
+              name;
+              no_infer;
+            },
+          _
+        )
         when should_destruct_union () ->
         destruct_optional
           ~f:(fun bound -> GenericT { reason = reason_of_t bound; bound; id; name; no_infer })
           reason
           t
           (UseT (use_op, OpenT tout))
-      | AnnotT (r, t, use_desc) ->
+      | (AnnotT (r, t, use_desc), _) ->
         let t = reposition_reason ~trace cx r ~use_desc t in
         let destructor = TypeDestructorT (use_op, reason, d) in
         rec_flow_t cx trace ~use_op:unknown_use (Cache.Eval.id cx t destructor, OpenT tout)
-      | GenericT { bound = AnnotT (_, t, use_desc); reason = r; name; id; no_infer } ->
+      | (GenericT { bound = AnnotT (_, t, use_desc); reason = r; name; id; no_infer }, _) ->
         let t = reposition_reason ~trace cx r ~use_desc t in
         let destructor = TypeDestructorT (use_op, reason, d) in
         rec_flow_t
@@ -7787,6 +7816,28 @@ struct
               )
           ))
 
+  and eagerly_eval_destructor_if_resolved cx ~trace use_op reason t d tvar =
+    eval_destructor cx ~trace use_op reason t d (reason, tvar);
+    let result = OpenT (reason, tvar) in
+    if
+      (not (Subst_name.Set.is_empty (Type_subst.free_var_finder cx t)))
+      || (not (Subst_name.Set.is_empty (Type_subst.free_var_finder_in_destructor cx d)))
+      || Tvar_resolver.has_unresolved_tvars cx t
+      || Tvar_resolver.has_unresolved_tvars_in_destructors cx d
+    then
+      result
+    else (
+      Tvar_resolver.resolve cx result;
+      let t = singleton_concrete_type_for_inspection cx reason result in
+      match t with
+      | OpenT (_, id) ->
+        let (_, constraints) = Context.find_constraints cx id in
+        (match constraints with
+        | FullyResolved t -> Context.force_fully_resolved_tvar cx t
+        | _ -> t)
+      | t -> t
+    )
+
   and mk_possibly_evaluated_destructor cx use_op reason t d id =
     let eval_t = EvalT (t, TypeDestructorT (use_op, reason, d), id) in
     if Subst_name.Set.is_empty (Type_subst.free_var_finder cx eval_t) then
@@ -10740,6 +10791,12 @@ struct
   and possible_concrete_types_for_inspection cx t =
     possible_concrete_types (fun ident -> ConcretizeForInspection ident) cx t
 
+  and singleton_concrete_type_for_inspection cx reason t =
+    match possible_concrete_types_for_inspection cx reason t with
+    | [] -> EmptyT.make reason
+    | [t] -> t
+    | t1 :: t2 :: ts -> UnionT (reason, UnionRep.make t1 t2 ts)
+
   and add_specialized_callee_method_action cx trace l = function
     | CallM { specialized_callee; _ }
     | ChainM { specialized_callee; _ } ->
@@ -10765,14 +10822,6 @@ module rec FlowJs : Flow_common.S = struct
 
   let react_get_config = React.get_config
 
-  (* Returns a list of concrete types after breaking up unions, maybe types, etc *)
-
-  let singleton_concrete_type_for_inspection cx reason t =
-    match possible_concrete_types_for_inspection cx reason t with
-    | [] -> EmptyT.make reason
-    | [t] -> t
-    | t1 :: t2 :: ts -> UnionT (reason, UnionRep.make t1 t2 ts)
-
   let possible_concrete_types_for_imports_exports =
     possible_concrete_types (fun ident -> ConcretizeForImportsExports ident)
 

src/typing/type_subst.ml

@@ -93,6 +93,10 @@ let free_var_finder cx ?(bound = Subst_name.Set.empty) t =
   let { free; _ } = visitor#type_ cx Polarity.Positive { free = Subst_name.Set.empty; bound } t in
   free
 
+let free_var_finder_in_destructor cx ?(bound = Subst_name.Set.empty) d =
+  let { free; _ } = visitor#destructor cx { free = Subst_name.Set.empty; bound } d in
+  free
+
 (** Substitute bound type variables with associated types in a type. **)
 
 let new_name name fvs =

src/typing/type_subst.mli

@@ -7,6 +7,9 @@
 
 val free_var_finder : Context.t -> ?bound:Subst_name.Set.t -> Type.t -> Subst_name.Set.t
 
+val free_var_finder_in_destructor :
+  Context.t -> ?bound:Subst_name.Set.t -> Type.destructor -> Subst_name.Set.t
+
 val new_name : Subst_name.t -> Subst_name.Set.t -> Subst_name.t
 
 val subst :

tests/opaque_types_and_component_syntax_destructors/.flowconfig

@@ -0,0 +1,4 @@
+[options]
+component_syntax=true
+no_flowlib=false
+all=true

tests/opaque_types_and_component_syntax_destructors/conditional.js

@@ -0,0 +1,7 @@
+component Foo() {
+  return null;
+}
+component Bar(x: React.Element<typeof Foo>) {
+  const y: number = x && 3; // OK
+  return null;
+}

tests/opaque_types_and_component_syntax_destructors/non-local.js

@@ -0,0 +1,9 @@
+import {Context} from './opaque';
+import {useContext} from 'react';
+import * as React from 'react';
+
+component Foo() {
+  const context = useContext(Context);
+  context as number; // ERROR
+  return null;
+}

tests/opaque_types_and_component_syntax_destructors/opaque.js

@@ -0,0 +1,11 @@
+import {createContext, useContext} from 'react'
+
+export opaque type Tag = number;
+
+export const Context: React$Context<Tag> = createContext<Tag>(3);
+
+component Foo() {
+  const context = useContext(Context);
+  context as number; // OK
+  return null;
+}

tests/opaque_types_and_component_syntax_destructors/opaque_types_and_component_syntax_destructors.exp

@@ -0,0 +1,19 @@
+Error ------------------------------------------------------------------------------------------------- non-local.js:7:3
+
+Cannot cast `context` to number because `Tag` [1] is incompatible with number [2]. [incompatible-cast]
+
+   non-local.js:7:3
+   7|   context as number; // ERROR
+        ^^^^^^^
+
+References:
+   opaque.js:5:37
+   5| export const Context: React$Context<Tag> = createContext<Tag>(3);
+                                          ^^^ [1]
+   non-local.js:7:14
+   7|   context as number; // ERROR
+                   ^^^^^^ [2]
+
+
+
+Found 1 error