Skip to content

Array spread syntax support #6608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
- Experimental support of tagged template literals, e.g. ```sql`select * from ${table}```. https://github.com/rescript-lang/rescript-compiler/pull/6250
- Experimental support for generic/custom JSX transforms. https://github.com/rescript-lang/rescript-compiler/pull/6565
- `dict` is now a builtin type. https://github.com/rescript-lang/rescript-compiler/pull/6590
- Add support for array spread. https://github.com/rescript-lang/rescript-compiler/pull/6608

#### :bug: Bug Fix

Expand Down
78 changes: 49 additions & 29 deletions jscomp/syntax/src/res_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ module ErrorMessages = struct
+ Array size check + `get` checks on the current pattern. If it's to \
obtain a subarray, use `Array.sub` or `Belt.Array.slice`."

let arrayExprSpread =
"Arrays can't use the `...` spread currently. Please use `concat` or other \
Array helpers."

let recordExprSpread =
"Records can only have one `...` spread, at the beginning.\n\
Explanation: since records have a known, fixed shape, a spread like `{a, \
Expand Down Expand Up @@ -3920,36 +3916,60 @@ and parseListExpr ~startPos p =
loc))
[(Asttypes.Nolabel, Ast_helper.Exp.array ~loc listExprs)]

(* Overparse ... and give a nice error message *)
and parseNonSpreadExp ~msg p =
let () =
match p.Parser.token with
| DotDotDot ->
Parser.err p (Diagnostics.message msg);
Parser.next p
| _ -> ()
in
match p.Parser.token with
| token when Grammar.isExprStart token -> (
let expr = parseExpr p in
match p.Parser.token with
| Colon ->
Parser.next p;
let typ = parseTypExpr p in
let loc = mkLoc expr.pexp_loc.loc_start typ.ptyp_loc.loc_end in
Some (Ast_helper.Exp.constraint_ ~loc expr typ)
| _ -> Some expr)
| _ -> None

and parseArrayExp p =
let startPos = p.Parser.startPos in
Parser.expect Lbracket p;
let exprs =
parseCommaDelimitedRegion p ~grammar:Grammar.ExprList ~closing:Rbracket
~f:(parseNonSpreadExp ~msg:ErrorMessages.arrayExprSpread)
let split_by_spread exprs =
List.fold_left
(fun acc curr ->
match (curr, acc) with
| (true, expr, startPos, endPos), _ ->
(* find a spread expression, prepend a new sublist *)
([], Some expr, startPos, endPos) :: acc
| ( (false, expr, startPos, _endPos),
(no_spreads, spread, _accStartPos, accEndPos) :: acc ) ->
(* find a non-spread expression, and the accumulated is not empty,
* prepend to the first sublist, and update the loc of the first sublist *)
(expr :: no_spreads, spread, startPos, accEndPos) :: acc
| (false, expr, startPos, endPos), [] ->
(* find a non-spread expression, and the accumulated is empty *)
[([expr], None, startPos, endPos)])
[] exprs
in
let listExprsRev =
parseCommaDelimitedReversedList p ~grammar:Grammar.ExprList
~closing:Rbracket ~f:parseSpreadExprRegionWithLoc
in
Parser.expect Rbracket p;
Ast_helper.Exp.array ~loc:(mkLoc startPos p.prevEndPos) exprs
let loc = mkLoc startPos p.prevEndPos in
let collectExprs = function
| [], Some spread, _startPos, _endPos -> [spread]
| exprs, Some spread, _startPos, _endPos ->
let els = Ast_helper.Exp.array ~loc exprs in
[els; spread]
| exprs, None, _startPos, _endPos ->
let els = Ast_helper.Exp.array ~loc exprs in
[els]
in
match split_by_spread listExprsRev with
| [] -> Ast_helper.Exp.array ~loc:(mkLoc startPos p.prevEndPos) []
| [(exprs, None, _, _)] ->
Ast_helper.Exp.array ~loc:(mkLoc startPos p.prevEndPos) exprs
| exprs ->
let xs = List.map collectExprs exprs in
let listExprs =
List.fold_right
(fun exprs1 acc ->
List.fold_right (fun expr1 acc1 -> expr1 :: acc1) exprs1 acc)
xs []
in
Ast_helper.Exp.apply ~loc
(Ast_helper.Exp.ident ~loc ~attrs:[spreadAttr]
(Location.mkloc
(Longident.Ldot
(Longident.Ldot (Longident.Lident "Belt", "Array"), "concatMany"))
loc))
[(Asttypes.Nolabel, Ast_helper.Exp.array ~loc listExprs)]

(* TODO: check attributes in the case of poly type vars,
* might be context dependend: parseFieldDeclaration (see ocaml) *)
Expand Down
16 changes: 16 additions & 0 deletions jscomp/syntax/src/res_parsetree_viewer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ let hasAwaitAttribute attrs =
| _ -> false)
attrs

