Skip to content

Commit 5651c48

Browse files
gabelevifacebook-github-bot
gabelevi
authored and
facebook-github-bot
committed
Constant folding array literal rest elements
Summary: This is a proof of concept for how I plan to revamp RestT. Rather than solving the case that I need, this tries to solve a simplier case: spreading tuples into array literals. For the RestT revamp, there are a bunch of things we need to be able to do * Assert that all rest elements are actually arrays (easy) * Extract the ArrT type info from rest elements (harder) * Treat tuple rest elements as multiple normal elements (hardest) The general idea here is to defer constructing the ArrT type until we've been able to resolve all the rest elements in an array literal. Then we can flatten tuples into normal elements (treat `[1, ...[2, 3], 4]` like `[1, 2, 3, 4]`. Since this IS constant folding, we need to be careful of infinite recursion. This diff takes avikchaudhuri's advice from {D3199590}, which is to keep track of the reasons which we've already seen and use duplicate reasons as an indicator of being in a loop. Reviewed By: samwgoldman Differential Revision: D4064784 fbshipit-source-id: 3dc0bd32fa26a9b0012256e877433311c2bd5138
1 parent 77ce66f commit 5651c48

File tree

13 files changed

+813
-107
lines changed

13 files changed

+813
-107
lines changed

‎newtests/array_literal_tuple_spread/_flowconfig

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[ignore]
2+
3+
[include]
4+
5+
[libs]
6+
7+
[options]
8+
all=true

