Skip to content

Commit

Permalink
[flow][numeric-keys] Add support for int-like property keys (#7593)
Browse files Browse the repository at this point in the history
Summary:
Our previous handling of numeric property keys is very unsafe. We add support for treating int-like numbers (less than the max safe integer) as property keys. We treat these numbers as they are treated at runtime - we convert them to strings. See post for more. Also see previous diff, as it adds some of the utilities used here.

Fixes #380 (#380 from 2015!)
Closes #7593

Changelog: [errors] You can now create and access object keys with int-like number literals, these act as their string equivalents.

Reviewed By: SamChou19815

Differential Revision: D51567388

fbshipit-source-id: de8f4ba906aaee0a4b881b087aedbf45d9c1c365
  • Loading branch information
gkz authored and facebook-github-bot committed Dec 4, 2023
1 parent 3032229 commit 80ca16f
Show file tree
Hide file tree
Showing 21 changed files with 740 additions and 399 deletions.
10 changes: 8 additions & 2 deletions src/typing/flow_js_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1911,7 +1911,7 @@ module GetPropT_kit (F : Get_prop_helper_sig) = struct
| (Some prop, _, _) -> Some (prop, PropertyMapProperty)
| (None, Named { name; from_indexed_access = true; _ }, Some { key; value; dict_polarity; _ })
when not ignore_dicts ->
F.dict_read_check cx trace ~use_op (string_key name reason_op, key);
F.dict_read_check cx trace ~use_op (type_of_key_name cx name reason_op, key);
Some
( Field
{ preferred_def_locs = None; key_loc = None; type_ = value; polarity = dict_polarity },
Expand Down Expand Up @@ -2005,7 +2005,7 @@ module GetPropT_kit (F : Get_prop_helper_sig) = struct
| (Named { name; _ }, None, Some { key; value; dict_polarity; _ })
when not (is_dictionary_exempt name) ->
(* Dictionaries match all property reads *)
F.dict_read_check cx trace ~use_op:unknown_use (string_key name reason_op, key);
F.dict_read_check cx trace ~use_op:unknown_use (type_of_key_name cx name reason_op, key);
Some (OrdinaryField { type_ = value; polarity = dict_polarity }, IndexerProperty)
| (Computed k, None, Some { key; value; dict_polarity; _ }) ->
F.dict_read_check cx trace ~use_op:unknown_use (k, key);
Expand Down Expand Up @@ -2175,6 +2175,12 @@ let propref_for_elem_t = function
| DefT (reason, StrT (Literal (_, name))) ->
let reason = replace_desc_reason (RProperty (Some name)) reason in
mk_named_prop ~reason ~from_indexed_access:true name
| GenericT { bound = DefT (_, NumT (Literal (_, (value, raw)))); reason = reason_num; _ }
| DefT (reason_num, NumT (Literal (_, (value, raw))))
when Js_number.is_float_safe_integer value ->
let reason = replace_desc_reason (RProperty (Some (OrdinaryName raw))) reason_num in
let name = OrdinaryName (Dtoa.ecma_string_of_float value) in
mk_named_prop ~reason ~from_indexed_access:true name
| l -> Computed l

let keylist_of_props props reason_op =
Expand Down
10 changes: 5 additions & 5 deletions tests/annotate_exports_object/annotate_exports_object.exp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function foo() {
};
}

export const foo_: {| "": number, "-": number, "\ud83d\ude0a": number |} = foo();
export const foo_: {| "": number, "-": number, "1": number, "\ud83d\ude0a": number |} = foo();

declare var b: {
'-': 1,
Expand All @@ -74,7 +74,7 @@ export const bar_: { [1]: 4, "": 2, "-": 1, "\ud83d\ude0a": 3 } = bar();
Stats:
Files changed: 4
Number of annotations added: 5
Total size of annotations: 17
Total size of annotations: 18
Number of sig. ver. errors: 5
Number of annotations required: 5
Number of annotations skipped: 0
Expand Down Expand Up @@ -153,7 +153,7 @@ function foo() {
};
}

export const foo_: { "": number, "-": number, "\ud83d\ude0a": number, ... } = foo();
export const foo_: { "": number, "-": number, "1": number, "\ud83d\ude0a": number, ... } = foo();

declare var b: {
'-': 1,
Expand Down Expand Up @@ -183,7 +183,7 @@ export const bar_: { [1]: 4, "": 2, "-": 1, "\ud83d\ude0a": 3 } = bar();

>>> ./d.js
12c12
< export const foo_: {| "": number, "-": number, "\ud83d\ude0a": number |} = foo();
< export const foo_: {| "": number, "-": number, "1": number, "\ud83d\ude0a": number |} = foo();
---
> export const foo_: { "": number, "-": number, "\ud83d\ude0a": number, ... } = foo();
> export const foo_: { "": number, "-": number, "1": number, "\ud83d\ude0a": number, ... } = foo();

30 changes: 29 additions & 1 deletion tests/component_syntax/component_syntax.exp
Original file line number Diff line number Diff line change
Expand Up @@ -2770,6 +2770,34 @@ You may only use an identifier or a destructured object as a component rest para
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Error ---------------------------------------------------------------------------------------- rest_constraints.js:13:31

Property `0` is missing in object type [1]. [prop-missing]

rest_constraints.js:13:31
13| component DestructuredArr(...[foo, bar]: {foo: number, bar: number}) { // ERROR destructure array
^^^

References:
rest_constraints.js:13:42
13| component DestructuredArr(...[foo, bar]: {foo: number, bar: number}) { // ERROR destructure array
^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]


Error ---------------------------------------------------------------------------------------- rest_constraints.js:13:36

Property `1` is missing in object type [1]. [prop-missing]

rest_constraints.js:13:36
13| component DestructuredArr(...[foo, bar]: {foo: number, bar: number}) { // ERROR destructure array
^^^

References:
rest_constraints.js:13:42
13| component DestructuredArr(...[foo, bar]: {foo: number, bar: number}) { // ERROR destructure array
^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]


Error ---------------------------------------------------------------------------------------- rest_constraints.js:17:42

You may only use an identifier or a destructured object as a component rest param. [invalid-component-prop]
Expand Down Expand Up @@ -3101,7 +3129,7 @@ Strict mode function may not have duplicate parameter names



Found 203 errors
Found 205 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
40 changes: 37 additions & 3 deletions tests/contextual_typing/contextual_typing.exp
Original file line number Diff line number Diff line change
Expand Up @@ -1061,19 +1061,53 @@ 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].

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

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


Error --------------------------------------------------------------------------------------------------- unions.js:4:55

An annotation on `v` is required because Flow cannot infer its type from local context. [missing-local-annot]

4| const x: {['A' | 'B']: (number) => number} = {[42]: v => v};
4| const x: {['A' | 'B']: (number) => number} = {[42]: v => v}; // ERROR
^


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]

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

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


Error --------------------------------------------------------------------------------------------------- unions.js:7:49

An annotation on `v` is required because Flow cannot infer its type from local context. [missing-local-annot]

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


Expand Down Expand Up @@ -1171,4 +1205,4 @@ References:



Found 84 errors
Found 86 errors
4 changes: 2 additions & 2 deletions tests/contextual_typing/unions.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//@flow

{
const x: {['A' | 'B']: (number) => number} = {[42]: v => v};
const x: {['A' | 'B']: (number) => number} = {[42]: v => v}; // ERROR
}
{
const x: {['A']: (number) => number} = {[42]: v => v};
const x: {['A']: (number) => number} = {[42]: v => v}; // ERROR
}
{
const x: {...} | {...} = {a: v => v};
Expand Down
47 changes: 46 additions & 1 deletion tests/destructuring/destructuring.exp
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,51 @@ References:
^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- tuple.js:5:4

Cannot cast `a` to empty because string [1] is incompatible with empty [2]. [incompatible-cast]

tuple.js:5:4
5| (a: empty); // ERROR
^

References:
tuple.js:2:21
2| declare const x: [string];
^^^^^^ [1]
tuple.js:5:7
5| (a: empty); // ERROR
^^^^^ [2]


Error --------------------------------------------------------------------------------------------------- tuple.js:11:10

Property `0` is missing in object type [1]. [prop-missing]

tuple.js:11:10
11| const [x] = o; // ERROR
^

References:
tuple.js:10:20
10| declare const o: {foo: 1};
^^^^^^^^ [1]


Error --------------------------------------------------------------------------------------------------- tuple.js:16:10

Property `0` is missing in statics of function type [1]. [prop-missing]

tuple.js:16:10
16| const [x] = f; // ERROR
^

References:
tuple.js:15:20
15| declare const f: number => boolean;
^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- unannotated.js:6:5

Cannot get `x.bar` because property `bar` is missing in object literal [1]. [prop-missing]
Expand All @@ -1477,7 +1522,7 @@ References:



Found 103 errors
Found 106 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
17 changes: 17 additions & 0 deletions tests/destructuring/tuple.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
declare const x: [string];
const [a] = x;
(a: string); // OK
(a: empty); // ERROR
}

// Non-arrays/tuples
{
declare const o: {foo: 1};
const [x] = o; // ERROR
}

{
declare const f: number => boolean;
const [x] = f; // ERROR
}
Loading

0 comments on commit 80ca16f

Please sign in to comment.