diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f4856d2..48f568da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ - Fix printing of comments inside JSX tag https://github.com/rescript-lang/syntax/pull/664 - Fix issue where formatter erases tail comments inside JSX tag https://github.com/rescript-lang/syntax/issues/663 - Fix issue where the JSX prop has type annotation of the first class module https://github.com/rescript-lang/syntax/pull/666 +- Fix issue where a spread `...x` in non-last position would not be reported as syntax error https://github.com/rescript-lang/syntax/pull/673/ #### :eyeglasses: Spec Compliance diff --git a/src/res_core.ml b/src/res_core.ml index 49d075c5..094307ec 100644 --- a/src/res_core.ml +++ b/src/res_core.ml @@ -50,7 +50,7 @@ module ErrorMessages = struct let listPatternSpread = "List pattern matches only supports one `...` spread, at the end.\n\ Explanation: a list spread at the tail is efficient, but a spread in the \ - middle would create new list[s]; out of performance concern, our pattern \ + middle would create new lists; out of performance concern, our pattern \ matching currently guarantees to never create new intermediate data." let recordPatternSpread = @@ -84,8 +84,8 @@ module ErrorMessages = struct let listExprSpread = "Lists can only have one `...` spread, and at the end.\n\ Explanation: lists are singly-linked list, where a node contains a value \ - and points to the next node. `list[a, ...bc]` efficiently creates a new \ - item and links `bc` as its next nodes. `[...bc, a]` would be expensive, \ + and points to the next node. `list{a, ...bc}` efficiently creates a new \ + item and links `bc` as its next nodes. `list{...bc, a}` would be expensive, \ as it'd need to traverse `bc` and prepend each item to `a` one by one. We \ therefore disallow such syntax sugar.\n\ Solution: directly use `concat`." @@ -3716,25 +3716,27 @@ and parseSpreadExprRegion p = | _ -> None and parseListExpr ~startPos p = - let listExprs = + let check_all_non_spread_exp exprs = + exprs + |> List.map (fun (spread, expr) -> + if spread then + Parser.err p (Diagnostics.message ErrorMessages.listExprSpread); + expr) + |> List.rev + in + let listExprsRev = parseCommaDelimitedReversedList p ~grammar:Grammar.ListExpr ~closing:Rbrace ~f:parseSpreadExprRegion in Parser.expect Rbrace p; let loc = mkLoc startPos p.prevEndPos in - match listExprs with - | (true, expr) :: exprs -> - let exprs = exprs |> List.map snd |> List.rev in + match listExprsRev with + | (true, (* spread expression *) + expr) :: exprs -> + let exprs = check_all_non_spread_exp exprs in makeListExpression loc exprs (Some expr) | exprs -> - let exprs = - exprs - |> List.map (fun (spread, expr) -> - if spread then - Parser.err p (Diagnostics.message ErrorMessages.listExprSpread); - expr) - |> List.rev - in + let exprs = check_all_non_spread_exp exprs in makeListExpression loc exprs None (* Overparse ... and give a nice error message *) diff --git a/tests/parsing/errors/other/expected/spread.res.txt b/tests/parsing/errors/other/expected/spread.res.txt index cfddb204..b6b0b914 100644 --- a/tests/parsing/errors/other/expected/spread.res.txt +++ b/tests/parsing/errors/other/expected/spread.res.txt @@ -49,6 +49,20 @@ Explanation: you can't collect a subset of a record's field into its own record, Solution: you need to pull out each field you want explicitly. + Syntax error! + tests/parsing/errors/other/spread.res:8:1-3 + + 6 │ + 7 │ let myList = list{...x, ...y} + 8 │ let list{...x, ...y} = myList + 9 │ + 10 │ type t = {...a} + + Lists can only have one `...` spread, and at the end. +Explanation: lists are singly-linked list, where a node contains a value and points to the next node. `list{a, ...bc}` efficiently creates a new item and links `bc` as its next nodes. `list{...bc, a}` would be expensive, as it'd need to traverse `bc` and prepend each item to `a` one by one. We therefore disallow such syntax sugar. +Solution: directly use `concat`. + + Syntax error! tests/parsing/errors/other/spread.res:8:13-22 @@ -59,7 +73,7 @@ Solution: you need to pull out each field you want explicitly. 10 │ 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 list[s]; out of performance concern, our pattern matching currently guarantees to never create new intermediate data. +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!