Skip to content

Commit

Permalink
[flow] Improve error messages on missing props that fail indexer lookups
Browse files Browse the repository at this point in the history
Summary:
If the property name -> indexer type flow does not pass then we do not bother subtyping the value types. This avoids redundant errors.

Notably, this can't be done during implicit instantiation because the key/value may contain implicitly instantiated tvars that rely on those flows to constrain them.

Changelog: [internal]

Reviewed By: SamChou19815

Differential Revision: D69056970

fbshipit-source-id: f97e5ef69ce326bdd0856c4c714c48917fb40ab4
  • Loading branch information
jbrown215 authored and facebook-github-bot committed Feb 4, 2025
1 parent 7602412 commit 53c63f5
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 145 deletions.
139 changes: 89 additions & 50 deletions src/typing/subtyping_kit.ml
Original file line number Diff line number Diff line change
Expand Up @@ -388,58 +388,97 @@ module Make (Flow : INPUT) : OUTPUT = struct
(match udict with
| None -> ()
| Some { key; value; dict_polarity; _ } ->
let keys =
Context.fold_real_props
cx
lflds
(fun name lp keys ->
let flow_prop_to_indexer lp name =
let use_op =
Frame
(PropertyCompatibility { prop = Some name; lower = lreason; upper = ureason }, use_op)
in
let lp =
match lp with
| Field
{
preferred_def_locs = _;
key_loc = _;
type_ = OptionalT { reason = _; type_ = lt; use_desc = _ };
polarity;
} ->
OrdinaryField { type_ = mod_t (Some name) ldro lt; polarity }
| _ -> Property.type_ lp |> TypeUtil.map_property ~f:(mod_t (Some name) ldro)
in
let up = OrdinaryField { type_ = mod_t (Some name) udro value; polarity = dict_polarity } in
begin
if lit then
match (Property.read_t_of_property_type lp, Property.read_t_of_property_type up) with
| (Some lt, Some ut) -> rec_flow cx trace (lt, UseT (use_op, ut))
| _ -> ()
else
let propref =
mk_named_prop ~reason:(replace_desc_reason (RProperty (Some name)) lreason) name
in
rec_flow_p cx ~trace ~use_op lreason ureason propref (lp, up)
end
in
(* If we are in implicit instantiation then we should always flow missing keys & value types to the
* upper dictionary because that information may be useful to infer a type. Outside of implicit instantiation,
* flowing both can cause redundant errors when the key is already not a valid indexer key, so we avoid the
* value flows when that does not pass *)
( if not (Context.in_implicit_instantiation cx) then
Context.iter_real_props cx lflds (fun name lp ->
if Context.has_prop cx uflds name then
keys
else
let use_op =
Frame
( PropertyCompatibility { prop = Some name; lower = lreason; upper = ureason },
use_op
()
else begin
if speculative_subtyping_succeeds cx (type_of_key_name cx name lreason) key then
flow_prop_to_indexer lp name
else
let use_op =
Frame
( PropertyCompatibility
{
prop = Some name;
(* Lower and upper are reversed in this case since the lower object
* is the one requiring the prop. *)
lower = ureason;
upper = lreason;
},
use_op
)
in
add_output
cx
Error_message.(
EIndexerCheckFailed
{
prop_name = name;
reason_prop = lreason;
reason_obj = ureason;
reason_indexer = reason_of_t key;
use_op;
}
)
in
let lp =
match lp with
| Field
{
preferred_def_locs = _;
key_loc = _;
type_ = OptionalT { reason = _; type_ = lt; use_desc = _ };
polarity;
} ->
OrdinaryField { type_ = mod_t (Some name) ldro lt; polarity }
| _ -> Property.type_ lp |> TypeUtil.map_property ~f:(mod_t (Some name) ldro)
in
let up =
OrdinaryField { type_ = mod_t (Some name) udro value; polarity = dict_polarity }
in
begin
if lit then
match
(Property.read_t_of_property_type lp, Property.read_t_of_property_type up)
with
| (Some lt, Some ut) -> rec_flow cx trace (lt, UseT (use_op, ut))
| _ -> ()
else
let propref =
mk_named_prop ~reason:(replace_desc_reason (RProperty (Some name)) lreason) name
in
rec_flow_p cx ~trace ~use_op lreason ureason propref (lp, up)
end;
type_of_key_name cx name lreason :: keys)
[]
|> union_of_ts lreason
in
rec_flow
cx
trace
( keys,
UseT (Frame (IndexerKeyCompatibility { lower = lreason; upper = ureason }, use_op), key)
);
end
)
else
let keys =
Context.fold_real_props
cx
lflds
(fun name lp keys ->
if Context.has_prop cx uflds name then
keys
else (
flow_prop_to_indexer lp name;
type_of_key_name cx name lreason :: keys
))
[]
|> union_of_ts lreason
in
rec_flow
cx
trace
( keys,
UseT (Frame (IndexerKeyCompatibility { lower = lreason; upper = ureason }, use_op), key)
)
);
(* If the left is inexact and the right is indexed, Flow mixed to the indexer
* value. Mixed represents the possibly unknown properties on the inexact object
*)
Expand Down
28 changes: 16 additions & 12 deletions tests/contextual_typing/contextual_typing.exp
Original file line number Diff line number Diff line change
Expand Up @@ -1070,21 +1070,21 @@ An annotation on `y` is required because Flow cannot infer its type from local c

Error --------------------------------------------------------------------------------------------------- unions.js:4:48

Cannot assign object literal to `x` because in the indexer property's key: [incompatible-type]
- Either property `42` [1] is incompatible with string literal `A` [2].
- Or property `42` [1] is incompatible with string literal `B` [3].
Cannot assign object literal to `x` because property `42` is missing in object type [1] but exists in object
literal [2]. Any property that does not exist in object type [1] must be compatible with its indexer union type [3].
[incompatible-type]

unions.js:4:48
4| const x: {['A' | 'B']: (number) => number} = {[42]: v => v}; // ERROR
^^^^^^^^^^^^^^ [1]
^^^^^^^^^^^^^^ [2]

References:
unions.js:4:14
unions.js:4:12
4| const x: {['A' | 'B']: (number) => number} = {[42]: v => v}; // ERROR
^^^ [2]
unions.js:4:20
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
unions.js:4:14
4| const x: {['A' | 'B']: (number) => number} = {[42]: v => v}; // ERROR
^^^ [3]
^^^^^^^^^ [3]


Error --------------------------------------------------------------------------------------------------- unions.js:4:55
Expand All @@ -1097,17 +1097,21 @@ An annotation on `v` is required because Flow cannot infer its type from local c

Error --------------------------------------------------------------------------------------------------- unions.js:7:42

Cannot assign object literal to `x` because property `42` [1] is incompatible with string literal `A` [2] in the indexer
property's key. [incompatible-type]
Cannot assign object literal to `x` because property `42` is missing in object type [1] but exists in object
literal [2]. Any property that does not exist in object type [1] must be compatible with its indexer string literal
`A` [3]. [incompatible-type]

unions.js:7:42
7| const x: {['A']: (number) => number} = {[42]: v => v}; // ERROR
^^^^^^^^^^^^^^ [1]
^^^^^^^^^^^^^^ [2]

References:
unions.js:7:12
7| const x: {['A']: (number) => number} = {[42]: v => v}; // ERROR
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
unions.js:7:14
7| const x: {['A']: (number) => number} = {[42]: v => v}; // ERROR
^^^ [2]
^^^ [3]


Error --------------------------------------------------------------------------------------------------- unions.js:7:49
Expand Down
42 changes: 12 additions & 30 deletions tests/indexed_objects/indexed_objects.exp
Original file line number Diff line number Diff line change
@@ -1,39 +1,21 @@
Error ---------------------------------------------------------------------------------------------------- test.js:15:23
Error ---------------------------------------------------------------------------------------------------- test.js:14:23

Cannot assign object literal to `error1` because string `foo` [1] is incompatible with string prefixed with `data-` [2]
in the indexer property's key. [incompatible-type]
Cannot assign object literal to `error1` because property `foo` is missing in `Props` [1] but exists in object
literal [2]. Any property that does not exist in `Props` [1] must be compatible with its indexer string prefixed with
`data-` [3]. [incompatible-type]

test.js:15:23
15| const error1: Props = {foo: 3};
^^^^^^^^ [1]
test.js:14:23
14| const error1: Props = {foo: 3}; // One error
^^^^^^^^ [2]

References:
test.js:14:15
14| const error1: Props = {foo: 3}; // One error
^^^^^ [1]
test.js:11:4
11| [StringPrefix<'data-'>]: string | void,
^^^^^^^^^^^^^^^^^^^^^ [2]
^^^^^^^^^^^^^^^^^^^^^ [3]


Error ---------------------------------------------------------------------------------------------------- test.js:15:29

Cannot assign object literal to `error1` because in property `foo`: [incompatible-type]
- Either number [1] is incompatible with string [2].
- Or number [1] is incompatible with undefined [3].

test.js:15:29
15| const error1: Props = {foo: 3};
^ [1]

References:
test.js:11:28
11| [StringPrefix<'data-'>]: string | void,
^^^^^^ [2]
test.js:11:37
11| [StringPrefix<'data-'>]: string | void,
^^^^ [3]



Found 2 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
Found 1 error
3 changes: 1 addition & 2 deletions tests/indexed_objects/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ type Props = {
[StringPrefix<'data-'>]: string | void,
}

// TODO: Combine errors
const error1: Props = {foo: 3};
const error1: Props = {foo: 3}; // One error
38 changes: 15 additions & 23 deletions tests/indexer/indexer.exp
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
Error ------------------------------------------------------------------------------------------------------- A.js:13:10

Cannot return object literal because string `foo` [1] is incompatible with number [2] in the indexer property's key.
[incompatible-return]
Cannot return object literal because property `foo` is missing in object type [1] but exists in object literal [2]. Any
property that does not exist in object type [1] must be compatible with its indexer number [3]. [incompatible-type]

A.js:13:10
13| return { foo: "bar" }
^^^^^^^^^^^^^^ [1]
^^^^^^^^^^^^^^ [2]

References:
A.js:12:18
12| function foo2(): {[key: number]: string} {
^^^^^^^^^^^^^^^^^^^^^^^ [1]
A.js:12:25
12| function foo2(): {[key: number]: string} {
^^^^^^ [2]
^^^^^^ [3]


Error ------------------------------------------------------------------------------------------------------- A.js:18:17
Expand All @@ -29,31 +32,20 @@ References:

Error ------------------------------------------------------------------------------------------------------- A.js:23:10

Cannot return object literal because string `foo` [1] is incompatible with number [2] in the indexer property's key.
[incompatible-return]
Cannot return object literal because property `foo` is missing in object type [1] but exists in object literal [2]. Any
property that does not exist in object type [1] must be compatible with its indexer number [3]. [incompatible-type]

A.js:23:10
23| return { foo: "bar" }
^^^^^^^^^^^^^^ [1]
^^^^^^^^^^^^^^ [2]

References:
A.js:22:25
A.js:22:18
22| function foo4(): {[key: number]: number} {
^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------- A.js:23:17

Cannot return object literal because string [1] is incompatible with number [2] in property `foo`. [incompatible-return]

A.js:23:17
23| return { foo: "bar" }
^^^^^ [1]

References:
A.js:22:34
^^^^^^^^^^^^^^^^^^^^^^^ [1]
A.js:22:25
22| function foo4(): {[key: number]: number} {
^^^^^^ [2]
^^^^^^ [3]


Error ------------------------------------------------------------------------------------------------------- A.js:38:17
Expand Down Expand Up @@ -114,4 +106,4 @@ Multiple indexers are not supported. [unsupported-syntax]



Found 8 errors
Found 7 errors
12 changes: 8 additions & 4 deletions tests/interface/interface.exp
Original file line number Diff line number Diff line change
Expand Up @@ -683,17 +683,21 @@ References:

Error ------------------------------------------------------------------------------------------------ interface.js:47:2

Cannot cast object literal to interface type because string `x` [1] is incompatible with number [2] in the indexer
property's key. [incompatible-cast]
Cannot cast object literal to interface type because property `x` is missing in interface type [1] but exists in object
literal [2]. Any property that does not exist in interface type [1] must be compatible with its indexer number [3].
[incompatible-type]

interface.js:47:2
47| ({x : 3} : interface {[number] : number}); // error
^^^^^^^ [1]
^^^^^^^ [2]

References:
interface.js:47:12
47| ({x : 3} : interface {[number] : number}); // error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
interface.js:47:24
47| ({x : 3} : interface {[number] : number}); // error
^^^^^^ [2]
^^^^^^ [3]


Error ------------------------------------------------------------------------------------------------ interface.js:51:2
Expand Down
Loading

0 comments on commit 53c63f5

Please sign in to comment.