From b5eecac4b1ffc9d75a58ec44f95ceb66079e41e1 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 18 Oct 2026 18:22:50 +0000 Subject: [PATCH 1/5] Represent array and list literals with spreads with a Pexp_spread node Why: The parser turned [a, ...b] and list{...a, b} into calls of Primitive_array.spread and Primitive_list.spread marked with res.spread, so the formatter had to rebuild the literal from the call and got it wrong: spreads of literals were flattened, attributes on the literal were dropped and an empty list spread printed as an empty element. Pexp_spread keeps the items and spreads in source order, and the type checker gives it a Texp_spread node. Translation emits new Parray_spread and Plist_spread primitives that compile to the same runtime calls. A list literal whose only spread is its tail stays a chain of ::. The parsetree and typed tree layouts change; the release bumps their magic numbers. AST modified (array literal with a spread): Example: let x = [1, ...ys] Before (parse): expression Pexp_apply expression attribute "res.spread" Pexp_ident "Primitive_array.spread" [ Nolabel expression Pexp_array [ expression Pexp_array [ expression Pexp_constant PConst_int (1,None) ] expression Pexp_ident "ys" ] ] After (parse): expression Pexp_spread array [ Pspread_item expression Pexp_constant PConst_int (1,None) Pspread expression Pexp_ident "ys" ] Typedtree added (Texp_spread): Example: let ys = [2] let x = [1, ...ys] After (typed): expression Texp_spread array [ expression Texp_array [ expression Texp_constant Const_int 1 ] expression Texp_ident "ys/1001" ] Format modified (literals with spreads): Example: let x = [...[1, 2], 3, ...[]] let y = @attr list{...xs, ...list{}, 1} Before (rescript): let x = [1, 2, 3, ] let y = list{...xs, , 1} After (rescript): let x = [...[1, 2], 3, ...[]] let y = @attr list{...xs, ...list{}, 1} Review: Why do the generated item groups have ghost locations? The v0 encoding has no marker on them, and the ghost flag tells a group of items from an array or list literal that is spread. A call built by a PPX with real locations decodes to spreads only, which concatenates the same values. Review: Why does the typed node hold the concatenated parts rather than the elements? The parts are the arrays and cons chains the runtime call takes, so the items keep the errors of array and list literals and translation needs no list building of its own. Area: syntax printer typechecker lambda codegen analysis reanalyze tests Output: unchanged, tests/tests JS byte-identical after a rebuild; a combinatorial harness of all item/spread sequences up to length 4 compiles to identical JS with the old and new bsc Formatting: changed spreads of array and list literals and attributes on literals with spreads are kept; comments in list spreads stay in place Formats: ast cmt (layouts changed; magic numbers left to yarn changelog:release) PPX: unchanged wire: the v0 encoding is the res.spread call it was, decoded back to Pexp_spread (ast-mapping fixture SpreadLiterals.res) Editor: unchanged: pipe completion types the literal as the runtime call; SpreadLiterals.res covers AST dump, hints, highlighting and completion Tests: +tests/syntax_tests/data/ast-mapping/SpreadLiterals.res +tests/analysis_tests/tests/src/SpreadLiterals.res +tests/build_tests/super_errors/fixtures/array_spread_type_mismatch.res +tests/build_tests/super_errors/fixtures/list_spread_type_mismatch.res +tests/build_tests/super_errors/fixtures/array_spread_annotation_mismatch.res ~tests/ounit_tests/ounit_ast_mapper0_tests.ml ~tests/syntax_tests/data/printer/expr/array.res ~tests/syntax_tests/data/printer/expr/list.res ~tests/syntax_tests/data/printer/comments/array.res Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EUpL6dKCiSwAeriDGRHqNr Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- analysis/reanalyze/src/arnold.ml | 2 + analysis/reanalyze/src/side_effects.ml | 2 + analysis/src/completion_front_end.ml | 12 ++ analysis/src/dump_ast.ml | 17 +++ analysis/src/hint.ml | 7 +- analysis/src/semantic_tokens.ml | 8 - analysis/src/utils.ml | 1 + changes/spread-literal-errors.polish.md | 1 + changes/spread-literal-formatting.fix.md | 1 + changes/spread-literal-node.internal.md | 1 + compiler/core/lam_analysis.ml | 3 +- compiler/core/lam_compile_primitive.ml | 2 + compiler/ml/ast_helper.ml | 1 + compiler/ml/ast_helper.mli | 2 + compiler/ml/ast_iterator.ml | 5 + compiler/ml/ast_mapper.ml | 7 + compiler/ml/ast_mapper_from0.ml | 49 ++++++ compiler/ml/ast_mapper_to0.ml | 6 + compiler/ml/ast_spread.ml | 95 ++++++++++++ compiler/ml/depend.ml | 5 + compiler/ml/lambda.ml | 6 + compiler/ml/lambda.mli | 4 + compiler/ml/parsetree.ml | 11 ++ compiler/ml/pprintast.ml | 11 ++ compiler/ml/printast.ml | 14 ++ compiler/ml/printlambda.ml | 2 + compiler/ml/printtyped.ml | 6 + compiler/ml/rec_check.ml | 10 +- compiler/ml/tast_iterator.ml | 1 + compiler/ml/tast_mapper.ml | 2 + compiler/ml/translcore.ml | 8 + compiler/ml/typecore.ml | 29 ++++ compiler/ml/typedtree.ml | 3 + compiler/ml/typedtree.mli | 3 + compiler/ml/typedtree_iter.ml | 1 + compiler/syntax/src/res_comments_table.ml | 33 ++-- compiler/syntax/src/res_core.ml | 124 ++++----------- compiler/syntax/src/res_parsetree_viewer.ml | 27 ---- compiler/syntax/src/res_parsetree_viewer.mli | 7 - compiler/syntax/src/res_printer.ml | 141 ++++-------------- .../tests/src/SpreadLiterals.res | 20 +++ .../tests/src/expected/SpreadLiterals.res.txt | 119 +++++++++++++++ ...ay_spread_annotation_mismatch.res.expected | 16 ++ .../array_spread_type_mismatch.res.expected | 11 ++ .../list_spread_type_mismatch.res.expected | 11 ++ .../array_spread_annotation_mismatch.res | 4 + .../fixtures/array_spread_type_mismatch.res | 4 + .../fixtures/list_spread_type_mismatch.res | 4 + tests/ounit_tests/ounit_ast_mapper0_tests.ml | 95 ++++++++++++ .../data/ast-mapping/SpreadLiterals.res | 15 ++ .../expected/SpreadLiterals.res.txt | 15 ++ .../expressions/expected/array.res.txt | 10 +- .../grammar/expressions/expected/list.res.txt | 2 +- .../recovery/expression/expected/list.res.txt | 3 +- .../data/printer/comments/array.res | 9 ++ .../printer/comments/expected/array.res.txt | 9 ++ .../syntax_tests/data/printer/expr/array.res | 12 ++ .../data/printer/expr/expected/array.res.txt | 12 ++ .../data/printer/expr/expected/list.res.txt | 4 + tests/syntax_tests/data/printer/expr/list.res | 4 + 60 files changed, 758 insertions(+), 291 deletions(-) create mode 100644 changes/spread-literal-errors.polish.md create mode 100644 changes/spread-literal-formatting.fix.md create mode 100644 changes/spread-literal-node.internal.md create mode 100644 compiler/ml/ast_spread.ml create mode 100644 tests/analysis_tests/tests/src/SpreadLiterals.res create mode 100644 tests/analysis_tests/tests/src/expected/SpreadLiterals.res.txt create mode 100644 tests/build_tests/super_errors/expected/array_spread_annotation_mismatch.res.expected create mode 100644 tests/build_tests/super_errors/expected/array_spread_type_mismatch.res.expected create mode 100644 tests/build_tests/super_errors/expected/list_spread_type_mismatch.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/array_spread_annotation_mismatch.res create mode 100644 tests/build_tests/super_errors/fixtures/array_spread_type_mismatch.res create mode 100644 tests/build_tests/super_errors/fixtures/list_spread_type_mismatch.res create mode 100644 tests/syntax_tests/data/ast-mapping/SpreadLiterals.res create mode 100644 tests/syntax_tests/data/ast-mapping/expected/SpreadLiterals.res.txt diff --git a/analysis/reanalyze/src/arnold.ml b/analysis/reanalyze/src/arnold.ml index b67b1a9fb08..419f0241d8c 100644 --- a/analysis/reanalyze/src/arnold.ml +++ b/analysis/reanalyze/src/arnold.ml @@ -995,6 +995,8 @@ module Compile = struct |> List.map (function Typedtree.Tdict_entry (_, e) | Tdict_spread e -> expression ~ctx e) |> Command.unordered_sequence + | Texp_spread (_, parts) -> + parts |> List.map (expression ~ctx) |> Command.unordered_sequence | Texp_assert _ -> Command.nothing | Texp_try (e, cases, finally) -> ( let c_e = e |> expression ~ctx in diff --git a/analysis/reanalyze/src/side_effects.ml b/analysis/reanalyze/src/side_effects.ml index 05a1d04b2b2..b7447ac2284 100644 --- a/analysis/reanalyze/src/side_effects.ml +++ b/analysis/reanalyze/src/side_effects.ml @@ -64,6 +64,8 @@ let rec expr_no_side_effects (expr : Typedtree.expression) = | Typedtree.Tdict_entry (_, e) -> e |> expr_no_side_effects (* Spreading a dict reads its properties *) | Tdict_spread _ -> false) + (* A call of the runtime's [spread] *) + | Texp_spread _ -> false | Texp_ifthenelse (e1, e2, eo) -> e1 |> expr_no_side_effects && e2 |> expr_no_side_effects && eo |> expr_opt_no_side_effects diff --git a/analysis/src/completion_front_end.ml b/analysis/src/completion_front_end.ml index 7cd50d5a5eb..8ad988f56b3 100644 --- a/analysis/src/completion_front_end.ml +++ b/analysis/src/completion_front_end.ml @@ -231,6 +231,18 @@ let rec expr_to_context_path_inner ~(in_jsx_context : bool) | [] -> None | exp :: _ -> expr_to_context_path ~in_jsx_context exp)) | Pexp_dict _ -> Some (CPDict None) + | Pexp_spread (kind, _) -> + (* Typed like the runtime call it compiles to, as when the parser + represented it as [Pexp_apply] *) + Some + (CPApply + ( CPId + { + path = [Ast_spread.module_name kind; "spread"]; + completion_context = Value; + loc = e.pexp_loc; + }, + [Nolabel] )) | Pexp_ident {txt = Lident "->"} -> None | Pexp_ident {txt; loc} -> Some diff --git a/analysis/src/dump_ast.ml b/analysis/src/dump_ast.ml index f0b63127cdb..9d420f4d9b5 100644 --- a/analysis/src/dump_ast.ml +++ b/analysis/src/dump_ast.ml @@ -222,6 +222,23 @@ and print_expr_item expr ~pos ~indentation = ^ "\n" ^ add_indentation indentation ^ ")" + | Pexp_spread (kind, elements) -> + (match kind with + | Spread_array -> "Pexp_spread(array,\n" + | Spread_list -> "Pexp_spread(list,\n") + ^ (elements + |> List.map (fun element -> + add_indentation (indentation + 1) + ^ + match element with + | Parsetree.Pspread_item expr -> + print_expr_item expr ~pos ~indentation:(indentation + 1) + | Pspread expr -> + "..." ^ print_expr_item expr ~pos ~indentation:(indentation + 1)) + |> String.concat "\n") + ^ "\n" + ^ add_indentation indentation + ^ ")" | Pexp_match (match_expr, cases) -> "Pexp_match(" ^ print_expr_item match_expr ~pos ~indentation:0 diff --git a/analysis/src/hint.ml b/analysis/src/hint.ml index e56ef411fe5..e65e02469cf 100644 --- a/analysis/src/hint.ml +++ b/analysis/src/hint.ml @@ -59,9 +59,10 @@ let inlay ~source ~kind_file ~pos ~max_length ~full ~state ~debug = match expr.pexp_desc with | Pexp_constant _ | Pexp_tuple _ | Pexp_record _ | Pexp_variant _ | Pexp_apply _ | Pexp_match _ | Pexp_construct _ | Pexp_ifthenelse _ - | Pexp_ternary _ | Pexp_array _ | Pexp_dict _ | Pexp_ident _ | Pexp_try _ - | Pexp_object_get _ | Pexp_object_set _ | Pexp_field _ | Pexp_open _ - | Pexp_fun _ | Pexp_template _ | Pexp_tagged_template _ -> + | Pexp_ternary _ | Pexp_array _ | Pexp_dict _ | Pexp_spread _ + | Pexp_ident _ | Pexp_try _ | Pexp_object_get _ | Pexp_object_set _ + | Pexp_field _ | Pexp_open _ | Pexp_fun _ | Pexp_template _ + | Pexp_tagged_template _ -> push vb.pvb_pat.ppat_loc Type | _ -> ()) | {pvb_pat = {ppat_desc = Ppat_tuple _}} -> process_pattern vb.pvb_pat diff --git a/analysis/src/semantic_tokens.ml b/analysis/src/semantic_tokens.ml index 1e0219e2d01..4aa49218cb4 100644 --- a/analysis/src/semantic_tokens.ml +++ b/analysis/src/semantic_tokens.ml @@ -257,14 +257,6 @@ let command ~debug ~emitter ~source ~kind_file = (* Don't emit semantic tokens for identifiers not present in source code *) let should_emit = match lid with - (* Collection spreads (`...`) are converted to primitive spread calls - with an `@res.spread` decorator. *) - | Ldot (Lident ("Primitive_array" | "Primitive_list"), "spread") -> - let has_spread_attr = - e.pexp_attributes - |> List.exists (fun ({Location.txt}, _) -> txt = "res.spread") - in - not has_spread_attr (* Array access (`arr[index]`) is converted to `Array.get` *) | Ldot (Lident "Array", "get") -> false (* Array mutation (`arr[index]`) is converted to `Array.set` *) diff --git a/analysis/src/utils.ml b/analysis/src/utils.ml index 8bea86f5a69..129dff3604c 100644 --- a/analysis/src/utils.ml +++ b/analysis/src/utils.ml @@ -123,6 +123,7 @@ let identify_pexp pexp = | Pexp_jsx_element _ -> "Pexp_jsx_element" | Pexp_regexp _ -> "Pexp_regexp" | Pexp_dict _ -> "Pexp_dict" + | Pexp_spread _ -> "Pexp_spread" | Pexp_template _ -> "Pexp_template" | Pexp_tagged_template _ -> "Pexp_tagged_template" diff --git a/changes/spread-literal-errors.polish.md b/changes/spread-literal-errors.polish.md new file mode 100644 index 00000000000..bda3a066206 --- /dev/null +++ b/changes/spread-literal-errors.polish.md @@ -0,0 +1 @@ +- Report a spread of the wrong collection type, such as a list spread into an array, as a mismatch of the spread instead of an array item. diff --git a/changes/spread-literal-formatting.fix.md b/changes/spread-literal-formatting.fix.md new file mode 100644 index 00000000000..cc712334f50 --- /dev/null +++ b/changes/spread-literal-formatting.fix.md @@ -0,0 +1 @@ +- Fix the formatter dropping spreads of array and list literals, attributes on literals with spreads, and comments in list literals with spreads. diff --git a/changes/spread-literal-node.internal.md b/changes/spread-literal-node.internal.md new file mode 100644 index 00000000000..146d6dce7ab --- /dev/null +++ b/changes/spread-literal-node.internal.md @@ -0,0 +1 @@ +- Represent array and list literals with spreads with a dedicated parsetree node. diff --git a/compiler/core/lam_analysis.ml b/compiler/core/lam_analysis.ml index de610d34089..07d85c1eb75 100644 --- a/compiler/core/lam_analysis.ml +++ b/compiler/core/lam_analysis.ml @@ -101,7 +101,8 @@ let rec no_side_effects (lam : Lambda.t) : bool = (* A spread reads the source's properties, which may run getters or proxy traps *) | Pdict_spread | Pjs_object_get _ | Pjs_object_set _ | Pdebugger - | Pjs_fn_method + (* Runtime calls, like the [Lapply] they were parsed into *) + | Parray_spread | Plist_spread | Pjs_fn_method (* Await promise *) | Pawait (* TODO *) diff --git a/compiler/core/lam_compile_primitive.ml b/compiler/core/lam_compile_primitive.ml index e36c74a927b..bb68ba83ec9 100644 --- a/compiler/core/lam_compile_primitive.ml +++ b/compiler/core/lam_compile_primitive.ml @@ -584,6 +584,8 @@ let translate output_prefix loc (cxt : Lam_compile_context.t) Some (Js_op.Lit txt, expr) | _ -> None)) | _ -> assert false) + | Parray_spread -> E.runtime_call Primitive_modules.array "spread" args + | Plist_spread -> E.runtime_call Primitive_modules.list "spread" args | Pdict_spread -> (* {...target, ...source}, with the rows of dict literals in place *) E.obj_entries diff --git a/compiler/ml/ast_helper.ml b/compiler/ml/ast_helper.ml index a5708c941f0..aec4e257793 100644 --- a/compiler/ml/ast_helper.ml +++ b/compiler/ml/ast_helper.ml @@ -172,6 +172,7 @@ module Exp = struct let setfield ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_setfield (a, b, c)) let array ?loc ?attrs a = mk ?loc ?attrs (Pexp_array a) let dict ?loc ?attrs a = mk ?loc ?attrs (Pexp_dict a) + let spread ?loc ?attrs kind a = mk ?loc ?attrs (Pexp_spread (kind, a)) let ifthenelse ?loc ?attrs a b c = mk ?loc ?attrs (Pexp_ifthenelse (a, b, c)) let ternary ?loc ?attrs condition consequent alternate = mk ?loc ?attrs (Pexp_ternary (condition, consequent, alternate)) diff --git a/compiler/ml/ast_helper.mli b/compiler/ml/ast_helper.mli index 9ccdc7cba5a..b5c4dd31c75 100644 --- a/compiler/ml/ast_helper.mli +++ b/compiler/ml/ast_helper.mli @@ -173,6 +173,8 @@ module Exp : sig ?loc:loc -> ?attrs:attrs -> expression -> lid -> expression -> expression val array : ?loc:loc -> ?attrs:attrs -> expression list -> expression val dict : ?loc:loc -> ?attrs:attrs -> dict_entry list -> expression + val spread : + ?loc:loc -> ?attrs:attrs -> spread_kind -> spread_element list -> expression val ifthenelse : ?loc:loc -> ?attrs:attrs -> diff --git a/compiler/ml/ast_iterator.ml b/compiler/ml/ast_iterator.ml index 5c4b4f04262..80f8897b203 100644 --- a/compiler/ml/ast_iterator.ml +++ b/compiler/ml/ast_iterator.ml @@ -364,6 +364,11 @@ module E = struct sub.expr sub e | Pdict_spread e -> sub.expr sub e) entries + | Pexp_spread (_, elements) -> + List.iter + (function + | Pspread_item e | Pspread e -> sub.expr sub e) + elements | Pexp_ifthenelse (e1, e2, e3) -> sub.expr sub e1; sub.expr sub e2; diff --git a/compiler/ml/ast_mapper.ml b/compiler/ml/ast_mapper.ml index 0c7f7d82fa0..de8efdb7e22 100644 --- a/compiler/ml/ast_mapper.ml +++ b/compiler/ml/ast_mapper.ml @@ -378,6 +378,13 @@ module E = struct Pdict_entry (map_loc sub key, sub.expr sub e) | Pdict_spread e -> Pdict_spread (sub.expr sub e)) entries) + | Pexp_spread (kind, elements) -> + spread ~loc ~attrs kind + (List.map + (function + | Pspread_item e -> Pspread_item (sub.expr sub e) + | Pspread e -> Pspread (sub.expr sub e)) + elements) | Pexp_ifthenelse (e1, e2, e3) -> ifthenelse ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2) (map_opt (sub.expr sub) e3) diff --git a/compiler/ml/ast_mapper_from0.ml b/compiler/ml/ast_mapper_from0.ml index b1021e8db07..106d9adae99 100644 --- a/compiler/ml/ast_mapper_from0.ml +++ b/compiler/ml/ast_mapper_from0.ml @@ -678,6 +678,47 @@ module E = struct | None -> None) | _ -> Option.map rows (dict_rows ~marker:Ast_dict.literal_marker e) + (* The kind and elements of an array or list literal with spreads encoded as + a call, see [Ast_spread] *) + let spread_elements (e : expression) = + let generated (e : expression) = + e.pexp_loc.loc_ghost && e.pexp_attributes = [] + in + let rec list_part (e : expression) = + match e.pexp_desc with + | Pexp_construct + ( {txt = Lident "::"}, + Some {pexp_desc = Pexp_tuple [item; tail]; pexp_attributes = []} ) + when generated e -> + `Item item :: list_part tail + | Pexp_construct ({txt = Lident "[]"}, None) when generated e -> [] + | _ -> [`Spread e] + in + let array_part (e : expression) = + match e.pexp_desc with + | Pexp_array items when generated e -> + List.map (fun item -> `Item item) items + | _ -> [`Spread e] + in + match e.pexp_desc with + | Pexp_apply + ( { + pexp_desc = Pexp_ident {txt = Ldot (Lident module_name, "spread")}; + pexp_attributes = [({txt}, PStr [])]; + }, + [ + ( Asttypes.Noloc.Nolabel, + {pexp_desc = Pexp_array parts; pexp_attributes = []} ); + ] ) + when txt = Ast_spread.marker -> ( + match Ast_spread.kind_of_module_name module_name with + | Some Spread_array -> + Some (Pt.Spread_array, List.concat_map array_part parts) + | Some Spread_list -> + Some (Pt.Spread_list, List.concat_map list_part parts) + | None -> None) + | _ -> None + let extract_for_of_attribute attrs = List.find_map (function @@ -932,6 +973,14 @@ module E = struct Pt.Pdict_entry (map_loc sub key, sub.expr sub value) | `Spread spread -> Pt.Pdict_spread (sub.expr sub spread)) entries) + | Pexp_apply _ when Option.is_some (spread_elements e) -> + let kind, elements = Option.get (spread_elements e) in + spread ~loc ~attrs kind + (List.map + (function + | `Item item -> Pt.Pspread_item (sub.expr sub item) + | `Spread spread -> Pt.Pspread (sub.expr sub spread)) + elements) | Pexp_apply ( {pexp_desc = Pexp_ident {txt = Longident.Lident "#="}}, [ diff --git a/compiler/ml/ast_mapper_to0.ml b/compiler/ml/ast_mapper_to0.ml index 7da9399a781..2cdb48c22a3 100644 --- a/compiler/ml/ast_mapper_to0.ml +++ b/compiler/ml/ast_mapper_to0.ml @@ -704,6 +704,12 @@ module E = struct into *) sub.expr sub (Ast_dict.to_calls ~loc:e.pexp_loc ~attrs:e.pexp_attributes entries) + | Pexp_spread (kind, elements) -> + (* v0 has no spread literals: they're the calls they used to be parsed + into *) + sub.expr sub + (Ast_spread.to_call ~loc:e.pexp_loc ~attrs:e.pexp_attributes kind + elements) | Pexp_ifthenelse (e1, e2, e3) -> ifthenelse ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2) (map_opt (sub.expr sub) e3) diff --git a/compiler/ml/ast_spread.ml b/compiler/ml/ast_spread.ml new file mode 100644 index 00000000000..1c8d9f27bf3 --- /dev/null +++ b/compiler/ml/ast_spread.ml @@ -0,0 +1,95 @@ +(* Array and list literals with spreads, [Pexp_spread]: [[a, ...b]] and + [list{...a, b}]. They used to be parsed into calls of the runtime's + [Primitive_array.spread] and [Primitive_list.spread], which remain their + encoding on the v0 PPX wire and the calls they compile to. The [spread] + carries the [res.spread] marker, and its argument is an array of parts: + - for arrays, the spreads, and array literals of the items between them; + - for lists, the items before each spread consed onto it + ([a :: b :: spread]), and a list literal of the items after the last one. + The array literals and [::] and [[]] constructors built from the items have + ghost locations, which tells them apart from array and list literals that + are spread. *) + +open Parsetree + +let marker = "res.spread" + +let module_name = function + | Spread_array -> Primitive_modules.array + | Spread_list -> Primitive_modules.list + +let kind_of_module_name name = + if name = Primitive_modules.array then Some Spread_array + else if name = Primitive_modules.list then Some Spread_list + else None + +(* The items before each spread with that spread, and the items after the last + spread *) +let groups (elements : spread_element list) : + (expression list * expression option) list = + let flush items acc = + match items with + | [] -> acc + | _ -> (List.rev items, None) :: acc + in + let rec loop items acc = function + | [] -> List.rev (flush items acc) + | Pspread_item item :: rest -> loop (item :: items) acc rest + | Pspread spread :: rest -> + loop [] ((List.rev items, Some spread) :: acc) rest + in + loop [] [] elements + +let ghost_span (first : expression) (last : expression) = + { + Location.loc_start = first.pexp_loc.loc_start; + loc_end = last.pexp_loc.loc_end; + loc_ghost = true; + } + +(* The parts in the argument of [Primitive_array.spread] or + [Primitive_list.spread], each an array or list *) +let parts kind elements = + let construct ~loc name args = + Ast_helper.Exp.construct ~loc + (Location.mkloc (Longident.Lident name) loc) + (Location.mkloc args loc) + in + List.concat_map + (fun (items, spread) -> + match (kind, items, List.rev items) with + | _, [], _ | _, _, [] -> Option.to_list spread + | Spread_array, first :: _, last :: _ -> + Ast_helper.Exp.array ~loc:(ghost_span first last) items + :: Option.to_list spread + | Spread_list, _, last :: _ -> + let tail = + match spread with + | Some spread -> spread + | None -> + construct + ~loc: + { + last.pexp_loc with + loc_start = last.pexp_loc.loc_end; + loc_ghost = true; + } + "[]" [] + in + [ + List.fold_right + (fun item tail -> + construct ~loc:(ghost_span item tail) "::" [item; tail]) + items tail; + ]) + (groups elements) + +(* The call encoding the literal [kind] of [elements] at [loc] *) +let to_call ~loc ~attrs kind elements = + Ast_helper.Exp.apply ~loc ~attrs + (Ast_helper.Exp.ident ~loc + ~attrs:[(Location.mknoloc marker, PStr [])] + (Location.mkloc + (Longident.Ldot (Longident.Lident (module_name kind), "spread")) + loc)) + [(Asttypes.Nolabel, Ast_helper.Exp.array ~loc (parts kind elements))] diff --git a/compiler/ml/depend.ml b/compiler/ml/depend.ml index 1c9b4350c3c..5c5239e444d 100644 --- a/compiler/ml/depend.ml +++ b/compiler/ml/depend.ml @@ -274,6 +274,11 @@ let rec add_expr bv exp = (function | Pdict_entry (_, e) | Pdict_spread e -> add_expr bv e) entries + | Pexp_spread (_, elements) -> + List.iter + (function + | Pspread_item e | Pspread e -> add_expr bv e) + elements | Pexp_ifthenelse (e1, e2, opte3) -> add_expr bv e1; add_expr bv e2; diff --git a/compiler/ml/lambda.ml b/compiler/ml/lambda.ml index 5e9d529f7c0..e8e6ec91b69 100644 --- a/compiler/ml/lambda.ml +++ b/compiler/ml/lambda.ml @@ -287,6 +287,10 @@ type primitive = | Ptemplate of Asttypes.template_segment list (* {...target, ...sources} *) | Pdict_spread + (* [Primitive_array.spread(parts)] and [Primitive_list.spread(parts)], the + concatenation of an array of arrays or lists *) + | Parray_spread + | Plist_spread and comparison = Ceq | Cneq | Clt | Cgt | Cle | Cge @@ -486,6 +490,8 @@ let eq_primitive_approx (lhs : primitive) (rhs : primitive) = | Pmakelist (* dict primitives *) | Pmakedict | Pdict_spread | Pdict_has + (* spread primitives *) + | Parray_spread | Plist_spread (* promise *) | Pawait (* etc *) diff --git a/compiler/ml/lambda.mli b/compiler/ml/lambda.mli index f0ed1cac2c8..3e22e7397a4 100644 --- a/compiler/ml/lambda.mli +++ b/compiler/ml/lambda.mli @@ -289,6 +289,10 @@ type primitive = | Ptemplate of template_segment list (* {...target, ...sources} *) | Pdict_spread + (* [Primitive_array.spread(parts)] and [Primitive_list.spread(parts)], the + concatenation of an array of arrays or lists *) + | Parray_spread + | Plist_spread and comparison = Ceq | Cneq | Clt | Cgt | Cle | Cge diff --git a/compiler/ml/parsetree.ml b/compiler/ml/parsetree.ml index 5b769c07d61..f7f2d037172 100644 --- a/compiler/ml/parsetree.ml +++ b/compiler/ml/parsetree.ml @@ -361,6 +361,11 @@ and expression_desc = | Pexp_setfield of expression * Longident.t loc * expression (* E1.l <- E2 *) | Pexp_array of expression list (* [| E1; ...; En |] *) | Pexp_dict of dict_entry list (* dict{"k1": E1, ...E2} *) + | Pexp_spread of spread_kind * spread_element list + (* [E1, ...E2] (Spread_array) or list{...E1, E2} (Spread_list). + The parser produces it only for literals that need it: an array + literal without a spread is [Pexp_array], and a list literal whose only + spread is its tail, list{E1, ...E2}, is a chain of [::] constructors. *) | Pexp_ifthenelse of expression * expression * expression option (* if E1 then E2 else E3 *) | Pexp_ternary of expression * expression * expression @@ -438,6 +443,12 @@ and dict_entry = | Pdict_entry of string loc * expression (* "k": E *) | Pdict_spread of expression (* ...E *) +and spread_kind = Spread_array | Spread_list + +and spread_element = + | Pspread_item of expression (* E *) + | Pspread of expression (* ...E *) + and jsx_element = | Jsx_fragment of jsx_fragment | Jsx_unary_element of jsx_unary_element diff --git a/compiler/ml/pprintast.ml b/compiler/ml/pprintast.ml index 796a0b7b7cd..561b25cc676 100644 --- a/compiler/ml/pprintast.ml +++ b/compiler/ml/pprintast.ml @@ -899,6 +899,17 @@ and simple_expr ctxt f x = | Pdict_spread e -> pp f "...%a" (simple_expr ctxt) e in pp f "@[dict{%a}@]" (list dict_entry ~sep:",@;") entries + | Pexp_spread (kind, elements) -> + let element f = function + | Pspread_item e -> simple_expr ctxt f e + | Pspread e -> pp f "...%a" (simple_expr ctxt) e + in + let opening, closing = + match kind with + | Spread_array -> ("[", "]") + | Spread_list -> ("list{", "}") + in + pp f "@[%s%a%s@]" opening (list element ~sep:",@;") elements closing | Pexp_break -> pp f "break" | Pexp_continue -> pp f "continue" | Pexp_while (e1, e2) -> diff --git a/compiler/ml/printast.ml b/compiler/ml/printast.ml index 101db14d639..49e01cb9ba0 100644 --- a/compiler/ml/printast.ml +++ b/compiler/ml/printast.ml @@ -352,6 +352,12 @@ and expression i ppf x = | Pexp_dict l -> line i ppf "Pexp_dict\n"; list i dict_entry ppf l + | Pexp_spread (kind, l) -> + line i ppf "Pexp_spread %s\n" + (match kind with + | Spread_array -> "array" + | Spread_list -> "list"); + list i spread_element ppf l | Pexp_ifthenelse (e1, e2, eo) -> line i ppf "Pexp_ifthenelse\n"; expression i ppf e1; @@ -483,6 +489,14 @@ and dict_entry i ppf = function line i ppf "Pdict_spread\n"; expression (i + 1) ppf e +and spread_element i ppf = function + | Pspread_item e -> + line i ppf "Pspread_item\n"; + expression (i + 1) ppf e + | Pspread e -> + line i ppf "Pspread\n"; + expression (i + 1) ppf e + and jsx_children i ppf children = line i ppf "jsx_children =\n"; list (i + 1) expression ppf children diff --git a/compiler/ml/printlambda.ml b/compiler/ml/printlambda.ml index 0457903c6a6..5b63855108f 100644 --- a/compiler/ml/printlambda.ml +++ b/compiler/ml/printlambda.ml @@ -205,6 +205,8 @@ let primitive ppf = function | Pmakelist -> fprintf ppf "makelist" | Pmakedict -> fprintf ppf "makedict" | Pdict_spread -> fprintf ppf "dict.spread" + | Parray_spread -> fprintf ppf "array.spread" + | Plist_spread -> fprintf ppf "list.spread" | Pdict_has -> fprintf ppf "dict.has" | Pis_null -> fprintf ppf "is_null" | Pis_undefined -> fprintf ppf "is_undefined" diff --git a/compiler/ml/printtyped.ml b/compiler/ml/printtyped.ml index f017f9a21cf..aeef6ee13ae 100644 --- a/compiler/ml/printtyped.ml +++ b/compiler/ml/printtyped.ml @@ -337,6 +337,12 @@ and expression i ppf x = | Texp_dict l -> line i ppf "Texp_dict\n"; list i dict_entry ppf l + | Texp_spread (kind, l) -> + line i ppf "Texp_spread %s\n" + (match kind with + | Spread_array -> "array" + | Spread_list -> "list"); + list i expression ppf l | Texp_ifthenelse (e1, e2, eo) -> line i ppf "Texp_ifthenelse\n"; expression i ppf e1; diff --git a/compiler/ml/rec_check.ml b/compiler/ml/rec_check.ml index 47a2a2e8760..fb8210bd004 100644 --- a/compiler/ml/rec_check.ml +++ b/compiler/ml/rec_check.ml @@ -211,10 +211,11 @@ let rec classify_expression : Typedtree.expression -> sd = | Texp_apply {funct = {exp_desc = Texp_ident (_, _, vd)}} when is_ref vd -> Static (* A dict literal is built by the [Pmakedict] and [Pdict_spread] primitives, - see [Translcore] *) - | Texp_apply _ | Texp_dict _ | Texp_match _ | Texp_ifthenelse _ - | Texp_object_get _ | Texp_object_set _ | Texp_field _ | Texp_assert _ - | Texp_try _ | Texp_tagged_template _ | Texp_template _ -> + and an array or list literal with spreads by [Parray_spread] or + [Plist_spread], see [Translcore] *) + | Texp_apply _ | Texp_dict _ | Texp_spread _ | Texp_match _ + | Texp_ifthenelse _ | Texp_object_get _ | Texp_object_set _ | Texp_field _ + | Texp_assert _ | Texp_try _ | Texp_tagged_template _ | Texp_template _ -> Dynamic let rec expression : Env.env -> Typedtree.expression -> Use.t = @@ -277,6 +278,7 @@ let rec expression : Env.env -> Typedtree.expression -> Use.t = | Tdict_entry (_, e) | Tdict_spread e -> expression env e in Use.inspect (list entry env entries) + | Texp_spread (_, parts) -> Use.inspect (list expression env parts) | Texp_construct (_, desc, exprs) -> let access_constructor = match desc.cstr_kind with diff --git a/compiler/ml/tast_iterator.ml b/compiler/ml/tast_iterator.ml index fb505e98fac..64966ea4283 100644 --- a/compiler/ml/tast_iterator.ml +++ b/compiler/ml/tast_iterator.ml @@ -185,6 +185,7 @@ let expr sub {exp_extra; exp_desc; exp_env; _} = (function | Tdict_entry (_, exp) | Tdict_spread exp -> sub.expr sub exp) entries + | Texp_spread (_, parts) -> List.iter (sub.expr sub) parts | Texp_ifthenelse (exp1, exp2, expo) -> sub.expr sub exp1; sub.expr sub exp2; diff --git a/compiler/ml/tast_mapper.ml b/compiler/ml/tast_mapper.ml index b8905b8d269..c3585180b42 100644 --- a/compiler/ml/tast_mapper.ml +++ b/compiler/ml/tast_mapper.ml @@ -252,6 +252,8 @@ let expr sub x = | Tdict_entry (key, exp) -> Tdict_entry (key, sub.expr sub exp) | Tdict_spread exp -> Tdict_spread (sub.expr sub exp)) entries) + | Texp_spread (kind, parts) -> + Texp_spread (kind, List.map (sub.expr sub) parts) | Texp_ifthenelse (exp1, exp2, expo) -> Texp_ifthenelse (sub.expr sub exp1, sub.expr sub exp2, opt (sub.expr sub) expo) diff --git a/compiler/ml/translcore.ml b/compiler/ml/translcore.ml index 173aa452480..5e5b0ae9bed 100644 --- a/compiler/ml/translcore.ml +++ b/compiler/ml/translcore.ml @@ -1436,6 +1436,14 @@ and transl_exp0 (e : Typedtree.expression) : Lambda.t = | Texp_array expr_list -> let ll = transl_list expr_list in prim ~primitive:Pmakearray ~args:ll e.exp_loc + | Texp_spread (kind, parts) -> + prim + ~primitive: + (match kind with + | Spread_array -> Parray_spread + | Spread_list -> Plist_spread) + ~args:[prim ~primitive:Pmakearray ~args:(transl_list parts) e.exp_loc] + e.exp_loc | Texp_dict entries -> ( (* [%makedict] of the rows ([Primitive_dict.make]), and [Pdict_spread] of a dict of the rows before the first spread, the spreads and the diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index eb5cac2cd01..a2b5fdbb8f7 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -283,6 +283,11 @@ let iter_expression f e = (function | Pdict_entry (_, e) | Pdict_spread e -> expr e) entries + | Pexp_spread (_, elements) -> + List.iter + (function + | Pspread_item e | Pspread e -> expr e) + elements | Pexp_await _ -> assert false (* should be handled earlier *) | Pexp_jsx_element _ -> raise (Error (e.pexp_loc, Env.empty, Jsx_not_enabled)) @@ -3227,6 +3232,30 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp exp_attributes = sexp.pexp_attributes; exp_env = env; } + | Pexp_spread (kind, elements) -> + let container_type ty = + match kind with + | Spread_array -> Predef.type_array ty + | Spread_list -> newgenty (Tconstr (Predef.path_list, [ty], ref Mnil)) + in + let ty = newgenvar () in + unify_exp_types ~context:None loc env (container_type ty) ty_expected; + (* The items are typed in the array and list literals they're grouped + into *) + let parts = + List.map + (fun spart -> type_expect ~context:None env spart (container_type ty)) + (Ast_spread.parts kind elements) + in + re + { + exp_desc = Texp_spread (kind, parts); + exp_loc = loc; + exp_extra = []; + exp_type = instance env ty_expected; + exp_attributes = sexp.pexp_attributes; + exp_env = env; + } | (Pexp_ifthenelse _ | Pexp_ternary _) as conditional -> ( let scond, sifso, sifnot, return_context = match conditional with diff --git a/compiler/ml/typedtree.ml b/compiler/ml/typedtree.ml index aaff3f86332..a72aa2099b9 100644 --- a/compiler/ml/typedtree.ml +++ b/compiler/ml/typedtree.ml @@ -123,6 +123,9 @@ and expression_desc = expression * Longident.t loc * label_description * expression | Texp_array of expression list | Texp_dict of dict_entry list + | Texp_spread of Parsetree.spread_kind * expression list + (* An array or list literal with spreads: the arrays or lists it + concatenates, see [Ast_spread] *) | Texp_ifthenelse of expression * expression * expression option | Texp_sequence of expression * expression | Texp_break diff --git a/compiler/ml/typedtree.mli b/compiler/ml/typedtree.mli index 54a7e69a766..6484730d860 100644 --- a/compiler/ml/typedtree.mli +++ b/compiler/ml/typedtree.mli @@ -242,6 +242,9 @@ and expression_desc = expression * Longident.t loc * label_description * expression | Texp_array of expression list | Texp_dict of dict_entry list + | Texp_spread of Parsetree.spread_kind * expression list + (* An array or list literal with spreads: the arrays or lists it + concatenates, see [Ast_spread] *) | Texp_ifthenelse of expression * expression * expression option | Texp_sequence of expression * expression | Texp_break diff --git a/compiler/ml/typedtree_iter.ml b/compiler/ml/typedtree_iter.ml index 0962b6e5a07..3480686a557 100644 --- a/compiler/ml/typedtree_iter.ml +++ b/compiler/ml/typedtree_iter.ml @@ -270,6 +270,7 @@ end = struct (function | Tdict_entry (_, exp) | Tdict_spread exp -> iter_expression exp) entries + | Texp_spread (_, parts) -> List.iter iter_expression parts | Texp_ifthenelse (exp1, exp2, expo) -> ( iter_expression exp1; iter_expression exp2; diff --git a/compiler/syntax/src/res_comments_table.ml b/compiler/syntax/src/res_comments_table.ml index fd7de0fa982..7cb6f8cb4fa 100644 --- a/compiler/syntax/src/res_comments_table.ml +++ b/compiler/syntax/src/res_comments_table.ml @@ -1493,6 +1493,13 @@ and walk_expression expr t comments = (Res_parsetree_viewer.dict_expr_rows ~loc:expr.pexp_loc entries); ] t comments + | Pexp_spread (_, elements) -> + walk_list + (List.map + (function + | Parsetree.Pspread_item e | Pspread e -> Expression e) + elements) + t comments | Pexp_apply { funct = @@ -1570,30 +1577,8 @@ and walk_expression expr t comments = walk_list (List.map (fun e -> Expression e) (tag :: values)) t comments | Pexp_template {values} -> walk_list (List.map (fun e -> Expression e) values) t comments - | Pexp_apply {funct = call_expr; args = arguments} -> ( - (* Special handling for array spread - treat it like an array *) - match call_expr.pexp_desc with - | Pexp_ident - {txt = Longident.Ldot (Longident.Lident "Primitive_array", "spread")} - when List.length arguments = 1 -> ( - match arguments with - | [(_, {pexp_desc = Pexp_array sub_arrays})] -> - (* Collect all individual expressions from sub-arrays *) - let all_exprs = - List.fold_left - (fun acc sub_array -> - match sub_array.Parsetree.pexp_desc with - | Pexp_array exprs -> acc @ exprs - | _ -> acc @ [sub_array]) - [] sub_arrays - in - walk_list (all_exprs |> List.map (fun e -> Expression e)) t comments - | _ -> - (* Fallback to regular apply handling *) - walk_apply_expr call_expr arguments t comments) - | _ -> - (* Regular apply handling *) - walk_apply_expr call_expr arguments t comments) + | Pexp_apply {funct = call_expr; args = arguments} -> + walk_apply_expr call_expr arguments t comments | Pexp_fun _ -> ( let _, parameters, return_expr = fun_expr expr in let comments = diff --git a/compiler/syntax/src/res_core.ml b/compiler/syntax/src/res_core.ml index 67ff15f6d83..b9dc31fb3fa 100644 --- a/compiler/syntax/src/res_core.ml +++ b/compiler/syntax/src/res_core.ml @@ -264,8 +264,6 @@ module Error_messages = struct "Spreading JSX children is no longer supported." end -let spread_attr = (Location.mknoloc "res.spread", Parsetree.PStr []) - (* Emit a deprecation warning when the legacy [(. ...)] uncurried syntax is encountered. Uncurried is the default since ReScript v11, so the leading dot is no longer meaningful; we still accept it so dependencies on older @@ -4184,62 +4182,32 @@ and parse_tuple_expr ~first ~start_pos p = let loc = mk_loc start_pos (Parser.position p) in Ast_helper.Exp.tuple ~loc exprs -and parse_spread_expr_region_with_loc p = - let start_pos = Parser.position p in +and parse_spread_element_region p = match Parser.peek p with | DotDotDot -> Parser.next p; - let expr = parse_constrained_or_coerced_expr p in - Some (true, expr, start_pos, Parser.position p) + Some (Parsetree.Pspread (parse_constrained_or_coerced_expr p)) | token when Grammar.is_expr_start token -> - Some - (false, parse_constrained_or_coerced_expr p, start_pos, Parser.position p) + Some (Parsetree.Pspread_item (parse_constrained_or_coerced_expr p)) | _ -> None and parse_list_expr ~start_pos p = - let split_by_spread exprs = - List.fold_left - (fun acc curr -> - match (curr, acc) with - | (true, expr, start_pos, end_pos), _ -> - (* find a spread expression, prepend a new sublist *) - ([], Some expr, start_pos, end_pos) :: acc - | ( (false, expr, start_pos, _endPos), - (no_spreads, spread, _accStartPos, acc_end_pos) :: 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, start_pos, acc_end_pos) :: acc - | (false, expr, start_pos, end_pos), [] -> - (* find a non-spread expression, and the accumulated is empty *) - [([expr], None, start_pos, end_pos)]) - [] exprs - in - let make_sub_expr = function - | exprs, Some spread, start_pos, end_pos -> - Ast_helper.Exp.make_list_expression (mk_loc start_pos end_pos) exprs - (Some spread) - | exprs, None, start_pos, end_pos -> - Ast_helper.Exp.make_list_expression (mk_loc start_pos end_pos) exprs None - in - let list_exprs_rev = - parse_comma_delimited_reversed_list p ~grammar:Grammar.ListExpr - ~closing:Rbrace ~f:parse_spread_expr_region_with_loc + let elements = + parse_comma_delimited_region p ~grammar:Grammar.ListExpr ~closing:Rbrace + ~f:parse_spread_element_region in Parser.expect Rbrace p; let loc = mk_loc start_pos (Parser.position p) in - match split_by_spread list_exprs_rev with - | [] -> Ast_helper.Exp.make_list_expression loc [] None - | [(exprs, Some spread, _, _)] -> - Ast_helper.Exp.make_list_expression loc exprs (Some spread) - | [(exprs, None, _, _)] -> Ast_helper.Exp.make_list_expression loc exprs None - | exprs -> - let list_exprs = List.map make_sub_expr exprs in - Ast_helper.Exp.apply ~loc - (Ast_helper.Exp.ident ~loc ~attrs:[spread_attr] - (Location.mkloc - (Longident.Ldot (Longident.Lident Primitive_modules.list, "spread")) - loc)) - [(Asttypes.Nolabel, Ast_helper.Exp.array ~loc list_exprs)] + (* Items, possibly followed by a spread, are a chain of [::] *) + let rec items_and_tail acc = function + | [] -> Some (List.rev acc, None) + | [Parsetree.Pspread tail] -> Some (List.rev acc, Some tail) + | Parsetree.Pspread_item item :: rest -> items_and_tail (item :: acc) rest + | Pspread _ :: _ -> None + in + match items_and_tail [] elements with + | Some (items, tail) -> Ast_helper.Exp.make_list_expression loc items tail + | None -> Ast_helper.Exp.spread ~loc Spread_list elements and parse_dict_expr ~start_pos p = let entries = @@ -4253,56 +4221,22 @@ and parse_dict_expr ~start_pos p = and parse_array_exp p = let start_pos = Parser.start_pos p in Parser.expect Lbracket p; - let split_by_spread exprs = - List.fold_left - (fun acc curr -> - match (curr, acc) with - | (true, expr, start_pos, end_pos), _ -> - (* find a spread expression, prepend a new sublist *) - ([], Some expr, start_pos, end_pos) :: acc - | ( (false, expr, start_pos, _endPos), - (no_spreads, spread, _accStartPos, acc_end_pos) :: 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, start_pos, acc_end_pos) :: acc - | (false, expr, start_pos, end_pos), [] -> - (* find a non-spread expression, and the accumulated is empty *) - [([expr], None, start_pos, end_pos)]) - [] exprs - in - let list_exprs_rev = - parse_comma_delimited_reversed_list p ~grammar:Grammar.ExprList - ~closing:Rbracket ~f:parse_spread_expr_region_with_loc + let elements = + parse_comma_delimited_region p ~grammar:Grammar.ExprList ~closing:Rbracket + ~f:parse_spread_element_region in Parser.expect Rbracket p; let loc = mk_loc start_pos (Parser.position p) in - let collect_exprs = 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 list_exprs_rev with - | [] -> Ast_helper.Exp.array ~loc:(mk_loc start_pos (Parser.position p)) [] - | [(exprs, None, _, _)] -> - Ast_helper.Exp.array ~loc:(mk_loc start_pos (Parser.position p)) exprs - | exprs -> - let xs = List.map collect_exprs exprs in - let list_exprs = - 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:[spread_attr] - (Location.mkloc - (Longident.Ldot (Longident.Lident Primitive_modules.array, "spread")) - loc)) - [(Nolabel, Ast_helper.Exp.array ~loc list_exprs)] + let items = + List.filter_map + (function + | Parsetree.Pspread_item item -> Some item + | Pspread _ -> None) + elements + in + if List.length items = List.length elements then + Ast_helper.Exp.array ~loc items + else Ast_helper.Exp.spread ~loc Spread_array elements (* TODO: check attributes in the case of poly type vars, * might be context dependend: parseFieldDeclaration (see ocaml) *) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index b11c5a08598..9b772741bc1 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -34,11 +34,6 @@ type dict_expr_part = | DictExprRows of Parsetree.expression | DictExprSpread of Parsetree.expression -let collect_array_expressions expr = - match expr.pexp_desc with - | Pexp_array exprs -> (exprs, None) - | _ -> ([], Some expr) - let collect_list_expressions expr = let rec collect acc expr = match expr.pexp_desc with @@ -511,28 +506,6 @@ let is_tagged_template_literal expr = | Pexp_tagged_template _ -> true | _ -> false -let has_spread_attr attrs = - List.exists - (fun attr -> - match attr with - | {Location.txt = "res.spread"}, _ -> true - | _ -> false) - attrs - -let is_spread_list expr = - match expr.pexp_desc with - | Pexp_ident - {txt = Longident.Ldot (Longident.Lident "Primitive_list", "spread")} -> - has_spread_attr expr.pexp_attributes - | _ -> false - -let is_spread_array expr = - match expr.pexp_desc with - | Pexp_ident - {txt = Longident.Ldot (Longident.Lident "Primitive_array", "spread")} -> - has_spread_attr expr.pexp_attributes - | _ -> false - let dict_has_spread entries = List.exists (function diff --git a/compiler/syntax/src/res_parsetree_viewer.mli b/compiler/syntax/src/res_parsetree_viewer.mli index 50557374d52..8b77a0ae2c4 100644 --- a/compiler/syntax/src/res_parsetree_viewer.mli +++ b/compiler/syntax/src/res_parsetree_viewer.mli @@ -29,10 +29,6 @@ val collect_if_expressions : (Location.t * Parsetree.expression * Parsetree.expression) list * Parsetree.expression option -val collect_array_expressions : - Parsetree.expression -> - Parsetree.expression list * Parsetree.expression option - val collect_list_expressions : Parsetree.expression -> Parsetree.expression list * Parsetree.expression option @@ -115,9 +111,6 @@ val is_block_expr : Parsetree.expression -> bool val is_template_literal : Parsetree.expression -> bool val is_tagged_template_literal : Parsetree.expression -> bool -val is_spread_list : Parsetree.expression -> bool - -val is_spread_array : Parsetree.expression -> bool val dict_has_spread : Parsetree.dict_entry list -> bool val dict_expr_rows : diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 5b92e1860b4..043d0f25ab9 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3582,14 +3582,8 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = print_template_literal ~state ~source_segments ~values cmt_tbl | Pexp_tagged_template {tag; raw_sources; values} -> print_tagged_template_literal ~state ~tag ~raw_sources ~values cmt_tbl - | Pexp_apply - {funct = e; args = [(Nolabel, {pexp_desc = Pexp_array sub_lists})]} - when Parsetree_viewer.is_spread_array e -> - print_array_spread_apply ~state sub_lists cmt_tbl - | Pexp_apply - {funct = e; args = [(Nolabel, {pexp_desc = Pexp_array sub_lists})]} - when Parsetree_viewer.is_spread_list e -> - print_list_spread_apply ~state sub_lists cmt_tbl + | Pexp_spread (kind, elements) -> + print_spread_expr ~state kind elements cmt_tbl | Pexp_apply _ -> if Parsetree_viewer.is_unary_expression e then print_unary_expression ~state e cmt_tbl @@ -4362,130 +4356,49 @@ and print_binary_expression ~state ~force_pipe_breaks ]) | _ -> Doc.nil -and print_array_spread_apply ~state sub_lists cmt_tbl = - let make_spread_doc comma_before_spread = function - | Some expr -> - (* Extract leading comments before dotdotdot *) +and print_spread_expr ~state kind elements cmt_tbl = + let expr_doc expr = + let doc = print_expression ~state expr cmt_tbl in + match Parens.expr_allowing_coercion expr with + | Parens.Parenthesized -> add_parens doc + | Braced braces -> print_braces doc expr braces + | Nothing -> doc + in + let element_doc = function + | Parsetree.Pspread_item expr -> + print_comments (expr_doc expr) cmt_tbl expr.pexp_loc + | Pspread expr -> + (* The leading comments go before the dots, so they're taken before the + expression is printed *) let leading_comments_doc = print_leading_comments Doc.nil cmt_tbl.Comment_table.leading - expr.Parsetree.pexp_loc + expr.pexp_loc in - (* Print expression without leading comments (they're already extracted) *) let expr_doc = - let doc = print_expression ~state expr cmt_tbl in - match Parens.expr_allowing_coercion expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc - in - (* Print trailing comments with the expression *) - let expr_with_trailing_comments = - print_trailing_comments expr_doc cmt_tbl.Comment_table.trailing - expr.Parsetree.pexp_loc + print_trailing_comments (expr_doc expr) cmt_tbl.Comment_table.trailing + expr.pexp_loc in - Doc.concat - [ - comma_before_spread; - leading_comments_doc; - Doc.dotdotdot; - expr_with_trailing_comments; - ] - | None -> Doc.nil - in - let make_sub_list_doc (expressions, spread) = - let comma_before_spread = - match expressions with - | [] -> Doc.nil - | _ -> Doc.concat [Doc.text ","; Doc.line] - in - let spread_doc = make_spread_doc comma_before_spread spread in - let expressions_doc = - Doc.join - ~sep:(Doc.concat [Doc.text ","; Doc.line]) - (List.map - (fun expr -> - let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr_allowing_coercion expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc) - expressions) - in - Doc.concat [expressions_doc; spread_doc] + Doc.concat [leading_comments_doc; Doc.dotdotdot; expr_doc] in Doc.group (Doc.concat [ - Doc.lbracket; + (match kind with + | Spread_array -> Doc.lbracket + | Spread_list -> Doc.text "list{"); Doc.indent (Doc.concat [ Doc.soft_line; Doc.join ~sep:(Doc.concat [Doc.text ","; Doc.line]) - (List.map make_sub_list_doc - (List.map Parsetree_viewer.collect_array_expressions - sub_lists)); + (List.map element_doc elements); ]); Doc.trailing_comma; Doc.soft_line; - Doc.rbracket; - ]) - -and print_list_spread_apply ~state sub_lists cmt_tbl = - let make_spread_doc comma_before_spread = function - | Some expr -> - Doc.concat - [ - comma_before_spread; - Doc.dotdotdot; - (let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr_allowing_coercion expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc); - ] - | None -> Doc.nil - in - let make_sub_list_doc (expressions, spread) = - let comma_before_spread = - match expressions with - | [] -> Doc.nil - | _ -> Doc.concat [Doc.text ","; Doc.line] - in - let spread_doc = make_spread_doc comma_before_spread spread in - Doc.concat - [ - Doc.join - ~sep:(Doc.concat [Doc.text ","; Doc.line]) - (List.map - (fun expr -> - let doc = print_expression_with_comments ~state expr cmt_tbl in - match Parens.expr_allowing_coercion expr with - | Parens.Parenthesized -> add_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc) - expressions); - spread_doc; - ] - in - Doc.group - (Doc.concat - [ - Doc.text "list{"; - Doc.indent - (Doc.concat - [ - Doc.soft_line; - Doc.join - ~sep:(Doc.concat [Doc.text ","; Doc.line]) - (List.map make_sub_list_doc - (List.map Parsetree_viewer.collect_list_expressions - sub_lists)); - ]); - Doc.trailing_comma; - Doc.soft_line; - Doc.rbrace; + (match kind with + | Spread_array -> Doc.rbracket + | Spread_list -> Doc.rbrace); ]) (* callExpr(arg1, arg2) *) diff --git a/tests/analysis_tests/tests/src/SpreadLiterals.res b/tests/analysis_tests/tests/src/SpreadLiterals.res new file mode 100644 index 00000000000..7b26a55ca5d --- /dev/null +++ b/tests/analysis_tests/tests/src/SpreadLiterals.res @@ -0,0 +1,20 @@ +let xs = [1, 2] +let ys = list{1, 2} + +let a = [0, ...xs] +// ^ast + +// Spread literals get an inferred-type hint +let arrays = [...xs, 3] +let lists = list{...ys, 3, ...ys} +//^hin + +// Only the spread identifiers get semantic tokens +let tokens = [...xs, ...xs] +//^hig + +// let x = [...xs, 3]->le +// ^com + +// let x = list{...ys, 3, ...ys}->le +// ^com diff --git a/tests/analysis_tests/tests/src/expected/SpreadLiterals.res.txt b/tests/analysis_tests/tests/src/expected/SpreadLiterals.res.txt new file mode 100644 index 00000000000..a4121c46139 --- /dev/null +++ b/tests/analysis_tests/tests/src/expected/SpreadLiterals.res.txt @@ -0,0 +1,119 @@ +Dump AST src/SpreadLiterals.res 3:8 + +Source: +// let a = [0, ...xs] +// ^ast + +<*>Pstr_value( + value: + Ppat_var(a) + expr: + <*>Pexp_spread(array, + Pexp_constant(Pconst_integer(0)) + ...Pexp_ident:xs + ) +) + +Inlay Hint src/SpreadLiterals.res 1:34 +[ + { + "kind": 1, + "label": ": array", + "paddingLeft": true, + "paddingRight": false, + "position": { "character": 10, "line": 12 } + }, + { + "kind": 1, + "label": ": list", + "paddingLeft": true, + "paddingRight": false, + "position": { "character": 9, "line": 8 } + }, + { + "kind": 1, + "label": ": array", + "paddingLeft": true, + "paddingRight": false, + "position": { "character": 10, "line": 7 } + }, + { + "kind": 1, + "label": ": array", + "paddingLeft": true, + "paddingRight": false, + "position": { "character": 5, "line": 3 } + }, + { + "kind": 1, + "label": ": list", + "paddingLeft": true, + "paddingRight": false, + "position": { "character": 6, "line": 1 } + } +] + +Highlight src/SpreadLiterals.res +structure items:6 diagnostics:0 +Variable: xs [0:4->0:6] +Variable: ys [1:4->1:6] +Variable: a [3:4->3:5] +Lident: xs 3:15 Variable +Variable: arrays [7:4->7:10] +Lident: xs 7:17 Variable +Variable: lists [8:4->8:9] +Lident: ys 8:20 Variable +Lident: ys 8:30 Variable +Variable: tokens [12:4->12:10] +Lident: xs 12:17 Variable +Lident: xs 12:24 Variable + +Complete src/SpreadLiterals.res 15:24 +pos_cursor:[15:24] pos_no_white:[15:23] Found expr:[15:11->15:25] +Completable: Cpath Value[Primitive_array, spread](Nolabel)->le +Package opens Stdlib.place holder Pervasives.JsxModules.place holder +Resolved opens 1 Stdlib +ContextPath Value[Primitive_array, spread](Nolabel)->le +ContextPath Value[Primitive_array, spread](Nolabel) +ContextPath Value[Primitive_array, spread] +Path Primitive_array.spread +Path Stdlib.Array.le +Path ArrayUtils.le +Path le +[ + { + "detail": "array<'a> => int", + "documentation": { + "kind": "markdown", + "value": "\n`length(array)` returns the length of (i.e. number of items in) the array.\n\nSee [`Array.length`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/length) on MDN.\n\n## Examples\n\n```rescript\nlet someArray = [\"hi\", \"hello\"]\n\nsomeArray->Array.length == 2\n```\n" + }, + "kind": 12, + "label": "Array.length", + "tags": [] + } +] + +Complete src/SpreadLiterals.res 18:35 +pos_cursor:[18:35] pos_no_white:[18:34] Found expr:[18:11->18:36] +Completable: Cpath Value[Primitive_list, spread](Nolabel)->le +Package opens Stdlib.place holder Pervasives.JsxModules.place holder +Resolved opens 1 Stdlib +ContextPath Value[Primitive_list, spread](Nolabel)->le +ContextPath Value[Primitive_list, spread](Nolabel) +ContextPath Value[Primitive_list, spread] +Path Primitive_list.spread +Path Stdlib.List.le +Path le +[ + { + "detail": "list<'a> => int", + "documentation": { + "kind": "markdown", + "value": "\n`length(list)` returns the length of `list`.\n\n## Examples\n\n```rescript\nList.length(list{1, 2, 3}) == 3\n```\n" + }, + "kind": 12, + "label": "List.length", + "tags": [] + } +] + diff --git a/tests/build_tests/super_errors/expected/array_spread_annotation_mismatch.res.expected b/tests/build_tests/super_errors/expected/array_spread_annotation_mismatch.res.expected new file mode 100644 index 00000000000..51b4b1cbb34 --- /dev/null +++ b/tests/build_tests/super_errors/expected/array_spread_annotation_mismatch.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/array_spread_annotation_mismatch.res:4:28-29 + + 2 │ + 3 │ // The error points at the spread, not at the whole literal + 4 │ let a: array = [...xs, 2] + 5 │ + + This has type: array + But it's expected to have type: array + + The incompatible parts: + int vs string + + You can convert int to string with Int.toString. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/array_spread_type_mismatch.res.expected b/tests/build_tests/super_errors/expected/array_spread_type_mismatch.res.expected new file mode 100644 index 00000000000..2cfbda1493e --- /dev/null +++ b/tests/build_tests/super_errors/expected/array_spread_type_mismatch.res.expected @@ -0,0 +1,11 @@ + + We've found a bug for you! + /.../fixtures/array_spread_type_mismatch.res:4:16-17 + + 2 │ + 3 │ // The error points at the spread list + 4 │ let a = [1, ...xs] + 5 │ + + This has type: list + But it's expected to have type: array \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/list_spread_type_mismatch.res.expected b/tests/build_tests/super_errors/expected/list_spread_type_mismatch.res.expected new file mode 100644 index 00000000000..75a8d538c21 --- /dev/null +++ b/tests/build_tests/super_errors/expected/list_spread_type_mismatch.res.expected @@ -0,0 +1,11 @@ + + We've found a bug for you! + /.../fixtures/list_spread_type_mismatch.res:4:20-21 + + 2 │ + 3 │ // The error points at the spread array + 4 │ let l = list{1, ...xs, 2, ...xs} + 5 │ + + This has type: array + But it's expected to have type: list \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/array_spread_annotation_mismatch.res b/tests/build_tests/super_errors/fixtures/array_spread_annotation_mismatch.res new file mode 100644 index 00000000000..e7acbb6fd9d --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/array_spread_annotation_mismatch.res @@ -0,0 +1,4 @@ +let xs = [1] + +// The error points at the spread, not at the whole literal +let a: array = [...xs, 2] diff --git a/tests/build_tests/super_errors/fixtures/array_spread_type_mismatch.res b/tests/build_tests/super_errors/fixtures/array_spread_type_mismatch.res new file mode 100644 index 00000000000..cfb36d05726 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/array_spread_type_mismatch.res @@ -0,0 +1,4 @@ +let xs = list{1} + +// The error points at the spread list +let a = [1, ...xs] diff --git a/tests/build_tests/super_errors/fixtures/list_spread_type_mismatch.res b/tests/build_tests/super_errors/fixtures/list_spread_type_mismatch.res new file mode 100644 index 00000000000..54ca6dc7325 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/list_spread_type_mismatch.res @@ -0,0 +1,4 @@ +let xs = [1] + +// The error points at the spread array +let l = list{1, ...xs, 2, ...xs} diff --git a/tests/ounit_tests/ounit_ast_mapper0_tests.ml b/tests/ounit_tests/ounit_ast_mapper0_tests.ml index bf4a9de4142..94ae5155408 100644 --- a/tests/ounit_tests/ounit_ast_mapper0_tests.ml +++ b/tests/ounit_tests/ounit_ast_mapper0_tests.ml @@ -2143,6 +2143,100 @@ let test_dict_literal_through_ast0 _ = () | _ -> assert_failure "Expected a call of Primitive_dict.make" +(* On the v0 wire, array and list literals with spreads are the calls they used + to be parsed into, see [Ast_spread]. They come back as [Pexp_spread]. *) +let test_spread_literal_through_ast0 _ = + List.iter + (fun (source, wire_module) -> + let parsed = + Res_driver.parse_implementation_from_source + ~display_filename:"Spread.res" ~source:("let x = " ^ source) + in + match parsed.parsetree with + | [{pstr_desc = Pstr_value (_, [{pvb_expr = e}])}] when not parsed.invalid + -> ( + let wire = map_expr_to0 e in + (match wire.pexp_desc with + | Parsetree0.Pexp_apply + ( { + pexp_desc = + Pexp_ident {txt = Longident.Ldot (Lident name, "spread")}; + pexp_attributes; + }, + _ ) -> + OUnit.assert_equal ~msg:source wire_module name; + OUnit.assert_equal ~msg:source ~printer:(String.concat ", ") + ["res.spread"] + (attr_names pexp_attributes) + | _ -> assert_failure ("Expected a spread call: " ^ source)); + match map_expr0 wire with + | {pexp_desc = Pexp_spread _} as back -> + (* Locations included: the parsed literal has real ones *) + OUnit.assert_bool (source ^ " roundtrips through ast0") (back = e) + | _ -> assert_failure ("Expected Pexp_spread back: " ^ source)) + | _ -> assert_failure ("Expected a let binding: " ^ source)) + [ + ("[...a]", "Primitive_array"); + ("[1, ...a, 2, 3, ...b, 4]", "Primitive_array"); + ("[...[1, 2], 3, ...[]]", "Primitive_array"); + ("[...[...a, 1], ...a]", "Primitive_array"); + ("@attr [1, ...(a: array)]", "Primitive_array"); + ("list{...a, 1}", "Primitive_list"); + ("list{1, ...a, 2, 3, ...b}", "Primitive_list"); + ("list{...list{1, 2}, ...a}", "Primitive_list"); + ("list{...list{...a, 1}, ...a, 2}", "Primitive_list"); + ]; + (* A call built by a PPX: parts that aren't ghost array literals are spreads, + which concatenates the same arrays *) + let spread_call parts = + Ast_helper0.Exp.apply ~loc + (Ast_helper0.Exp.ident ~loc + ~attrs:[attr "res.spread" (Parsetree0.PStr [])] + (located_string + (Longident.Ldot (Longident.Lident "Primitive_array", "spread")))) + [(Asttypes.Noloc.Nolabel, Ast_helper0.Exp.array ~loc parts)] + in + let one = + Ast_helper0.Exp.constant ~loc (Parsetree0.Pconst_integer ("1", None)) + in + let a = Ast_helper0.Exp.ident ~loc (located_string (Longident.Lident "a")) in + (match + (map_expr0 + (spread_call [Ast_helper0.Exp.array ~loc:(source_loc 0 3) [one]; a])) + .pexp_desc + with + | Pexp_spread + ( Spread_array, + [ + Pspread {pexp_desc = Pexp_array [_]}; + Pspread {pexp_desc = Pexp_ident _}; + ] ) -> + () + | _ -> assert_failure "Expected the spreads of [1] and a"); + (match + (map_expr0 (spread_call [Ast_helper0.Exp.array ~loc [one]; a])).pexp_desc + with + | Pexp_spread + ( Spread_array, + [ + Pspread_item {pexp_desc = Pexp_constant _}; + Pspread {pexp_desc = Pexp_ident _}; + ] ) -> + () + | _ -> assert_failure "Expected the item 1 and the spread of a"); + (* Without the marker, it stays a call *) + match + (map_expr0 + (Ast_helper0.Exp.apply ~loc + (Ast_helper0.Exp.ident ~loc + (located_string + (Longident.Ldot (Longident.Lident "Primitive_array", "spread")))) + [(Asttypes.Noloc.Nolabel, Ast_helper0.Exp.array ~loc [a])])) + .pexp_desc + with + | Pexp_apply _ -> () + | _ -> assert_failure "Expected a call of Primitive_array.spread" + let test_removed_regexp_extension _ = List.iter (fun source -> @@ -2711,6 +2805,7 @@ let suites = "variant_spread_pattern_through_ast0" >:: test_variant_spread_pattern_through_ast0; "dict_literal_through_ast0" >:: test_dict_literal_through_ast0; + "spread_literal_through_ast0" >:: test_spread_literal_through_ast0; "dict_pattern_through_ast0" >:: test_dict_pattern_through_ast0; "inline_record_definition_roundtrips_through_ast0" >:: test_inline_record_definition_roundtrips_through_ast0; diff --git a/tests/syntax_tests/data/ast-mapping/SpreadLiterals.res b/tests/syntax_tests/data/ast-mapping/SpreadLiterals.res new file mode 100644 index 00000000000..6fae06a592e --- /dev/null +++ b/tests/syntax_tests/data/ast-mapping/SpreadLiterals.res @@ -0,0 +1,15 @@ +let a = [...xs] +let a = [1, ...xs, 2, 3, ...ys, 4] +let a = [...[1, 2], 3, ...[]] +let a = [...[...xs, 1], ...ys] +let a = @attr [1, ...(xs: array)] +let a = [/* before */ ...xs, 1] + +let l = list{...xs, 1} +let l = list{1, ...xs, 2, 3, ...ys} +let l = list{...list{1, 2}, ...xs} +let l = list{...list{...xs, 1}, ...xs, 2} +let l = @attr list{...xs, 1} +let l = list{1, ...xs} +let userArraySpread = Primitive_array.spread([[1], xs]) +let userListSpread = Primitive_list.spread([list{1}, xs]) diff --git a/tests/syntax_tests/data/ast-mapping/expected/SpreadLiterals.res.txt b/tests/syntax_tests/data/ast-mapping/expected/SpreadLiterals.res.txt new file mode 100644 index 00000000000..6fae06a592e --- /dev/null +++ b/tests/syntax_tests/data/ast-mapping/expected/SpreadLiterals.res.txt @@ -0,0 +1,15 @@ +let a = [...xs] +let a = [1, ...xs, 2, 3, ...ys, 4] +let a = [...[1, 2], 3, ...[]] +let a = [...[...xs, 1], ...ys] +let a = @attr [1, ...(xs: array)] +let a = [/* before */ ...xs, 1] + +let l = list{...xs, 1} +let l = list{1, ...xs, 2, 3, ...ys} +let l = list{...list{1, 2}, ...xs} +let l = list{...list{...xs, 1}, ...xs, 2} +let l = @attr list{...xs, 1} +let l = list{1, ...xs} +let userArraySpread = Primitive_array.spread([[1], xs]) +let userListSpread = Primitive_list.spread([list{1}, xs]) diff --git a/tests/syntax_tests/data/parsing/grammar/expressions/expected/array.res.txt b/tests/syntax_tests/data/parsing/grammar/expressions/expected/array.res.txt index af12c028724..89a2d8bc743 100644 --- a/tests/syntax_tests/data/parsing/grammar/expressions/expected/array.res.txt +++ b/tests/syntax_tests/data/parsing/grammar/expressions/expected/array.res.txt @@ -1,8 +1,8 @@ let x = [|1;2;3|] let x = [|1;2;3|] let x = [|(1 : int);(2 : int);(3 : int)|] -let x = ((Primitive_array.spread)[@res.spread ]) [|[|4;5|];y|] -let x = ((Primitive_array.spread)[@res.spread ]) [|[|4;5|];y;[|7|];y|] -let x = ((Primitive_array.spread)[@res.spread ]) [|[|4;5|];(y : int array)|] -let x = ((Primitive_array.spread)[@res.spread ]) [|[|4;5;k|];y|] -let x = ((Primitive_array.spread)[@res.spread ]) [|y|] \ No newline at end of file +let x = [4, 5, ...y] +let x = [4, 5, ...y, 7, ...y] +let x = [4, 5, ...(y : int array)] +let x = [4, 5, k, ...y] +let x = [...y] \ No newline at end of file diff --git a/tests/syntax_tests/data/parsing/grammar/expressions/expected/list.res.txt b/tests/syntax_tests/data/parsing/grammar/expressions/expected/list.res.txt index 8506759923e..29bb44c082a 100644 --- a/tests/syntax_tests/data/parsing/grammar/expressions/expected/list.res.txt +++ b/tests/syntax_tests/data/parsing/grammar/expressions/expected/list.res.txt @@ -3,5 +3,5 @@ let x = [1; 2; 3] let x = [1; 2; 3] let x = [(1 : int); (2 : int); (3 : int)] let x = 4 :: 5 :: y -let x = ((Primitive_list.spread)[@res.spread ]) [|(1 :: x);(2 :: 3 :: x)|] +let x = list{1, ...x, 2, 3, ...x} let x = 1 :: 2 :: (y : int list) \ No newline at end of file diff --git a/tests/syntax_tests/data/parsing/recovery/expression/expected/list.res.txt b/tests/syntax_tests/data/parsing/recovery/expression/expected/list.res.txt index 52cb3b7918a..9d046c2d292 100644 --- a/tests/syntax_tests/data/parsing/recovery/expression/expected/list.res.txt +++ b/tests/syntax_tests/data/parsing/recovery/expression/expected/list.res.txt @@ -40,8 +40,7 @@ let flags = let rec loop [arity:1]items = {match items with | [|{js|-pp|js};_ppFlag;rest|] -> loop rest - | [|x;rest|] -> - ((Primitive_array.spread)[@res.spread ]) [|[|x|];(loop rest)|] + | [|x;rest|] -> [x, ...(loop rest)] | [||] -> [||]} in (loop parts) -> (String.concat {js| |js})} : flags \ No newline at end of file diff --git a/tests/syntax_tests/data/printer/comments/array.res b/tests/syntax_tests/data/printer/comments/array.res index 8bb35e20cd3..675e9fee965 100644 --- a/tests/syntax_tests/data/printer/comments/array.res +++ b/tests/syntax_tests/data/printer/comments/array.res @@ -52,3 +52,12 @@ let _ = [ // comment 3 b, c ] + +let _ = list{ + // comment 0 + ...xs, + // comment 1 + a, + /* comment 2 */ ...ys /* comment 3 */, + b, +} diff --git a/tests/syntax_tests/data/printer/comments/expected/array.res.txt b/tests/syntax_tests/data/printer/comments/expected/array.res.txt index 2b22a80327f..f333107201a 100644 --- a/tests/syntax_tests/data/printer/comments/expected/array.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/array.res.txt @@ -49,3 +49,12 @@ let _ = [ b, c, ] + +let _ = list{ + // comment 0 + ...xs, + // comment 1 + a, + /* comment 2 */ ...ys /* comment 3 */, + b, +} diff --git a/tests/syntax_tests/data/printer/expr/array.res b/tests/syntax_tests/data/printer/expr/array.res index c004b465002..b4caf8d0dc5 100644 --- a/tests/syntax_tests/data/printer/expr/array.res +++ b/tests/syntax_tests/data/printer/expr/array.res @@ -20,3 +20,15 @@ let x = @attr [ ] let x = [biggerThanZero ? 1 : 0, biggerThanZero ? 1 : 0] + +let x = [...xs] +let x = [1, ...xs, 2, 3, ...ys] +let x = [...[1, 2], 3, ...[]] +let x = [...[...xs, 1], ...ys] +let x = @attr [1, ...xs] +let x = [...(xs: array), ...ys->Array.map(f)] +let x = [ + ...superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, + superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, + ...superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, +] diff --git a/tests/syntax_tests/data/printer/expr/expected/array.res.txt b/tests/syntax_tests/data/printer/expr/expected/array.res.txt index f317af7a860..8565e0dc359 100644 --- a/tests/syntax_tests/data/printer/expr/expected/array.res.txt +++ b/tests/syntax_tests/data/printer/expr/expected/array.res.txt @@ -22,3 +22,15 @@ let x = ] let x = [biggerThanZero ? 1 : 0, biggerThanZero ? 1 : 0] + +let x = [...xs] +let x = [1, ...xs, 2, 3, ...ys] +let x = [...[1, 2], 3, ...[]] +let x = [...[...xs, 1], ...ys] +let x = @attr [1, ...xs] +let x = [...(xs: array), ...ys->Array.map(f)] +let x = [ + ...superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, + superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, + ...superLongIdentiiiiiiiifffffffiiiiieeeeeeeer, +] diff --git a/tests/syntax_tests/data/printer/expr/expected/list.res.txt b/tests/syntax_tests/data/printer/expr/expected/list.res.txt index a2d1b7a0c22..f3123a9c4f0 100644 --- a/tests/syntax_tests/data/printer/expr/expected/list.res.txt +++ b/tests/syntax_tests/data/printer/expr/expected/list.res.txt @@ -44,3 +44,7 @@ let x = list{ superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, ...superLoooooooooooooooooooooooooooooongListHere, } + +let x = list{...list{1, 2}, ...xs} +let x = list{...xs, ...list{}, 1} +let x = @attr list{...xs, 1} diff --git a/tests/syntax_tests/data/printer/expr/list.res b/tests/syntax_tests/data/printer/expr/list.res index 86b02f08e84..ef5b2f62251 100644 --- a/tests/syntax_tests/data/printer/expr/list.res +++ b/tests/syntax_tests/data/printer/expr/list.res @@ -46,3 +46,7 @@ let x = list{ superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, ...superLoooooooooooooooooooooooooooooongListHere, } + +let x = list{...list{1, 2}, ...xs} +let x = list{...xs, ...list{}, 1} +let x = @attr list{...xs, 1} From 6562b42cdeca6b4e1c2eb425120f0e4505203f0d Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 27 Oct 2026 07:00:46 +0000 Subject: [PATCH 2/5] Move the v0 spread encoding into the PPX bridge and test spread literals The res.spread call is only the v0 PPX wire format, so it now lives in the bridge: Ast_mapper_to0 builds it (spread_call, spread_marker) and Ast_mapper_from0 decodes it, with the module-name lookup local to the decoder. Ast_spread keeps what the type checker and analysis use: groups, parts and module_name. The ghost locations of the generated parts stay, as for any node the compiler makes up. Behaviour is unchanged. The new mocha test runs array literals with several spreads, items between them, a leading item and an empty spread, list literals with spreads before the tail and a tail-only spread, and checks that both kinds evaluate their items and spreads left to right; its generated JavaScript pins the Primitive_array.spread and Primitive_list.spread calls and the order of their parts. Area: compiler/ml tests Output: unchanged Tests: +tests/tests/src/spread_literal_test.res Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/ml/ast_mapper_from0.ml | 15 ++-- compiler/ml/ast_mapper_to0.ml | 23 +++++- compiler/ml/ast_spread.ml | 32 ++------ tests/tests/src/spread_literal_test.mjs | 98 +++++++++++++++++++++++++ tests/tests/src/spread_literal_test.res | 33 +++++++++ 5 files changed, 170 insertions(+), 31 deletions(-) create mode 100644 tests/tests/src/spread_literal_test.mjs create mode 100644 tests/tests/src/spread_literal_test.res diff --git a/compiler/ml/ast_mapper_from0.ml b/compiler/ml/ast_mapper_from0.ml index 106d9adae99..b2e74930cb9 100644 --- a/compiler/ml/ast_mapper_from0.ml +++ b/compiler/ml/ast_mapper_from0.ml @@ -679,8 +679,13 @@ module E = struct | _ -> Option.map rows (dict_rows ~marker:Ast_dict.literal_marker e) (* The kind and elements of an array or list literal with spreads encoded as - a call, see [Ast_spread] *) + a call, see [Ast_mapper_to0.spread_call] *) let spread_elements (e : expression) = + let kind_of_module_name name = + if name = Primitive_modules.array then Some Pt.Spread_array + else if name = Primitive_modules.list then Some Pt.Spread_list + else None + in let generated (e : expression) = e.pexp_loc.loc_ghost && e.pexp_attributes = [] in @@ -710,11 +715,11 @@ module E = struct ( Asttypes.Noloc.Nolabel, {pexp_desc = Pexp_array parts; pexp_attributes = []} ); ] ) - when txt = Ast_spread.marker -> ( - match Ast_spread.kind_of_module_name module_name with - | Some Spread_array -> + when txt = Ast_mapper_to0.spread_marker -> ( + match kind_of_module_name module_name with + | Some Pt.Spread_array -> Some (Pt.Spread_array, List.concat_map array_part parts) - | Some Spread_list -> + | Some Pt.Spread_list -> Some (Pt.Spread_list, List.concat_map list_part parts) | None -> None) | _ -> None diff --git a/compiler/ml/ast_mapper_to0.ml b/compiler/ml/ast_mapper_to0.ml index 2cdb48c22a3..3e23688c761 100644 --- a/compiler/ml/ast_mapper_to0.ml +++ b/compiler/ml/ast_mapper_to0.ml @@ -124,6 +124,26 @@ let remove_ppx_context_string_attr attrs = let map_loc sub {loc; txt} = {loc = sub.location sub loc; txt} +(* v0 has no array or list literals with spreads, [Pexp_spread]: they're the + calls of [Primitive_array.spread] and [Primitive_list.spread] they used to be + parsed into, whose [spread] carries the [res.spread] marker. The argument is + an array of the [Ast_spread.parts]; [Ast_mapper_from0] decodes the parts + with ghost locations back into the items. *) +let spread_marker = "res.spread" + +let spread_call ~loc ~attrs kind elements = + Ast_helper.Exp.apply ~loc ~attrs + (Ast_helper.Exp.ident ~loc + ~attrs:[(Location.mknoloc spread_marker, PStr [])] + (Location.mkloc + (Longident.Ldot + (Longident.Lident (Ast_spread.module_name kind), "spread")) + loc)) + [ + ( Asttypes.Nolabel, + Ast_helper.Exp.array ~loc (Ast_spread.parts kind elements) ); + ] + (* Internal Parsetree0 bridge metadata; public res.* attributes pass through. *) let record_rest_attr_name = "_res.record_rest" let constructor_args_attr_name = "_res.constructor_args" @@ -708,8 +728,7 @@ module E = struct (* v0 has no spread literals: they're the calls they used to be parsed into *) sub.expr sub - (Ast_spread.to_call ~loc:e.pexp_loc ~attrs:e.pexp_attributes kind - elements) + (spread_call ~loc:e.pexp_loc ~attrs:e.pexp_attributes kind elements) | Pexp_ifthenelse (e1, e2, e3) -> ifthenelse ~loc ~attrs (sub.expr sub e1) (sub.expr sub e2) (map_opt (sub.expr sub) e3) diff --git a/compiler/ml/ast_spread.ml b/compiler/ml/ast_spread.ml index 1c8d9f27bf3..cdf55030b37 100644 --- a/compiler/ml/ast_spread.ml +++ b/compiler/ml/ast_spread.ml @@ -1,28 +1,22 @@ (* Array and list literals with spreads, [Pexp_spread]: [[a, ...b]] and - [list{...a, b}]. They used to be parsed into calls of the runtime's - [Primitive_array.spread] and [Primitive_list.spread], which remain their - encoding on the v0 PPX wire and the calls they compile to. The [spread] - carries the [res.spread] marker, and its argument is an array of parts: + [list{...a, b}]. They compile to calls of the runtime's + [Primitive_array.spread] and [Primitive_list.spread], whose argument is an + array of parts: - for arrays, the spreads, and array literals of the items between them; - for lists, the items before each spread consed onto it ([a :: b :: spread]), and a list literal of the items after the last one. - The array literals and [::] and [[]] constructors built from the items have - ghost locations, which tells them apart from array and list literals that - are spread. *) + The type checker types a literal as its parts, so the items keep the errors + of array and list literals. The array literals and [::] and [[]] + constructors built from the items have ghost locations, like any node the + compiler makes up; on the v0 PPX wire they also tell the items apart from + array and list literals that are spread, see [Ast_mapper_to0.spread_call]. *) open Parsetree -let marker = "res.spread" - let module_name = function | Spread_array -> Primitive_modules.array | Spread_list -> Primitive_modules.list -let kind_of_module_name name = - if name = Primitive_modules.array then Some Spread_array - else if name = Primitive_modules.list then Some Spread_list - else None - (* The items before each spread with that spread, and the items after the last spread *) let groups (elements : spread_element list) : @@ -83,13 +77,3 @@ let parts kind elements = items tail; ]) (groups elements) - -(* The call encoding the literal [kind] of [elements] at [loc] *) -let to_call ~loc ~attrs kind elements = - Ast_helper.Exp.apply ~loc ~attrs - (Ast_helper.Exp.ident ~loc - ~attrs:[(Location.mknoloc marker, PStr [])] - (Location.mkloc - (Longident.Ldot (Longident.Lident (module_name kind), "spread")) - loc)) - [(Asttypes.Nolabel, Ast_helper.Exp.array ~loc (parts kind elements))] diff --git a/tests/tests/src/spread_literal_test.mjs b/tests/tests/src/spread_literal_test.mjs new file mode 100644 index 00000000000..bbf288c2bc9 --- /dev/null +++ b/tests/tests/src/spread_literal_test.mjs @@ -0,0 +1,98 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; +import * as Stdlib_List from "@rescript/runtime/lib/es6/Stdlib_List.mjs"; +import * as Primitive_list from "@rescript/runtime/lib/es6/Primitive_list.mjs"; +import * as Primitive_array from "@rescript/runtime/lib/es6/Primitive_array.mjs"; + +let log = { + contents: /* [] */0 +}; + +function note(s, v) { + log.contents = {hd: s, tl: log.contents}; + return v; +} + +Mocha.describe("Spread_literal_test", () => { + Mocha.test("array spreads keep items and spreads in order", () => { + let a = [ + 1, + 2 + ]; + let b = [5]; + Test_utils.eq("File \"spread_literal_test.res\", line 14, characters 7-14", Primitive_array.spread([ + a, + [ + 3, + 4 + ], + b, + [6] + ]), [ + 1, + 2, + 3, + 4, + 5, + 6 + ]); + Test_utils.eq("File \"spread_literal_test.res\", line 15, characters 7-14", Primitive_array.spread([ + [0], + a, + [], + b + ]), [ + 0, + 1, + 2, + 5 + ]); + }); + Mocha.test("list spreads keep items and spreads in order", () => { + let xs = {hd: 1, tl: {hd: 2, tl: /* [] */0}}; + let ys = {hd: 5, tl: /* [] */0}; + Test_utils.eq("File \"spread_literal_test.res\", line 21, characters 7-14", Primitive_list.spread([ + xs, + {hd: 3, tl: ys}, + {hd: 6, tl: /* [] */0} + ]), {hd: 1, tl: {hd: 2, tl: {hd: 3, tl: {hd: 5, tl: {hd: 6, tl: /* [] */0}}}}}); + Test_utils.eq("File \"spread_literal_test.res\", line 22, characters 7-14", Primitive_list.spread([ + {hd: 0, tl: xs}, + /* [] */0, + {hd: 4, tl: ys} + ]), {hd: 0, tl: {hd: 1, tl: {hd: 2, tl: {hd: 4, tl: {hd: 5, tl: /* [] */0}}}}}); + Test_utils.eq("File \"spread_literal_test.res\", line 23, characters 7-14", {hd: 0, tl: xs}, {hd: 0, tl: {hd: 1, tl: {hd: 2, tl: /* [] */0}}}); + }); + Mocha.test("items and spreads are evaluated left to right", () => { + log.contents = /* [] */0; + let arr = Primitive_array.spread([ + [note("a", 1)], + note("b", [2]), + [note("c", 3)] + ]); + let lst = Primitive_list.spread([ + {hd: note("d", 4), tl: note("e", {hd: 5, tl: /* [] */0})}, + {hd: note("f", 6), tl: /* [] */0} + ]); + Test_utils.eq("File \"spread_literal_test.res\", line 30, characters 7-14", [ + arr, + lst + ], [ + [ + 1, + 2, + 3 + ], + {hd: 4, tl: {hd: 5, tl: {hd: 6, tl: /* [] */0}}} + ]); + Test_utils.eq("File \"spread_literal_test.res\", line 31, characters 7-14", Stdlib_List.reverse(log.contents), {hd: "a", tl: {hd: "b", tl: {hd: "c", tl: {hd: "d", tl: {hd: "e", tl: {hd: "f", tl: /* [] */0}}}}}}); + }); +}); + +export { + log, + note, +} +/* Not a pure module */ diff --git a/tests/tests/src/spread_literal_test.res b/tests/tests/src/spread_literal_test.res new file mode 100644 index 00000000000..660491eacb8 --- /dev/null +++ b/tests/tests/src/spread_literal_test.res @@ -0,0 +1,33 @@ +open Mocha +open Test_utils + +let log = ref(list{}) +let note = (s, v) => { + log := list{s, ...log.contents} + v +} + +describe(__MODULE__, () => { + test("array spreads keep items and spreads in order", () => { + let a = [1, 2] + let b = [5] + eq(__LOC__, [...a, 3, 4, ...b, 6], [1, 2, 3, 4, 5, 6]) + eq(__LOC__, [0, ...a, ...[], ...b], [0, 1, 2, 5]) + }) + + test("list spreads keep items and spreads in order", () => { + let xs = list{1, 2} + let ys = list{5} + eq(__LOC__, list{...xs, 3, ...ys, 6}, list{1, 2, 3, 5, 6}) + eq(__LOC__, list{0, ...xs, ...list{}, 4, ...ys}, list{0, 1, 2, 4, 5}) + eq(__LOC__, list{0, ...xs}, list{0, 1, 2}) + }) + + test("items and spreads are evaluated left to right", () => { + log := list{} + let arr = [note("a", 1), ...note("b", [2]), note("c", 3)] + let lst = list{note("d", 4), ...note("e", list{5}), note("f", 6)} + eq(__LOC__, (arr, lst), ([1, 2, 3], list{4, 5, 6})) + eq(__LOC__, List.reverse(log.contents), list{"a", "b", "c", "d", "e", "f"}) + }) +}) From 603b128055ad50b985a1a5044875684d487236d4 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 18 Oct 2026 18:28:26 +0000 Subject: [PATCH 3/5] Keep line comments in operands followed by a block Why: #9061 keeps a line comment in the operand of switch, try, if, while or a loop by putting the operand into parentheses when Doc.has_pending_line_suffix finds the comment still queued at its end. A comment after an item or argument then moves to the end of the operand, away from the item it describes. The operand now first breaks the groups around such a comment (Doc.break_before_line_suffixes), so the comment stays after its item and the parentheses are only needed when it is still pending. A binary expression is left flat: breaking it would flush the comment after the operator, where the next format attaches it to the right operand, so it keeps #9061's parentheses. In an operand with its own parentheses, such as a coercion, the comment also stays after its item inside them (operandLineComment.res). Format modified (line comment in a switch operand): Example: let x = switch [ a, // c b, ] { | _ => 1 } Before (rescript): let x = switch ( [a, b] // c ) { | _ => 1 } After (rescript): let x = switch [ a, // c b, ] { | _ => 1 } Review: Why not make every same-line line comment break its group? That changes code that formats stably today, such as foo(v :> b) // comment, which would split over three lines; the drift only happens before a block. Review: Why are binary operands exempt? With their group broken, `switch a // c\n+ b {` printed `a + // c\nb` and then `a +\n// c\nb` on the next format; flat, it prints `switch (\n a + b // c\n) {` and stays. Area: printer tests Output: unchanged Formatting: operands of switch, try, if, while and loops with a line comment after an item or argument keep it there; binary operands keep #9061's parentheses Formats: none PPX: unchanged Editor: unchanged Tests: +tests/syntax_tests/data/printer/comments/operandBeforeBlock.res Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EUpL6dKCiSwAeriDGRHqNr Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- changes/operand-line-comments.fix.md | 1 + compiler/syntax/src/res_doc.ml | 18 +++ compiler/syntax/src/res_doc.mli | 7 ++ compiler/syntax/src/res_printer.ml | 32 +++-- .../expected/operandBeforeBlock.res.txt | 112 ++++++++++++++++++ .../expected/operandLineComment.res.txt | 30 ++++- .../printer/comments/operandBeforeBlock.res | 63 ++++++++++ 7 files changed, 245 insertions(+), 18 deletions(-) create mode 100644 changes/operand-line-comments.fix.md create mode 100644 tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt create mode 100644 tests/syntax_tests/data/printer/comments/operandBeforeBlock.res diff --git a/changes/operand-line-comments.fix.md b/changes/operand-line-comments.fix.md new file mode 100644 index 00000000000..20fc6d0187c --- /dev/null +++ b/changes/operand-line-comments.fix.md @@ -0,0 +1 @@ +- Keep a line comment after an item or argument in the operand of `switch`, `try`, `if`, `while` or a loop next to that item when formatting, instead of moving it to the end of the operand. diff --git a/compiler/syntax/src/res_doc.ml b/compiler/syntax/src/res_doc.ml index 5ba1a50e8cf..0f93d71ef6c 100644 --- a/compiler/syntax/src/res_doc.ml +++ b/compiler/syntax/src/res_doc.ml @@ -122,6 +122,24 @@ let rec will_break doc = | IfBreaks {yes; no} -> will_break yes || will_break no | _ -> false +(* See documentation in interface file *) +let rec break_before_line_suffixes doc = + match doc with + | LineSuffix _ -> Concat [doc; BreakParent] + | Concat docs -> Concat (List.map break_before_line_suffixes docs) + | Indent doc -> Indent (break_before_line_suffixes doc) + | Group {should_break; doc} -> + Group {should_break; doc = break_before_line_suffixes doc} + | IfBreaks {yes; no; broken} -> + IfBreaks + { + yes = break_before_line_suffixes yes; + no = break_before_line_suffixes no; + broken; + } + | CustomLayout docs -> CustomLayout (List.map break_before_line_suffixes docs) + | Nil | Text _ | LineBreak _ | BreakParent -> doc + (* See documentation in interface file *) let has_pending_line_suffix doc = (* [pending]: a line suffix is queued; [broken]: the innermost enclosing diff --git a/compiler/syntax/src/res_doc.mli b/compiler/syntax/src/res_doc.mli index 13caa1a0c25..d125af5c073 100644 --- a/compiler/syntax/src/res_doc.mli +++ b/compiler/syntax/src/res_doc.mli @@ -41,6 +41,13 @@ val tilde : t val equal : t val trailing_comma : t +val break_before_line_suffixes : t -> t +(** [break_before_line_suffixes doc] makes every line suffix in [doc] break the + groups around it, so that a trailing line comment stays where it was written + instead of moving to the end of the line. Use it for a doc that is followed + by an opening brace on the same line, where the comment would move into the + block. *) + val will_break : t -> bool (** [will_break doc] checks whether [doc] contains forced line breaks. For each [custom_layout], it only inspects the first alternative. diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 043d0f25ab9..b7ba5579307 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3166,10 +3166,18 @@ and print_expression_args ~state (args : Parsetree.expression list) cmt_tbl = ] (* The operand of [switch], [try], [if], [while] or a loop, which is followed - by more code on the same line. A line comment still pending at its end - would move after that code, into the block, so the operand goes into - parentheses with the comment before the [)]. *) -and print_operand_before_brace doc = + by a block on the same line. A line comment in it breaks the groups around + it, so that it stays where it was written; one still pending at the end of + the operand would move into the block, so the operand goes into + parentheses with the comment before the [)]. A binary expression is left + flat: breaking it would print the comment after the operator, where the + next format attaches it to the right operand, so its comment goes into the + parentheses instead. *) +and print_operand_before_brace ~expr doc = + let doc = + if Parsetree_viewer.is_binary_expression expr then doc + else Doc.break_before_line_suffixes doc + in if Doc.has_pending_line_suffix doc then add_operand_parens doc else doc (* Parentheses around such an operand, or around one that needs them, such as @@ -3209,7 +3217,7 @@ and print_if_chain ~state pexp_attributes ifs else_expr cmt_tbl = Doc.concat [ if_txt; - print_operand_before_brace (Doc.group condition); + print_operand_before_brace ~expr:if_expr (Doc.group condition); Doc.space; (let then_expr = match Parsetree_viewer.process_braces then_expr with @@ -3683,7 +3691,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = (Doc.concat [ Doc.text "while "; - print_operand_before_brace + print_operand_before_brace ~expr:expr1 (match Parens.expr expr1 with | Parens.Parenthesized -> condition | _ when Parsetree_viewer.is_block_expr expr1 -> condition @@ -3701,14 +3709,14 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = (let doc = print_expression_with_comments ~state from_expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr:from_expr (match Parens.expr from_expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc from_expr braces | Nothing -> doc)); print_direction_flag direction_flag; (let doc = print_expression_with_comments ~state to_expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr:to_expr (match Parens.expr to_expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc to_expr braces @@ -3726,7 +3734,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = (let doc = print_expression_with_comments ~state array_expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr:array_expr (match Parens.expr array_expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc array_expr braces @@ -3744,7 +3752,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = (let doc = print_expression_with_comments ~state iterable_expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr:iterable_expr (match Parens.expr iterable_expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc iterable_expr braces @@ -3830,7 +3838,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = | Pexp_try (expr, cases, finally) -> let expr_doc = let doc = print_expression_with_comments ~state expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr (match Parens.expr expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc expr braces @@ -3856,7 +3864,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = | Pexp_match (expr, cases) -> let expr_doc = let doc = print_expression_with_comments ~state expr cmt_tbl in - print_operand_before_brace + print_operand_before_brace ~expr (match Parens.expr expr with | Parens.Parenthesized -> add_operand_parens doc | Braced braces -> print_braces doc expr braces diff --git a/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt b/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt new file mode 100644 index 00000000000..cb72a59e795 --- /dev/null +++ b/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt @@ -0,0 +1,112 @@ +// Line comments in an operand followed by a block stay in the operand +let x = switch [ + a, // c + b, +] { +| _ => 1 +} + +let x = switch list{ + a, // c + ...b, + ...c, +} { +| _ => 1 +} + +let x = switch f( + a, // c + b, +) { +| _ => 1 +} + +let x = switch ( + a // c +) { +| _ => 1 +} + +let x = switch a /* c */ { +| _ => 1 +} + +let x = try [ + ...a, // c + b, +] catch { +| _ => 1 +} + +let x = try ( + a // c +) catch { +| _ => 1 +} + +let x = if ( + a // c +) { + 1 +} else { + 2 +} + +let x = if c { + 1 +} else if ( + a // c +) { + 2 +} else { + 3 +} + +let x = while ( + a // c +) { + () +} + +let x = for i in ( + a // c +) to ( + b // d +) { + () +} + +let x = for y of [ + a, // c + b, +] { + () +} + +let x = async () => { + for await y of ( + a // c + ) { + () + } +} + +let x = switch ( + a + b // c +) { +| _ => 1 +} + +let x = if ( + a && b // c +) { + 1 +} else { + 2 +} + +let x = while ( + f(a, b) + 1 > 0 // c +) { + () +} diff --git a/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt b/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt index edce0545f19..c280fb798bc 100644 --- a/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/operandLineComment.res.txt @@ -110,37 +110,55 @@ try ( // An operand printed in its own parentheses keeps the comment inside them switch ( - f(a, b) :> int // c + f( + a, // c + b, + ) :> int ) { | _ => () } switch ( - f(a, b): int // c + f( + a, // c + b, + ): int ) { | _ => () } if ( - f(a, b) :> bool // c + f( + a, // c + b, + ) :> bool ) { () } while ( - f(a, b) :> bool // c + f( + a, // c + b, + ) :> bool ) { () } for x of ( - f(a, b) :> array // c + f( + a, // c + b, + ) :> array ) { () } try ( - f(a, b) :> int // c + f( + a, // c + b, + ) :> int ) catch { | _ => () } diff --git a/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res b/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res new file mode 100644 index 00000000000..6f4d77e1827 --- /dev/null +++ b/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res @@ -0,0 +1,63 @@ +// Line comments in an operand followed by a block stay in the operand +let x = switch [ + a, // c + b, +] { | _ => 1 } + +let x = switch list{ + a, // c + ...b, + ...c, +} { | _ => 1 } + +let x = switch f( + a, // c + b, +) { | _ => 1 } + +let x = switch a // c +{ | _ => 1 } + +let x = switch a /* c */ { | _ => 1 } + +let x = try [ + ...a, // c + b, +] catch { | _ => 1 } + +let x = try a // c +catch { | _ => 1 } + +let x = if a // c +{ 1 } else { 2 } + +let x = if c { 1 } else if a // c +{ 2 } else { 3 } + +let x = while a // c +{ () } + +let x = for i in a // c +to b // d +{ () } + +let x = for y of [ + a, // c + b, +] { () } + +let x = async () => { + for await y of a // c + { () } +} + +let x = switch a // c ++ b { | _ => 1 } + +let x = if a // c +&& b { 1 } else { 2 } + +let x = while f( + a, // c + b, +) + 1 > 0 { () } From a57123e5eefe79d7c0c54c654a68a9ab2f8fb9ad Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 27 Oct 2026 08:19:21 +0000 Subject: [PATCH 4/5] Print the operands of switch, try and loops with one helper The operands of switch, try, for, for..of and for await..of were each printed by the same block: the expression, its parentheses or braces, and print_operand_before_brace. print_block_operand does that once, so the expression is passed to print_operand_before_brace in one place; if and while keep their own condition printing. The output is unchanged. Area: printer Output: unchanged Formatting: unchanged, no snapshot changes Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/syntax/src/res_printer.ml | 62 ++++++++---------------------- 1 file changed, 16 insertions(+), 46 deletions(-) diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index b7ba5579307..9b0eab7b271 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3180,6 +3180,16 @@ and print_operand_before_brace ~expr doc = in if Doc.has_pending_line_suffix doc then add_operand_parens doc else doc +(* An operand followed by a block, see [print_operand_before_brace], with the + parentheses or braces it needs *) +and print_block_operand ~state expr cmt_tbl = + let doc = print_expression_with_comments ~state expr cmt_tbl in + print_operand_before_brace ~expr + (match Parens.expr expr with + | Parens.Parenthesized -> add_operand_parens doc + | Braced braces -> print_braces doc expr braces + | Nothing -> doc) + (* Parentheses around such an operand, or around one that needs them, such as a coercion. They break when a line comment is still pending at the end of the operand, so that the comment stays before the [)]. *) @@ -3706,21 +3716,9 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for "; print_pattern ~state pattern cmt_tbl; Doc.text " in "; - (let doc = - print_expression_with_comments ~state from_expr cmt_tbl - in - print_operand_before_brace ~expr:from_expr - (match Parens.expr from_expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc from_expr braces - | Nothing -> doc)); + print_block_operand ~state from_expr cmt_tbl; print_direction_flag direction_flag; - (let doc = print_expression_with_comments ~state to_expr cmt_tbl in - print_operand_before_brace ~expr:to_expr - (match Parens.expr to_expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc to_expr braces - | Nothing -> doc)); + print_block_operand ~state to_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3731,14 +3729,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for "; print_pattern ~state pattern cmt_tbl; Doc.text " of "; - (let doc = - print_expression_with_comments ~state array_expr cmt_tbl - in - print_operand_before_brace ~expr:array_expr - (match Parens.expr array_expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc array_expr braces - | Nothing -> doc)); + print_block_operand ~state array_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3749,14 +3740,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = Doc.text "for await "; print_pattern ~state pattern cmt_tbl; Doc.text " of "; - (let doc = - print_expression_with_comments ~state iterable_expr cmt_tbl - in - print_operand_before_brace ~expr:iterable_expr - (match Parens.expr iterable_expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc iterable_expr braces - | Nothing -> doc)); + print_block_operand ~state iterable_expr cmt_tbl; Doc.space; print_expression_block ~state ~braces:true body cmt_tbl; ]) @@ -3836,14 +3820,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = | Pexp_sequence _ -> print_expression_block ~state ~braces:true e cmt_tbl | Pexp_let _ -> print_expression_block ~state ~braces:true e cmt_tbl | Pexp_try (expr, cases, finally) -> - let expr_doc = - let doc = print_expression_with_comments ~state expr cmt_tbl in - print_operand_before_brace ~expr - (match Parens.expr expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc) - in + let expr_doc = print_block_operand ~state expr cmt_tbl in let catch_doc = match cases with | [] -> Doc.nil @@ -3862,14 +3839,7 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl = in Doc.concat [Doc.text "try "; expr_doc; catch_doc; finally_doc] | Pexp_match (expr, cases) -> - let expr_doc = - let doc = print_expression_with_comments ~state expr cmt_tbl in - print_operand_before_brace ~expr - (match Parens.expr expr with - | Parens.Parenthesized -> add_operand_parens doc - | Braced braces -> print_braces doc expr braces - | Nothing -> doc) - in + let expr_doc = print_block_operand ~state expr cmt_tbl in Doc.concat [ Doc.text "switch "; From d32ce0b4638f9ec4af9992281914e2b966933139 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 27 Oct 2026 09:25:40 +0000 Subject: [PATCH 5/5] Keep operands with a comment before a nested operator in parentheses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: Only a binary operand was left flat. A binary expression inside another operand, as in `[a // c\n+ b]`, still had its group broken, so the comment was printed after the operator and the next format attached it to the right operand. An operand is now also left flat when a binary expression or pipe in it has its operator at the start of a line, after the left operand, which is where such a comment sits (Parsetree_viewer.has_line_break_before_operator). The comment then goes into #9061's parentheses. A comment after an item that holds a binary expression keeps its place. Format modified (binary expression in an array operand): Example: let x = switch [a // c + b, d] { | _ => 1 } Before (rescript): first run, then second run let x = switch [ a + // c b, d, ] { | _ => 1 } ⇝ let x = switch [ a + // c b, d, ] { | _ => 1 } After (rescript): stable let x = switch ( [a + b, d] // c ) { | _ => 1 } Review: Why look at the line of the operator rather than at the comments? A line comment between the left operand and the operator always puts the operator on a later line, and the check needs only the expression, before printing takes the comments out of the table. A multi-line binary expression without such a comment only costs another comment in the same operand its place next to its item. Area: printer tests Output: unchanged Tests: ~tests/syntax_tests/data/printer/comments/operandBeforeBlock.res Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/syntax/src/res_parsetree_viewer.ml | 23 +++++++++++++++ compiler/syntax/src/res_parsetree_viewer.mli | 6 ++++ compiler/syntax/src/res_printer.ml | 8 +++-- .../expected/operandBeforeBlock.res.txt | 29 +++++++++++++++++++ .../printer/comments/operandBeforeBlock.res | 16 ++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) diff --git a/compiler/syntax/src/res_parsetree_viewer.ml b/compiler/syntax/src/res_parsetree_viewer.ml index 9b772741bc1..79bf6b979d9 100644 --- a/compiler/syntax/src/res_parsetree_viewer.ml +++ b/compiler/syntax/src/res_parsetree_viewer.ml @@ -305,6 +305,29 @@ let is_binary_expression expr = true | _ -> false +let has_line_break_before_operator expr = + let found = ref false in + let check_expr iterator (e : Parsetree.expression) = + match e.pexp_desc with + | Pexp_apply + { + funct = + { + pexp_desc = + Pexp_ident {txt = Longident.Lident operator; loc = operator_loc}; + }; + args = [(Nolabel, left); (Nolabel, _)]; + } + when not_ghost_operator operator operator_loc + && left.pexp_loc.loc_end.pos_lnum < operator_loc.loc_start.pos_lnum + -> + found := true + | _ -> if not !found then Ast_iterator.default_iterator.expr iterator e + in + let iterator = {Ast_iterator.default_iterator with expr = check_expr} in + iterator.expr iterator expr; + !found + let is_equality_operator operator = match operator with | "==" | "===" | "!=" | "!==" -> true diff --git a/compiler/syntax/src/res_parsetree_viewer.mli b/compiler/syntax/src/res_parsetree_viewer.mli index 8b77a0ae2c4..1935ecc1893 100644 --- a/compiler/syntax/src/res_parsetree_viewer.mli +++ b/compiler/syntax/src/res_parsetree_viewer.mli @@ -62,6 +62,12 @@ val operator_precedence : string -> int val not_ghost_operator : string -> Location.t -> bool val is_unary_expression : Parsetree.expression -> bool val is_binary_expression : Parsetree.expression -> bool + +val has_line_break_before_operator : Parsetree.expression -> bool +(** [has_line_break_before_operator expr] checks whether [expr] contains a + binary expression, a pipe included, whose operator starts a line, after its + left operand, where a line comment can be written. *) + val is_rhs_binary_operator : string -> bool val is_equality_operator : string -> bool diff --git a/compiler/syntax/src/res_printer.ml b/compiler/syntax/src/res_printer.ml index 9b0eab7b271..e95cef7dc46 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3172,10 +3172,14 @@ and print_expression_args ~state (args : Parsetree.expression list) cmt_tbl = parentheses with the comment before the [)]. A binary expression is left flat: breaking it would print the comment after the operator, where the next format attaches it to the right operand, so its comment goes into the - parentheses instead. *) + parentheses instead. So is an operand with a binary expression inside whose + operator starts a line, where a comment after the left operand may sit. *) and print_operand_before_brace ~expr doc = let doc = - if Parsetree_viewer.is_binary_expression expr then doc + if + Parsetree_viewer.is_binary_expression expr + || Parsetree_viewer.has_line_break_before_operator expr + then doc else Doc.break_before_line_suffixes doc in if Doc.has_pending_line_suffix doc then add_operand_parens doc else doc diff --git a/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt b/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt index cb72a59e795..67e50bcb411 100644 --- a/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt +++ b/tests/syntax_tests/data/printer/comments/expected/operandBeforeBlock.res.txt @@ -110,3 +110,32 @@ let x = while ( ) { () } + +// A binary expression inside the operand, with the comment before its operator +let x = switch ( + [a + b, d] // c +) { +| _ => 1 +} + +let x = if ( + f(a && b) // c +) { + 1 +} else { + 2 +} + +let x = for y of ( + [a->f(x)] // c +) { + () +} + +// A binary expression inside the operand, with the comment after an item +let x = switch [ + a + b, // c + d, +] { +| _ => 1 +} diff --git a/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res b/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res index 6f4d77e1827..3f3c391a5c6 100644 --- a/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res +++ b/tests/syntax_tests/data/printer/comments/operandBeforeBlock.res @@ -61,3 +61,19 @@ let x = while f( a, // c b, ) + 1 > 0 { () } + +// A binary expression inside the operand, with the comment before its operator +let x = switch [a // c ++ b, d] { | _ => 1 } + +let x = if f(a // c +&& b) { 1 } else { 2 } + +let x = for y of [a // c +->f(x)] { () } + +// A binary expression inside the operand, with the comment after an item +let x = switch [ + a + b, // c + d, +] { | _ => 1 }