Skip to content

Commit

Permalink
[flow] More principled opaque type underlying_t unwrapping
Browse files Browse the repository at this point in the history
Summary:
This diff is a different approach that tries to solve the same problem D55411073 tries to solve: the current system to decide whether we unwrap underlying_t is error prone.

We depend on reason's source equality to decide whether the operation and the opaque type are in the same file. Especially for the reason of opaque type, we are not even using the def location. Reasons are unreliable. This diff switches to use ALoc.id's source and `Context.file`. The opaque type's id will always have source that's the defining file of the type, and `Context.file` always return the current file we are checking, so this is actually bulletproof.

Changelog: [errors] We fixed a bug that causes opaque type's underlying representation to leak beyond the file it's defined. You might see more errors as a result.

Reviewed By: panagosg7

Differential Revision: D67147723

fbshipit-source-id: a2b18c7648be6947f89c157959bb605d4f9c75af
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Dec 13, 2024
1 parent a87d923 commit 469961d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 12 deletions.
36 changes: 28 additions & 8 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1446,8 +1446,11 @@ struct
rec_flow cx trace (reposition_reason cx ~trace reason ~use_desc l, u)
(* Store the opaque type when doing `ToStringT`, so we can use that
rather than just `string` if the underlying is `string`. *)
| (OpaqueT (r, { underlying_t = Some t; _ }), ToStringT { reason; t_out; _ })
when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
| ( OpaqueT
(_, { opaque_id = Opaque.UserDefinedOpaqueTypeId opaque_id; underlying_t = Some t; _ }),
ToStringT { reason; t_out; _ }
)
when ALoc.source (opaque_id :> ALoc.t) = Some (Context.file cx) ->
rec_flow cx trace (t, ToStringT { orig_t = Some l; reason; t_out })
(* Use the upper bound of OpaqueT if it's available, for operations that must be
* performed on some concretized types. *)
Expand All @@ -1460,8 +1463,11 @@ struct
rec_flow cx trace (t, ToStringT { orig_t = Some l; reason; t_out })
(* If the type is still in the same file it was defined, we allow it to
* expose its underlying type information *)
| (OpaqueT (r, { underlying_t = Some t; _ }), _)
when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
| ( OpaqueT
(_, { opaque_id = Opaque.UserDefinedOpaqueTypeId opaque_id; underlying_t = Some t; _ }),
_
)
when ALoc.source (opaque_id :> ALoc.t) = Some (Context.file cx) ->
rec_flow cx trace (t, u)
(*****************************************************)
(* keys (NOTE: currently we only support string keys *)
Expand Down Expand Up @@ -6600,10 +6606,24 @@ struct
in
(match (t, d) with
| ( GenericT
{ bound = OpaqueT (_, { underlying_t = Some t; _ }); reason = r; id; name; no_infer },
{
bound =
OpaqueT
( _,
{
opaque_id = Opaque.UserDefinedOpaqueTypeId opaque_id;
underlying_t = Some t;
_;
}
);
reason = r;
id;
name;
no_infer;
},
_
)
when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
when ALoc.source (opaque_id :> ALoc.t) = Some (Context.file cx) ->
eval_destructor
cx
~trace
Expand All @@ -6613,8 +6633,8 @@ struct
d
tout
| (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) ->
| (OpaqueT (_, { opaque_id = Opaque.UserDefinedOpaqueTypeId id; underlying_t = Some t; _ }), _)
when ALoc.source (id :> ALoc.t) = Some (Context.file cx) ->
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
Expand Down
14 changes: 10 additions & 4 deletions src/typing/subtyping_kit.ml
Original file line number Diff line number Diff line change
Expand Up @@ -897,13 +897,19 @@ module Make (Flow : INPUT) : OUTPUT = struct
flow_type_args cx trace ~use_op lreason ureason ltargs utargs
(* If the type is still in the same file it was defined, we allow it to
* expose its underlying type information *)
| (OpaqueT (r, { underlying_t = Some t; _ }), _)
when ALoc.source (loc_of_reason r) = ALoc.source (def_loc_of_reason r) ->
| ( OpaqueT
(_, { opaque_id = Opaque.UserDefinedOpaqueTypeId opaque_id; underlying_t = Some t; _ }),
_
)
when ALoc.source (opaque_id :> ALoc.t) = Some (Context.file cx) ->
rec_flow_t cx trace ~use_op (t, u)
(* If the lower bound is in the same file as where the opaque type was defined,
* we expose the underlying type information *)
| (_, OpaqueT (r, { underlying_t = Some t; _ }))
when ALoc.source (loc_of_reason (reason_of_t l)) = ALoc.source (def_loc_of_reason r) ->
| ( _,
OpaqueT
(_, { opaque_id = Opaque.UserDefinedOpaqueTypeId opaque_id; underlying_t = Some t; _ })
)
when ALoc.source (opaque_id :> ALoc.t) = Some (Context.file cx) ->
rec_flow_t cx trace ~use_op (l, t)
(***********************)
(* Numeric string keys *)
Expand Down

0 comments on commit 469961d

Please sign in to comment.