‎newtests/array_literal_tuple_spread/test.js

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
/* @flow */
2+
3+
4+
import {suite, test} from '../../tsrc/test/Tester';
5+
6+
export default suite(({addFile, addFiles, addCode}) => [
7+
test("any flowing to spreads", [
8+
addCode(`
9+
function withoutAny(tup: [1,2], notAny: [3, 4]): [1, 2, 3, 4] {
10+
return [...tup, ...notAny];
11+
}
12+
function withAny(tup: [1,2], any: any): [1, 2, 3, 4] {
13+
return [...tup, ...any];
14+
}
15+
`)
16+
.noNewErrors()
17+
.because('Adding any should not cause new errors'),
18+
]),
19+
/* We used to try to "summarize" elements of non-tuple arrays, which would
20+
* strip away literal information from string and number types. However, this
21+
* was pretty broken, and Sam found this following example to demonstrate how.
22+
*/
23+
test("Sam's example of multiple lower bounds and SummarizeT", [
24+
addCode(`
25+
function f(b: boolean): [Array<?number>] {
26+
var x = null;
27+
if (b) {
28+
x = 0;
29+
}
30+
var [xs] = f(b);
31+
return [[...xs, x]];
32+
}
33+
`).noNewErrors()
34+
.because(
35+
'x has multiple lower bounds, so if we unify prematurely we can get ' +
36+
'an error when figuring out the summarized element type for x',
37+
),
38+
]),
39+
test('Avoid infinite recursion due to a loop', [
40+
addCode(`
41+
let foo = [0];
42+
for (let x = 1; x < 3; x++) {
43+
foo = [...foo, x];
44+
}
45+
(foo: [0, 1, 2]);
46+
`)
47+
.newErrors(
48+
`
49+
test.js:6
50+
6: foo = [...foo, x];
51+
^ number. Expected number literal \`1\`
52+
8: (foo: [0, 1, 2]);
53+
^ number literal \`1\`
54+
55+
test.js:6
56+
6: foo = [...foo, x];
57+
^ number. Expected number literal \`2\`
58+
8: (foo: [0, 1, 2]);
59+
^ number literal \`2\`
60+
61+
test.js:6
62+
6: foo = [...foo, x];
63+
^ number. Expected number literal \`2\`, got \`1\` instead
64+
8: (foo: [0, 1, 2]);
65+
^ number literal \`2\`
66+
67+
test.js:8
68+
8: (foo: [0, 1, 2]);
69+
^^^ array literal. Only tuples and array literals with known elements can flow to
70+
8: (foo: [0, 1, 2]);
71+
^^^^^^^^^ tuple type
72+
73+
test.js:8
74+
8: (foo: [0, 1, 2]);
75+
^^^ array literal. Tuple arity mismatch. This tuple has 1 elements and cannot flow to the 3 elements of
76+
8: (foo: [0, 1, 2]);
77+
^^^^^^^^^ tuple type
78+
79+
test.js:8
80+
8: (foo: [0, 1, 2]);
81+
^^^ array literal. Tuple arity mismatch. This tuple has 2 elements and cannot flow to the 3 elements of
82+
8: (foo: [0, 1, 2]);
83+
^^^^^^^^^ tuple type
84+
`,
85+
),
86+
]),
87+
test('Avoid infinite recursion due to polymorphic recursion', [
88+
addCode(`
89+
function foo<T: Array<*>>(arr: T) {
90+
if (arr.length > 10) return arr;
91+
return foo([...arr, 1]);
92+
}
93+
const ret = foo([1]);
94+
`),
95+
addCode('(ret: void);')
96+
.newErrors(
97+
`
98+
test.js:11
99+
11: (ret: void);
100+
^^^ array type. This type is incompatible with
101+
11: (ret: void);
102+
^^^^ undefined
103+
`,
104+
)
105+
.because('The constant folding should turn the tuple into an array'),
106+
addCode(`
107+
(ret[5]: 1);
108+
(ret[5]: 2);
109+
`)
110+
.newErrors(
111+
`
112+
test.js:6
113+
6: return foo([...arr, 1]);
114+
^ number. Expected number literal \`2\`, got \`1\` instead
115+
15: (ret[5]: 2);
116+
^ number literal \`2\`
117+
118+
test.js:8
119+
8: const ret = foo([1]);
120+
^ number. Expected number literal \`2\`, got \`1\` instead
121+
15: (ret[5]: 2);
122+
^ number literal \`2\`
123+
`,
124+
)
125+
.because('The element type should be `1`'),
126+
]),
127+
test('Avoid infinite recursion due to recursion', [
128+
addCode(`
129+
function foo(arr) {
130+
if (arr.length > 10) return arr;
131+
return foo([...arr, 1]);
132+
}
133+
const ret = foo([1]);
134+
`),
135+
addCode('(ret: void);')
136+
.newErrors(
137+
`
138+
test.js:11
139+
11: (ret: void);
140+
^^^ array literal. This type is incompatible with
141+
11: (ret: void);
142+
^^^^ undefined
143+
`,
144+
)
145+
.because('The constant folding should turn the tuple into an array'),
146+
addCode(`
147+
(ret[5]: 1);
148+
(ret[5]: 2);
149+
`)
150+
.newErrors(
151+
`
152+
test.js:8
153+
8: const ret = foo([1]);
154+
^ number. Expected number literal \`2\`, got \`1\` instead
155+
15: (ret[5]: 2);
156+
^ number literal \`2\`
157+
`,
158+
)
159+
.because('The element type should be `1`'),
160+
]),
161+
test('Spreading in a tuple should produce another tuple', [
162+
addCode(`
163+
var a = [2];
164+
var b = [4, 5];
165+
var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
166+
`).newErrors(
167+
`
168+
test.js:4
169+
4: var a = [2];
170+
^ number. Expected number literal \`20\`, got \`2\` instead
171+
6: var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
172+
^^ number literal \`20\`
173+
174+
test.js:6
175+
6: var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
176+
^ number. Expected number literal \`30\`, got \`3\` instead
177+
6: var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
178+
^^ number literal \`30\`
179+
180+
test.js:6
181+
6: var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
182+
^ number. Expected number literal \`60\`, got \`6\` instead
183+
6: var x: [1,20,30,4,5,60] = [1, ...a, 3, ...b, 6];
184+
^^ number literal \`60\`
185+
`,
186+
)
187+
]),
188+
test('Explicit union should become an explicit union', [
189+
addCode(`
190+
function test(arr: [1] | [2, 3]): [1, 10] | [2, 3, 10] {
191+
return [...arr, 10];
192+
}
193+
`).noNewErrors(),
194+
]),
195+
test('Non-polymorphic function', [
196+
addCode(`
197+
function foo(arr) {
198+
return [...arr, 1];
199+
}
200+
const ret1 = foo([2]);
201+
const ret2 = foo([3]);
202+
`)
203+
.noNewErrors(),
204+
addCode('(ret1[0]: 2);')
205+
.newErrors(
206+
`
207+
test.js:8
208+
8: const ret2 = foo([3]);
209+
^ number. Expected number literal \`2\`, got \`3\` instead
210+
11: (ret1[0]: 2);
211+
^ number literal \`2\`
212+
`,
213+
)
214+
.because('Flow infers the return type to [2,1] | [3,1]'),
215+
addCode('(ret2[0]: 3);')
216+
.newErrors(
217+
`
218+
test.js:7
219+
7: const ret1 = foo([2]);
220+
^ number. Expected number literal \`3\`, got \`2\` instead
221+
13: (ret2[0]: 3);
222+
^ number literal \`3\`
223+
`,
224+
)
225+
.because('Flow infers the return type to [2,1] | [3,1]'),
226+
]),
227+
test('Flowing a non-array to a rest element', [
228+
addCode(`
229+
const num = 123;
230+
const tuple = [...num];
231+
`).newErrors(
232+
`
233+
test.js:5
234+
5: const tuple = [...num];
235+
^^^^^^^^ array literal. Expected rest element to be an array or tuple instead of
236+
5: const tuple = [...num];
237+
^^^ number
238+
`,
239+
),
240+
]),
241+
test('Spreading an Array<T> should result in a non-tuple array', [
242+
addCode(`
243+
const tup: Array<number> = [1,2,3];
244+
const nonTup = [...tup];
245+
(nonTup: [1,2,3]);
246+
`).newErrors(
247+
`
248+
test.js:6
249+
6: (nonTup: [1,2,3]);
250+
^^^^^^ array literal. Only tuples and array literals with known elements can flow to
251+
6: (nonTup: [1,2,3]);
252+
^^^^^^^ tuple type
253+
`,
254+
)
255+
]),
256+
test('Spreading a $ReadOnlyArray should result in a non-tuple array', [
257+
addCode(`
258+
const tup: $ReadOnlyArray<number> = [1,2,3];
259+
const nonTup = [...tup];
260+
(nonTup: [1,2,3]);
261+
`).newErrors(
262+
`
263+
test.js:6
264+
6: (nonTup: [1,2,3]);
265+
^^^^^^ array literal. Only tuples and array literals with known elements can flow to
266+
6: (nonTup: [1,2,3]);
267+
^^^^^^^ tuple type
268+
`,
269+
)
270+
]),
271+
]);

