From 5cb0c8d804a1bc2d4bc8ea29cdfde731a79d8ed0 Mon Sep 17 00:00:00 2001 From: George Zahariev Date: Wed, 29 Nov 2023 12:42:31 -0800 Subject: [PATCH] [flow] Union of exact objects allow computed access as long as one of the members of the union have the property Summary: This already worked for named properties since D34408092, here we make it work for computed properties as well. This will be more common once we support numeric keys, as they always need to use computed access. Changelog: [errors] Union of exact objects allow computed access as long as one of the members of the union have the property Reviewed By: SamChou19815 Differential Revision: D51578298 fbshipit-source-id: a134633e4e6ad861cf866ad9d652127b5f1885b3 --- src/typing/debug_js.ml | 2 +- src/typing/default_resolve.ml | 2 +- src/typing/flow_js.ml | 21 ++-- src/typing/implicit_instantiation.ml | 1 + src/typing/statement.ml | 11 ++- src/typing/type.ml | 9 +- src/typing/typeUtil.ml | 2 +- src/typing/type_hint.ml | 5 +- tests/exact/exact.exp | 121 ++++++++++++++++-------- tests/exact/nonstrict-access.js | 22 ++++- tests/exact_empty_objects/defaulting.js | 8 ++ 11 files changed, 145 insertions(+), 59 deletions(-) diff --git a/src/typing/debug_js.ml b/src/typing/debug_js.ml index f2578bb74a5..b1ff7cb1cd8 100644 --- a/src/typing/debug_js.ml +++ b/src/typing/debug_js.ml @@ -751,7 +751,7 @@ and dump_use_t_ (depth, tvars) cx t = | ExportTypeT _ -> p t | ImplicitVoidReturnT _ -> p t | AssertExportIsTypeT _ -> p t - | GetElemT { use_op = _; reason = _; from_annot; key_t; tout = (preason, ptvar) } -> + | GetElemT { use_op = _; reason = _; id = _; from_annot; key_t; tout = (preason, ptvar) } -> p ~extra: (spf diff --git a/src/typing/default_resolve.ml b/src/typing/default_resolve.ml index 55826a44213..147513997d3 100644 --- a/src/typing/default_resolve.ml +++ b/src/typing/default_resolve.ml @@ -54,7 +54,7 @@ let rec default_resolve_touts ~flow ?resolve_callee cx loc u = in let resolve_elem_action action = match action with - | ReadElem (_, tvar) -> resolve_tvar tvar + | ReadElem (_, _, tvar) -> resolve_tvar tvar | WriteElem (_, topt, _) -> map_opt resolve topt | CallElem (_, action) -> resolve_method_action action in diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml index e32d45c975d..a53df05226a 100644 --- a/src/typing/flow_js.ml +++ b/src/typing/flow_js.ml @@ -891,7 +891,7 @@ struct tout_tvar ) | OptionalIndexedAccessTypeIndex key_t -> - GetElemT { use_op; reason; from_annot = true; key_t; tout = tout_tvar } + GetElemT { use_op; reason; id = None; from_annot = true; key_t; tout = tout_tvar } in rec_flow cx trace (l, u) (*************) @@ -1655,14 +1655,14 @@ struct | (UnionT (_, rep), PredicateT (((MaybeP | NotP MaybeP | ExistsP | NotP ExistsP) as p), t)) when UnionRep.is_optimized_finally rep -> predicate cx trace t l p - | (UnionT (_, rep), ElemT (use_op, reason, obj, ReadElem (true (* annot *), tout))) -> + | (UnionT (_, rep), ElemT (use_op, reason, obj, ReadElem (id, true (* annot *), tout))) -> let reason = update_desc_reason invalidate_rtype_alias reason in let (t0, (t1, ts)) = UnionRep.members_nel rep in let f t = AnnotT ( reason, Tvar.mk_no_wrap_where cx reason (fun tvar -> - rec_flow cx trace (t, ElemT (use_op, reason, obj, ReadElem (true, tvar))) + rec_flow cx trace (t, ElemT (use_op, reason, obj, ReadElem (id, true, tvar))) ), false ) @@ -4150,7 +4150,7 @@ struct (* strings may have their characters read *) (******************************************) | ( DefT (reason_s, StrT _), - GetElemT { use_op; reason = reason_op; from_annot = _; key_t; tout } + GetElemT { use_op; reason = reason_op; id = _; from_annot = _; key_t; tout } ) -> rec_flow cx trace (key_t, UseT (use_op, NumT.why reason_s)); rec_flow_t cx trace ~use_op:unknown_use (StrT.why reason_op, OpenT tout) @@ -4167,9 +4167,9 @@ struct ) -> rec_flow cx trace (key, ElemT (use_op, reason_op, l, WriteElem (tin, tout, mode))) | ( (DefT (_, (ObjT _ | ArrT _ | InstanceT _)) | AnyT _), - GetElemT { use_op; reason = reason_op; from_annot; key_t; tout } + GetElemT { use_op; reason = reason_op; id; from_annot; key_t; tout } ) -> - rec_flow cx trace (key_t, ElemT (use_op, reason_op, l, ReadElem (from_annot, tout))) + rec_flow cx trace (key_t, ElemT (use_op, reason_op, l, ReadElem (id, from_annot, tout))) | ( (DefT (_, (ObjT _ | ArrT _ | InstanceT _)) | AnyT _), CallElemT (use_op, reason_call, reason_lookup, key, action) ) -> @@ -6813,7 +6813,8 @@ struct else getprop_ub () | Elem key_t -> - GetElemT { use_op = unknown_use; reason; from_annot = annot; key_t; tout = tvar } + GetElemT + { use_op = unknown_use; reason; id = None; from_annot = annot; key_t; tout = tvar } | ObjRest xs -> ObjRestT (reason, xs, OpenT tvar, id) | ArrRest i -> ArrRestT (unknown_use, reason, i, OpenT tvar) | Default -> PredicateT (NotP VoidP, tvar) @@ -7077,7 +7078,7 @@ struct let reason_op = replace_desc_reason (RProperty (Some name)) reason in GetPropT (use_op, reason, None, mk_named_prop ~reason:reason_op name, tout) | ElementType { index_type; _ } -> - GetElemT { use_op; reason; from_annot = true; key_t = index_type; tout } + GetElemT { use_op; reason; id = None; from_annot = true; key_t = index_type; tout } | OptionalIndexedAccessNonMaybeType { index } -> OptionalIndexedAccessT { use_op; reason; index; tout_tvar = tout } | OptionalIndexedAccessResultType { void_reason } -> @@ -7551,7 +7552,7 @@ struct and elem_action_on_obj cx trace ~use_op l obj reason_op action = let propref = propref_for_elem_t l in match action with - | ReadElem (_, t) -> rec_flow cx trace (obj, GetPropT (use_op, reason_op, None, propref, t)) + | ReadElem (id, _, t) -> rec_flow cx trace (obj, GetPropT (use_op, reason_op, id, propref, t)) | WriteElem (tin, tout, mode) -> rec_flow cx trace (obj, SetPropT (use_op, reason_op, propref, mode, Normal, tin, None)); Base.Option.iter ~f:(fun t -> rec_flow_t cx trace ~use_op:unknown_use (obj, t)) tout @@ -9622,7 +9623,7 @@ struct and perform_elem_action cx trace ~use_op ~restrict_deletes reason_op l value action = match (action, restrict_deletes) with - | (ReadElem (_, t), _) -> + | (ReadElem (_, _, t), _) -> let loc = loc_of_reason reason_op in rec_flow_t cx trace ~use_op:unknown_use (reposition cx ~trace loc value, OpenT t) | (WriteElem (tin, tout, Assign), _) diff --git a/src/typing/implicit_instantiation.ml b/src/typing/implicit_instantiation.ml index 73e465b34c7..8ad95d09b38 100644 --- a/src/typing/implicit_instantiation.ml +++ b/src/typing/implicit_instantiation.ml @@ -539,6 +539,7 @@ module Make (Observer : OBSERVER) (Flow : Flow_common.S) : S = struct { use_op = unknown_use; reason; + id = None; from_annot = true; key_t = NumT.make reason; tout; diff --git a/src/typing/statement.ml b/src/typing/statement.ml index 8991ae5a632..d6e6de4f673 100644 --- a/src/typing/statement.ml +++ b/src/typing/statement.ml @@ -4090,7 +4090,16 @@ module Make | (Member { Member._object; property = Member.PropertyExpression index; comments }, _) -> let reason = mk_reason (RProperty None) loc in let use_op = Op (GetProperty (mk_expression_reason ex)) in - let get_opt_use tind _ = OptGetElemT (use_op, reason, false (* annot *), tind) in + (* Only create an id if the property expression is a literal, which are + treated like named props. *) + let id = + match index with + | (_, StringLiteral _) + | (_, NumberLiteral _) -> + Some (mk_id ()) + | _ -> None + in + let get_opt_use tind _ = OptGetElemT (use_op, reason, id, false (* annot *), tind) in let get_mem_t tind reason obj_t = Tvar_resolver.mk_tvar_and_fully_resolve_no_wrap_where cx reason (fun t -> let use = apply_opt_use (get_opt_use tind reason) t in diff --git a/src/typing/type.ml b/src/typing/type.ml index 9c5accadc61..12645ee2481 100644 --- a/src/typing/type.ml +++ b/src/typing/type.ml @@ -588,6 +588,7 @@ module rec TypeTerm : sig | GetElemT of { use_op: use_op; reason: reason; + id: ident option; from_annot: bool; key_t: t; tout: tvar; @@ -1014,7 +1015,7 @@ module rec TypeTerm : sig | OptGetPropT of use_op * reason * ident option * propref | OptGetPrivatePropT of use_op * reason * string * class_binding list * bool | OptTestPropT of use_op * reason * ident * propref - | OptGetElemT of use_op * reason * bool (* from annot *) * t + | OptGetElemT of use_op * reason * ident option * bool (* from annot *) * t | OptCallElemT of use_op * (* call *) reason * (* lookup *) reason * t * opt_method_action and opt_state = @@ -1411,7 +1412,7 @@ module rec TypeTerm : sig to initialize the property value, but in order to avoid race conditions we need to ensure that reads happen after writes. *) and elem_action = - | ReadElem of bool (* annot *) * tvar + | ReadElem of ident option * bool (* annot *) * tvar | WriteElem of t * t option (* tout *) * set_mode | CallElem of reason * method_action @@ -4346,8 +4347,8 @@ let apply_opt_use opt_use t_out = | OptGetPropT (u, r, i, p) -> GetPropT (u, r, i, p, t_out) | OptGetPrivatePropT (u, r, s, cbs, b) -> GetPrivatePropT (u, r, s, cbs, b, t_out) | OptTestPropT (u, r, i, p) -> TestPropT (u, r, i, p, t_out) - | OptGetElemT (use_op, reason, from_annot, key_t) -> - GetElemT { use_op; reason; from_annot; key_t; tout = t_out } + | OptGetElemT (use_op, reason, id, from_annot, key_t) -> + GetElemT { use_op; reason; id; from_annot; key_t; tout = t_out } | OptCallElemT (u, r1, r2, elt, call) -> CallElemT (u, r1, r2, elt, apply_opt_action call t_out) let mk_enum_type reason enum = diff --git a/src/typing/typeUtil.ml b/src/typing/typeUtil.ml index edbc1f80fc4..a9fbede7a3b 100644 --- a/src/typing/typeUtil.ml +++ b/src/typing/typeUtil.ml @@ -416,7 +416,7 @@ and mod_reason_of_opt_use_t f = function | OptGetPrivatePropT (use_op, reason, name, bindings, static) -> OptGetPrivatePropT (use_op, f reason, name, bindings, static) | OptTestPropT (use_op, reason, id, n) -> OptTestPropT (use_op, f reason, id, n) - | OptGetElemT (use_op, reason, annot, it) -> OptGetElemT (use_op, f reason, annot, it) + | OptGetElemT (use_op, reason, id, annot, it) -> OptGetElemT (use_op, f reason, id, annot, it) | OptCallElemT (use_op, r1, r2, elt, call) -> OptCallElemT (use_op, f r1, r2, elt, call) let rec util_use_op_of_use_t : diff --git a/src/typing/type_hint.ml b/src/typing/type_hint.ml index 56e38c80231..9896bbcfca3 100644 --- a/src/typing/type_hint.ml +++ b/src/typing/type_hint.ml @@ -355,6 +355,7 @@ and type_of_hint_decomposition cx op reason t = { use_op = unknown_use; reason; + id = None; from_annot = true; key_t = DefT (reason, NumT num); tout; @@ -516,7 +517,9 @@ and type_of_hint_decomposition cx op reason t = | Decomp_ObjComputed reason -> let key_t = Type_env.find_write cx Env_api.ExpressionLoc reason in Tvar.mk_no_wrap_where cx reason (fun tout -> - let use_t = GetElemT { use_op = unknown_use; reason; from_annot = true; key_t; tout } in + let use_t = + GetElemT { use_op = unknown_use; reason; id = None; from_annot = true; key_t; tout } + in SpeculationFlow.resolved_lower_flow_unsafe cx reason (t, use_t) ) | Decomp_ObjSpread -> diff --git a/tests/exact/exact.exp b/tests/exact/exact.exp index 441d205d58a..bdd2573b0cf 100644 --- a/tests/exact/exact.exp +++ b/tests/exact/exact.exp @@ -186,100 +186,147 @@ References: ^^^^^^^^^ [2] -Error ----------------------------------------------------------------------------------------- nonstrict-access.js:17:4 +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:18:4 Cannot cast `x.some` to string because `void` (due to access of non-existent property `some`) [1] is incompatible with string [2]. [incompatible-cast] - nonstrict-access.js:17:4 - 17| (x.some: string); // ERROR + nonstrict-access.js:18:4 + 18| (x.some: string); // ERROR ^^^^^^ [1] References: - nonstrict-access.js:17:12 - 17| (x.some: string); // ERROR + nonstrict-access.js:18:12 + 18| (x.some: string); // ERROR ^^^^^^ [2] -Error ----------------------------------------------------------------------------------------- nonstrict-access.js:19:6 +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:19:4 + +Cannot cast `x['some']` to string because `void` (due to access of non-existent property `some`) [1] is incompatible +with string [2]. [incompatible-cast] + + nonstrict-access.js:19:4 + 19| (x['some']: string); // ERROR + ^^^^^^^^^ [1] + +References: + nonstrict-access.js:19:15 + 19| (x['some']: string); // ERROR + ^^^^^^ [2] + + +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:22:6 Cannot get `x.NONE` because property `NONE` is missing in object type [1]. [prop-missing] - nonstrict-access.js:19:6 - 19| (x.NONE); // ERROR + nonstrict-access.js:22:6 + 22| (x.NONE); // ERROR ^^^^ References: - nonstrict-access.js:13:18 - 13| declare var x: T; - ^ [1] + nonstrict-access.js:13:20 + 13| declare const x: T; + ^ [1] + + +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:23:6 +Cannot get `x['NONE']` because property `NONE` is missing in object type [1]. [prop-missing] -Error ----------------------------------------------------------------------------------------- nonstrict-access.js:24:4 + nonstrict-access.js:23:6 + 23| (x['NONE']); // ERROR + ^^^^^^ + +References: + nonstrict-access.js:13:20 + 13| declare const x: T; + ^ [1] + + +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:28:4 Cannot cast `some` to string because `void` (due to access of non-existent property `some`) [1] is incompatible with string [2]. [incompatible-cast] - nonstrict-access.js:24:4 - 24| (some: string); // ERROR + nonstrict-access.js:28:4 + 28| (some: string); // ERROR ^^^^ References: - nonstrict-access.js:22:15 - 22| const {all, some} = x; + nonstrict-access.js:26:15 + 26| const {all, some} = x; ^^^^ [1] - nonstrict-access.js:24:10 - 24| (some: string); // ERROR + nonstrict-access.js:28:10 + 28| (some: string); // ERROR ^^^^^^ [2] -Error ---------------------------------------------------------------------------------------- nonstrict-access.js:27:10 +Error ---------------------------------------------------------------------------------------- nonstrict-access.js:31:10 Property `NONE` is missing in object type [1]. [prop-missing] - nonstrict-access.js:27:10 - 27| const {NONE} = x; // ERROR + nonstrict-access.js:31:10 + 31| const {NONE} = x; // ERROR ^^^^ References: - nonstrict-access.js:13:18 - 13| declare var x: T; - ^ [1] + nonstrict-access.js:13:20 + 13| declare const x: T; + ^ [1] -Error ----------------------------------------------------------------------------------------- nonstrict-access.js:43:4 +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:47:4 Cannot cast `obj.some` to string because `void` (due to access of non-existent property `some`) [1] is incompatible with string [2]. [incompatible-cast] - nonstrict-access.js:43:4 - 43| (obj.some: string); // ERROR + nonstrict-access.js:47:4 + 47| (obj.some: string); // ERROR ^^^^^^^^ [1] References: - nonstrict-access.js:43:14 - 43| (obj.some: string); // ERROR + nonstrict-access.js:47:14 + 47| (obj.some: string); // ERROR ^^^^^^ [2] -Error ----------------------------------------------------------------------------------------- nonstrict-access.js:50:4 +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:54:4 Cannot cast `some` to string because `void` (due to access of non-existent property `some`) [1] is incompatible with string [2]. [incompatible-cast] - nonstrict-access.js:50:4 - 50| (some: string); // ERROR + nonstrict-access.js:54:4 + 54| (some: string); // ERROR ^^^^ References: - nonstrict-access.js:46:15 - 46| const {all, some, baz} = obj; + nonstrict-access.js:50:15 + 50| const {all, some, baz} = obj; ^^^^ [1] - nonstrict-access.js:50:10 - 50| (some: string); // ERROR + nonstrict-access.js:54:10 + 54| (some: string); // ERROR ^^^^^^ [2] +Error ----------------------------------------------------------------------------------------- nonstrict-access.js:82:7 + +Cannot get `Foo[k]` because property `xxx` is missing in object type [1]. [prop-missing] + + nonstrict-access.js:82:7 + 82| Foo[k]; // ERROR: prop-missing + ^ + +References: + nonstrict-access.js:75:21 + v + 75| declare const Foo:{ + 76| foo: 1, + 77| bar: 2, + 78| }; + ^ [1] + + Error ----------------------------------------------------------------------------------------------- not_object.js:3:15 Cannot assign `3` to `x` because number [1] is incompatible with object type [2]. [incompatible-type] @@ -621,4 +668,4 @@ References: -Found 42 errors +Found 45 errors diff --git a/tests/exact/nonstrict-access.js b/tests/exact/nonstrict-access.js index 6fa99f6bfab..e6acd2ee9f1 100644 --- a/tests/exact/nonstrict-access.js +++ b/tests/exact/nonstrict-access.js @@ -10,13 +10,17 @@ type T = {| { - declare var x: T; + declare const x: T; // Prop access (x.all: string); // OK + (x['all']: string); // OK (x.some: string); // ERROR + (x['some']: string); // ERROR (x.some: string | void); // OK + (x['some']: string | void); // OK (x.NONE); // ERROR + (x['NONE']); // ERROR // Destructuring const {all, some} = x; @@ -29,7 +33,7 @@ type T = {| // Object literals are exact { - declare var cond: boolean; + declare const cond: boolean; const toSpread = cond ? {all: "foo"} : {all: "foo", some: "bar"}; const obj = { ...toSpread, @@ -55,7 +59,7 @@ type T = {| // It should not be possible to create such an object (intersection of // different exact objects), but unfortunately we have many instances // of this (e.g. React props) that exist due to other unsoundness issues. - declare var o: {|a: number|} & {|b: string|}; + declare const o: {|a: number|} & {|b: string|}; // Prop access (o.a: number); // OK @@ -65,3 +69,15 @@ type T = {| (a: number); // OK (b: string); // OK } + +// Computed with union +{ + declare const Foo:{ + foo: 1, + bar: 2, + }; + type K = 'foo' | 'bar' | 'xxx'; + declare const k: K; + + Foo[k]; // ERROR: prop-missing +} diff --git a/tests/exact_empty_objects/defaulting.js b/tests/exact_empty_objects/defaulting.js index 9963a853494..32f262c46cf 100644 --- a/tests/exact_empty_objects/defaulting.js +++ b/tests/exact_empty_objects/defaulting.js @@ -109,3 +109,11 @@ declare var a: ?any; const {y} = a || {}; // OK const {z} = a ? a : {}; // OK } + +// Computed props +{ + const x = o ?? {}; + + const a = x['prop']; + (a: number | void); // OK +}