let collectArrayExpressions expr =
match expr.pexp_desc with
| Pexp_array exprs -> (exprs, None)
| _ -> ([], Some expr)

let collectListExpressions expr =
let rec collect acc expr =
match expr.pexp_desc with
Expand Down Expand Up @@ -678,6 +683,17 @@ let isSpreadBeltListConcat expr =
hasSpreadAttr expr.pexp_attributes
| _ -> false

let isSpreadBeltArrayConcat expr =
match expr.pexp_desc with
| Pexp_ident
{
txt =
Longident.Ldot
(Longident.Ldot (Longident.Lident "Belt", "Array"), "concatMany");
} ->
hasSpreadAttr expr.pexp_attributes
| _ -> false

(* Blue | Red | Green -> [Blue; Red; Green] *)
let collectOrPatternChain pat =
let rec loop pattern chain =
Expand Down
6 changes: 6 additions & 0 deletions jscomp/syntax/src/res_parsetree_viewer.mli
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ val collectIfExpressions :
(Location.t * ifConditionKind * Parsetree.expression) list
* Parsetree.expression option

val collectArrayExpressions :
Parsetree.expression ->
Parsetree.expression list * Parsetree.expression option

val collectListExpressions :
Parsetree.expression ->
Parsetree.expression list * Parsetree.expression option
Expand Down Expand Up @@ -142,6 +146,8 @@ val hasTemplateLiteralAttr : Parsetree.attributes -> bool

val isSpreadBeltListConcat : Parsetree.expression -> bool

val isSpreadBeltArrayConcat : Parsetree.expression -> bool

val collectOrPatternChain : Parsetree.pattern -> Parsetree.pattern list

val processBracesAttr :
Expand Down
58 changes: 58 additions & 0 deletions jscomp/syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3046,6 +3046,9 @@ and printExpression ~state (e : Parsetree.expression) cmtTbl =
Doc.rbrace;
])
| extension -> printExtension ~state ~atModuleLvl:false extension cmtTbl)
| Pexp_apply (e, [(Nolabel, {pexp_desc = Pexp_array subLists})])
when ParsetreeViewer.isSpreadBeltArrayConcat e ->
printBeltArrayConcatApply ~state subLists cmtTbl
| Pexp_apply (e, [(Nolabel, {pexp_desc = Pexp_array subLists})])
when ParsetreeViewer.isSpreadBeltListConcat e ->
printBeltListConcatApply ~state subLists cmtTbl
Expand Down Expand Up @@ -3813,6 +3816,61 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
])
| _ -> Doc.nil

and printBeltArrayConcatApply ~state subLists cmtTbl =
let makeSpreadDoc commaBeforeSpread = function
| Some expr ->
Doc.concat
[
commaBeforeSpread;
Doc.dotdotdot;
(let doc = printExpressionWithComments ~state expr cmtTbl in
match Parens.expr expr with
| Parens.Parenthesized -> addParens doc
| Braced braces -> printBraces doc expr braces
| Nothing -> doc);
]
| None -> Doc.nil
in
let makeSubListDoc (expressions, spread) =
let commaBeforeSpread =
match expressions with
| [] -> Doc.nil
| _ -> Doc.concat [Doc.text ","; Doc.line]
in
let spreadDoc = makeSpreadDoc commaBeforeSpread spread in
Doc.concat
[
Doc.join
~sep:(Doc.concat [Doc.text ","; Doc.line])
(List.map
(fun expr ->
let doc = printExpressionWithComments ~state expr cmtTbl in
match Parens.expr expr with
| Parens.Parenthesized -> addParens doc
| Braced braces -> printBraces doc expr braces
| Nothing -> doc)
expressions);
spreadDoc;
]
in
Doc.group
(Doc.concat
[
Doc.lbracket;
Doc.indent
(Doc.concat
[
Doc.softLine;
Doc.join
~sep:(Doc.concat [Doc.text ","; Doc.line])
(List.map makeSubListDoc
(List.map ParsetreeViewer.collectArrayExpressions subLists));
]);
Doc.trailingComma;
Doc.softLine;
Doc.rbracket;
])

and printBeltListConcatApply ~state subLists cmtTbl =
let makeSpreadDoc commaBeforeSpread = function
| Some expr ->
Expand Down
68 changes: 28 additions & 40 deletions jscomp/syntax/tests/parsing/errors/other/expected/spread.res.txt
Original file line number Diff line number Diff line change
@@ -1,79 +1,67 @@

Syntax error!
tests/parsing/errors/other/spread.res:1:12-14
tests/parsing/errors/other/spread.res:1:6-8