‎src/common/reason.ml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,3 +623,8 @@ let repos_reason loc reason =
623623

624624
let update_origin_of_reason origin reason =
625625
{ reason with origin = origin }
626+
627+
module ReasonSet = Set.Make(struct
628+
type t = reason
629+
let compare = Pervasives.compare
630+
end)

‎src/common/reason.mli

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,5 @@ val repos_reason: Loc.t -> reason -> reason
211211
val update_origin_of_reason: reason option -> reason -> reason
212212

213213
val do_patch: string list -> (int * int * string) list -> string
214+
215+
module ReasonSet: Set.S with type elt = reason

‎src/typing/debug_js.ml

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,10 +367,6 @@ and _json_of_use_t_impl json_cx t = Hh_json.(
367367
"result", _json_of_t json_cx t
368368
]
369369

370-
| SummarizeT (_, t) -> [
371-
"type", _json_of_t json_cx t
372-
]
373-
374370
| BindT (_, funtype)
375371
| CallT (_, funtype) -> [
376372
"funType", json_of_funcalltype json_cx funtype
@@ -688,6 +684,45 @@ and _json_of_use_t_impl json_cx t = Hh_json.(
688684
("refined_t", _json_of_t_impl json_cx t)
689685
]
690686
]
687+
| ResolveRestT (_, {
688+
rrt_id;
689+
rrt_resolved;
690+
rrt_unresolved;
691+
rrt_resolve_to;
692+
rrt_tout;
693+
}) -> [
694+
"id", JSON_Number (string_of_int rrt_id);
695+
"resolved", JSON_Array (List.map (fun param ->
696+
let kind, t = match param with
697+
| ResolvedParam t -> "ResolvedParam", t
698+
| ResolvedRestParam (at) ->
699+
"ResolvedRestParam", ArrT (locationless_reason (RCustom "array"), at)
700+
| ResolvedAnyRestParam ->
701+
"ResolvedAnyRestParam", AnyT (locationless_reason (RAny))
702+
in
703+
JSON_Object [
704+
"kind", JSON_String kind;
705+
"type", _json_of_t_impl json_cx t;
706+
]
707+
) rrt_resolved);
708+
"unresolved", JSON_Array (List.map (fun param ->
709+
let kind, t = match param with
710+
| UnresolvedParam t -> "UnresolvedParam", t
711+
| UnresolvedRestParam t -> "UnresolvedRestParam", t in
712+
JSON_Object [
713+
"kind", JSON_String kind;
714+
"type", _json_of_t_impl json_cx t;
715+
]
716+
) rrt_unresolved);
717+
"resolve_to", JSON_Object (
718+
let kind = match rrt_resolve_to with
719+
| ResolveSpreadsToTuple -> "ResolveSpreadsToTuple"
720+
| ResolveSpreadsToArray -> "ResolveSpreadsToArray"
721+
| ResolveSpreadsToArrayLiteral -> "ResolveSpreadsToArrayLiteral" in
722+
[ "kind", JSON_String kind ]
723+
);
724+
"t_out", _json_of_t json_cx rrt_tout;
725+
]
691726
)
692727
)
693728

