From 469961d92b9d34f47823ff427a07104949da6231 Mon Sep 17 00:00:00 2001 From: Sam Zhou Date: Fri, 13 Dec 2024 11:13:16 -0800 Subject: [PATCH] [flow] More principled opaque type underlying_t unwrapping 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 --- src/typing/flow_js.ml | 36 ++++++++++++++++++++++++++++-------- src/typing/subtyping_kit.ml | 14 ++++++++++---- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml index 9fc3b62e599..02da052f54c 100644 --- a/src/typing/flow_js.ml +++ b/src/typing/flow_js.ml @@ -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. *) @@ -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 *) @@ -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 @@ -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 diff --git a/src/typing/subtyping_kit.ml b/src/typing/subtyping_kit.ml index adef0c82ea4..565c5575763 100644 --- a/src/typing/subtyping_kit.ml +++ b/src/typing/subtyping_kit.ml @@ -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 *)