1 │ let arr = [...x, ...y]
2 │ let [...arr, _] = [1, 2, 3]
3 │

Arrays can't use the `...` spread currently. Please use `concat` or other Array helpers.


Syntax error!
tests/parsing/errors/other/spread.res:2:6-8

1 │ let arr = [...x, ...y]
2 │ let [...arr, _] = [1, 2, 3]
3 │
4 │ let record = {...x, ...y}
1 │ let [...arr, _] = [1, 2, 3]
2 │
3 │ let record = {...x, ...y}

Array's `...` spread is not supported in pattern matches.
Explanation: such spread would create a subarray; out of performance concern, our pattern matching currently guarantees to never create new intermediate data.
Solution: if it's to validate the first few elements, use a `when` clause + Array size check + `get` checks on the current pattern. If it's to obtain a subarray, use `Array.sub` or `Belt.Array.slice`.


Syntax error!
tests/parsing/errors/other/spread.res:4:21-23
tests/parsing/errors/other/spread.res:3:21-23

2 │ let [...arr, _] = [1, 2, 3]
3
4 │ let record = {...x, ...y}
5 │ let {...x, ...y} = myRecord
6
1 │ let [...arr, _] = [1, 2, 3]
2
3 │ let record = {...x, ...y}
4 │ let {...x, ...y} = myRecord
5

Records can only have one `...` spread, at the beginning.
Explanation: since records have a known, fixed shape, a spread like `{a, ...b}` wouldn't make sense, as `b` would override every field of `a` anyway.


Syntax error!
tests/parsing/errors/other/spread.res:5:15-18
tests/parsing/errors/other/spread.res:4:15-18

3
4 │ let record = {...x, ...y}
5 │ let {...x, ...y} = myRecord
6
7 │ let list{...x, ...y} = myList
2
3 │ let record = {...x, ...y}
4 │ let {...x, ...y} = myRecord
5
6 │ let list{...x, ...y} = myList

Record's `...` spread is not supported in pattern matches.
Explanation: you can't collect a subset of a record's field into its own record, since a record needs an explicit declaration and that subset wouldn't have one.
Solution: you need to pull out each field you want explicitly.


Syntax error!
tests/parsing/errors/other/spread.res:7:13-22
tests/parsing/errors/other/spread.res:6:13-22

5 │ let {...x, ...y} = myRecord
6
7 │ let list{...x, ...y} = myList
8
9 │ type t = {...a}
4 │ let {...x, ...y} = myRecord
5
6 │ let list{...x, ...y} = myList
7
8 │ type t = {...a}

List pattern matches only supports one `...` spread, at the end.
Explanation: a list spread at the tail is efficient, but a spread in the middle would create new lists; out of performance concern, our pattern matching currently guarantees to never create new intermediate data.


Syntax error!
tests/parsing/errors/other/spread.res:10:20
tests/parsing/errors/other/spread.res:9:20

8
9 │ type t = {...a}
10 │ type t = Foo({...a})
11 │ type t = option<foo, {...x}>
12
7
8 │ type t = {...a}
9 │ type t = Foo({...a})
10 │ type t = option<foo, {...x}>
11

I'm not sure what to parse here when looking at ")".

let arr = [|x;y|]
let [|arr;_|] = [|1;2;3|]
let record = { x with y }
let { x; y } = myRecord
Expand Down
1 change: 0 additions & 1 deletion jscomp/syntax/tests/parsing/errors/other/spread.res
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
let arr = [...x, ...y]
let [...arr, _] = [1, 2, 3]

let record = {...x, ...y}
Expand Down
15 changes: 15 additions & 0 deletions jscomp/syntax/tests/parsing/grammar/expressions/array.res
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,18 @@ let x = [1, 2, 3,]

// with constrained expressions
let x = [1 :int, (2: int), 3 : int]

// spread
let x = [4, 5, ...y]

// spread anywhere
let x = [4, 5, ...y, 7, ...y]

// spread constrained expressions
let x = [4, 5, ...y: array<int>]

// spread with other variable
let x = [4, 5, k, ...y]

// the only spread
let x = [...y]
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
let x = [|1;2;3|]
let x = [|1;2;3|]
let x = [|(1 : int);(2 : int);(3 : int)|]
let x = [|(1 : int);(2 : int);(3 : int)|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];y;[|7|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];(y : int array)|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5;k|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|y|]
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ let flags =
let rec loop items =
((match items with
| [|{js|-pp|js};_ppFlag;rest|] -> loop rest
| [|x;rest|] -> [|x;(loop rest)|]
| [|x;rest|] ->
((Belt.Array.concatMany)[@res.spread ]) [|[|x|];(loop rest)|]
| [||] -> [||])
[@res.braces ]) in
(loop parts) |> (String.concat {js| |js}))
Expand Down