@@ -1518,6 +1553,12 @@ and dump_use_t_ (depth, tvars) cx t =
15181553
| RefineT _ -> p t
15191554
| ReposLowerT (_, arg) -> p ~extra:(use_kid arg) t
15201555
| ReposUseT (_, _, arg) -> p ~extra:(kid arg) t
1556+
| ResolveRestT (_, {rrt_resolve_to; rrt_tout; _;}) ->
1557+
p ~extra:(spf "%s, %s" (match rrt_resolve_to with
1558+
| ResolveSpreadsToTuple -> "ResolveSpreadsToTuple"
1559+
| ResolveSpreadsToArrayLiteral -> "ResolveSpreadsToArrayLiteral"
1560+
| ResolveSpreadsToArray -> "ResolveSpreadsToArray")
1561+
(kid rrt_tout)) t
15211562
| SentinelPropTestT (l, sense, sentinel, result) -> p ~reason:false
15221563
~extra:(spf "%s, %b, %s, %s"
15231564
(kid l)
@@ -1531,7 +1572,6 @@ and dump_use_t_ (depth, tvars) cx t =
15311572
(kid result))
15321573
t
15331574
| SubstOnPredT _ -> p t
1534-
| SummarizeT (_, arg) -> p ~extra:(kid arg) t
15351575
| SuperT _ -> p t
15361576
| ImplementsT arg -> p ~reason:false ~extra:(kid arg) t
15371577
| SetElemT (_, ix, etype) -> p ~extra:(spf "%s, %s" (kid ix) (kid etype)) t

‎src/typing/flow_error.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ let rec error_of_msg ~trace_reasons ~op ~source_file =
229229
| ObjectMapi -> "Expected object instead of")
230230
| ReactCreateElementT _ -> "Expected React component instead of"
231231
| CallLatentPredT _ -> "Expected predicated function instead of"
232+
| ResolveRestT _ ->
233+
"Expected rest element to be an array or tuple instead of"
232234
| TypeAppVarianceCheckT _ -> "Expected polymorphic type instead of"
233235
(* unreachable or unclassified use-types. until we have a mechanical way
234236
to verify that all legit use types are listed above, we can't afford

0 commit comments

Comments
 (0)