diff --git a/.claude/skills/records/SKILL.md b/.claude/skills/records/SKILL.md index 699294964e4..931ca0e5c3e 100644 --- a/.claude/skills/records/SKILL.md +++ b/.claude/skills/records/SKILL.md @@ -175,14 +175,16 @@ node scripts/commitRecords.js --out page.html [--pr ] [--title ] \ - `--pr` and `--title` head the page with the pull request. `.github/workflows/semantic_diff.yml` renders each pull request's range - and comments the page's link. + and comments the records as Markdown (`--markdown --url `, + scripts/recordMarkdown.js) with the page's link: after a push that keeps + the previous head, only the commits it added (`--since `). - In prose, `#1234` and full pull request and issue URLs link as `#1234`, and the 9-character short sha of a commit on the page links to its card. ## What the page shows The page is the heading, then a card per commit. A card shows the commit's -short sha, author and date, the pull request a `(#1234)` at the end of the -subject names, and the message drawn: the subject, the prose, each object -block with its example and forms, the `Breaking:` sections as banners, the -`Review:` sections as a list, and the co-authors. +short sha, author and co-authors, date, the pull request a `(#1234)` at +the end of the subject names, and the message drawn: the subject, the +prose, each object block with its example and forms, the `Breaking:` +sections as banners, and the `Review:` sections as a list. diff --git a/.github/workflows/semantic_diff.yml b/.github/workflows/semantic_diff.yml index 8bb1c65a481..152f2419052 100644 --- a/.github/workflows/semantic_diff.yml +++ b/.github/workflows/semantic_diff.yml @@ -1,7 +1,10 @@ name: Semantic Diff # Renders the commit records of each pull request (scripts/commitRecords.js) -# as a review page on the CDN and links it from the pull request. +# as a review page on the CDN, and comments them on the pull request as +# Markdown (scripts/recordMarkdown.js) with the page's link. The page holds +# every commit; the comment holds the commits a push added, or every commit +# when the pull request is opened or reopened or its branch is force-pushed. # # A pull request from a branch of this repository runs on pull_request, with # its own workflow and renderer. A pull request from a fork runs on @@ -34,6 +37,9 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} PR_TITLE: ${{ github.event.pull_request.title }} BASE_SHA: ${{ github.event.pull_request.base.sha }} + # The head before a push; empty when the pull request is opened or + # reopened. + BEFORE_SHA: ${{ github.event.action == 'synchronize' && github.event.before || '' }} steps: - name: Checkout renderer uses: actions/checkout@v7 @@ -48,9 +54,21 @@ jobs: run: | git fetch --no-tags --filter=blob:none origin "refs/pull/${PR_NUMBER}/head:refs/heads/pr-${PR_NUMBER}" base=$(git merge-base "$BASE_SHA" "pr-${PR_NUMBER}") + head=$(git rev-parse --short=9 "pr-${PR_NUMBER}") + # The query string names the rendered commit, so each push gets a + # URL that no cache has seen. + url="https://cdn.rescript-lang.org/semantic-diff/pr-${PR_NUMBER}/index.html?v=${head}" + # A push that keeps the previous head adds commits on top of it: + # the comment holds those. After a force push the previous head is + # no ancestor (nor, from a fork, fetched), and the comment holds + # every commit. + since=() + if [ -n "$BEFORE_SHA" ] && git merge-base --is-ancestor "$BEFORE_SHA" "pr-${PR_NUMBER}" 2>/dev/null; then + since=(--since "$BEFORE_SHA") + fi mkdir -p semantic-diff - node scripts/commitRecords.js --out semantic-diff/index.html --pr "$PR_NUMBER" --title "$PR_TITLE" "${base}..pr-${PR_NUMBER}" - echo "head=$(git rev-parse --short=9 "pr-${PR_NUMBER}")" >> "$GITHUB_OUTPUT" + node scripts/commitRecords.js --out semantic-diff/index.html --markdown semantic-diff/comment.md "${since[@]}" --url "$url" --pr "$PR_NUMBER" --title "$PR_TITLE" "${base}..pr-${PR_NUMBER}" + echo "head=${head}" >> "$GITHUB_OUTPUT" - name: Setup Rclone uses: cometkim/rclone-actions/setup-rclone@8d9c6b95341f7698609deee65c3d62af42893288 # main, 2025-06-18 @@ -68,18 +86,17 @@ jobs: - name: Upload semantic diff run: rclone copyto --s3-no-check-bucket semantic-diff/index.html "rescript:cdn-assets/semantic-diff/pr-${PR_NUMBER}/index.html" - # The query string names the rendered commit, so each push gets a URL - # that no cache has seen. - - name: Comment semantic diff URL + # A push that adds only merge commits writes no comment. + - name: Comment semantic diff + if: hashFiles('semantic-diff/comment.md') != '' uses: thollander/actions-comment-pull-request@v3 with: pr-number: ${{ github.event.pull_request.number }} - comment-tag: semantic-diff - # Each push replaces the comment, so the newest link is the last - # comment on the pull request. - mode: recreate - message: | - Semantic diff: https://cdn.rescript-lang.org/semantic-diff/pr-${{ github.event.pull_request.number }}/index.html?v=${{ steps.render.outputs.head }} + # Each push gets its own comment, so the comments of earlier pushes + # stay; a re-run of the same head updates its comment. + comment-tag: semantic-diff-${{ steps.render.outputs.head }} + mode: upsert + file-path: semantic-diff/comment.md cleanup: if: >- diff --git a/analysis/reanalyze/src/arnold.ml b/analysis/reanalyze/src/arnold.ml index 6b078c99c0c..7869ee15af4 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) -> let c_e = e |> expression ~ctx in diff --git a/analysis/reanalyze/src/side_effects.ml b/analysis/reanalyze/src/side_effects.ml index 45b38e3f1ac..15473ad3f7a 100644 --- a/analysis/reanalyze/src/side_effects.ml +++ b/analysis/reanalyze/src/side_effects.ml @@ -62,6 +62,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 6318d031ef8..c840ea449a5 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 272b62d60e8..02f1e6ddfa8 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 e0b081d3d66..638d6af84c9 100644 --- a/compiler/core/lam_compile_primitive.ml +++ b/compiler/core/lam_compile_primitive.ml @@ -580,6 +580,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/ext/config.ml b/compiler/ext/config.ml index 1f2c26527d0..462f0a3912a 100644 --- a/compiler/ext/config.ml +++ b/compiler/ext/config.ml @@ -2,9 +2,9 @@ let cmi_magic_number = "Caml1999I040" (* Magic numbers for marshaled values of the *current* parsetree, whose layout changes across compiler versions. *) -and ast_impl_magic_number = "ResImpl01317" +and ast_impl_magic_number = "ResImpl01318" -and ast_intf_magic_number = "ResIntf01317" +and ast_intf_magic_number = "ResIntf01318" (* Magic numbers of the frozen Parsetree0 (OCaml 4.06) layout used on the external-PPX wire. They must never be written in front of a @@ -13,7 +13,7 @@ and ast0_impl_magic_number = "Caml1999M022" and ast0_intf_magic_number = "Caml1999N022" -and cmt_magic_number = "Caml1999T047" +and cmt_magic_number = "Caml1999T048" let get_load_path () = (Compiler_request_state.current ()).load_path let set_load_path path = (Compiler_request_state.current ()).load_path <- path diff --git a/compiler/ml/ast_helper.ml b/compiler/ml/ast_helper.ml index cf0bf224c1a..ca4124339f3 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 05908095de9..df3f1859e78 100644 --- a/compiler/ml/ast_helper.mli +++ b/compiler/ml/ast_helper.mli @@ -167,6 +167,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 1f85c513c0c..84b604a32a7 100644 --- a/compiler/ml/ast_iterator.ml +++ b/compiler/ml/ast_iterator.ml @@ -363,6 +363,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 619047539f5..8722351856a 100644 --- a/compiler/ml/ast_mapper.ml +++ b/compiler/ml/ast_mapper.ml @@ -375,6 +375,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 ca5d1021fc4..af6751100c5 100644 --- a/compiler/ml/ast_mapper_from0.ml +++ b/compiler/ml/ast_mapper_from0.ml @@ -677,6 +677,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 @@ -916,6 +957,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 8901c201e0b..c1840c0c39e 100644 --- a/compiler/ml/ast_mapper_to0.ml +++ b/compiler/ml/ast_mapper_to0.ml @@ -693,6 +693,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 ad1ec95cfe2..48ee6186e5f 100644 --- a/compiler/ml/depend.ml +++ b/compiler/ml/depend.ml @@ -273,6 +273,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 275103d415f..e35c7d946d7 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 @@ -485,6 +489,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 0b7a0e97eb9..dea17d92e7c 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 b38d060e25a..909c0aab81f 100644 --- a/compiler/ml/parsetree.ml +++ b/compiler/ml/parsetree.ml @@ -359,6 +359,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 @@ -436,6 +441,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 5fb63f4a96e..3f0cccf6aca 100644 --- a/compiler/ml/pprintast.ml +++ b/compiler/ml/pprintast.ml @@ -890,6 +890,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 94763a1d864..4126d45a22c 100644 --- a/compiler/ml/printast.ml +++ b/compiler/ml/printast.ml @@ -351,6 +351,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; @@ -482,6 +488,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 90029de4e3a..d87c60c91c8 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 7e59096a454..9413352146c 100644 --- a/compiler/ml/printtyped.ml +++ b/compiler/ml/printtyped.ml @@ -336,6 +336,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 421ca6195d4..08f1a96f5cd 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 80caa9b8520..189485efc17 100644 --- a/compiler/ml/tast_iterator.ml +++ b/compiler/ml/tast_iterator.ml @@ -184,6 +184,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 a6606fe8f17..15c6a69abd3 100644 --- a/compiler/ml/tast_mapper.ml +++ b/compiler/ml/tast_mapper.ml @@ -250,6 +250,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 cfcd1eec5d5..3895cebed32 100644 --- a/compiler/ml/translcore.ml +++ b/compiler/ml/translcore.ml @@ -1424,6 +1424,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 02399c5683f..84b9438ba92 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -265,6 +265,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)) @@ -3199,6 +3204,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 688dc72836b..17e22a9e83b 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 98b5295ebea..1d2ff8efe2b 100644 --- a/compiler/ml/typedtree.mli +++ b/compiler/ml/typedtree.mli @@ -239,6 +239,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 a748bff361d..7ac08cab747 100644 --- a/compiler/ml/typedtree_iter.ml +++ b/compiler/ml/typedtree_iter.ml @@ -269,6 +269,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 f69fcf96894..781d55236b6 100644 --- a/compiler/syntax/src/res_comments_table.ml +++ b/compiler/syntax/src/res_comments_table.ml @@ -1461,6 +1461,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 = @@ -1538,30 +1545,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 7bf65cfc348..35b4427d6d2 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 @@ -4163,62 +4161,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 = @@ -4232,56 +4200,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 8c0424b1ba3..94acc89c99d 100644 --- a/compiler/syntax/src/res_printer.ml +++ b/compiler/syntax/src/res_printer.ml @@ -3560,14 +3560,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 @@ -4321,130 +4315,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/package.json b/package.json index f42dfdf0abf..09612a20c37 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "changelog:release": "node scripts/changelog.js release", "changelog:open": "node scripts/changelog.js open", "test-changelog": "node --test scripts/changelog.test.js", - "test-commit-records": "node --test scripts/commitRecords.test.js" + "test-commit-records": "node --test scripts/commitRecords.test.js scripts/recordMarkdown.test.js" }, "files": [ "CHANGELOG.md", diff --git a/scripts/commitRecords.js b/scripts/commitRecords.js index d3abaf98228..091fe7fd1b2 100644 --- a/scripts/commitRecords.js +++ b/scripts/commitRecords.js @@ -10,10 +10,15 @@ * node scripts/commitRecords.js --out review.html ... * node scripts/commitRecords.js --out review.html ..... * --pr and --title head the page with the pull request. + * --markdown also draws the records as the pull request's comment + * (scripts/recordMarkdown.js), linking the page at --url . With + * --since , the comment draws only the commits lacks, and is + * not written when there are none. */ import { execFileSync } from "node:child_process"; import { writeFileSync } from "node:fs"; +import { commentMarkdown } from "./recordMarkdown.js"; import { CSS, esc, pageBody, pageTitle, parseRecord } from "./recordRender.js"; /** @import { CommitRecord, Group } from "./recordRender.js" */ @@ -61,14 +66,19 @@ const options = {}; /** @type {string[]} */ const targets = []; for (let i = 0; i < args.length; i++) { - if (["--out", "--pr", "--title"].includes(args[i]) && i + 1 < args.length) + if ( + ["--out", "--pr", "--title", "--markdown", "--url", "--since"].includes( + args[i], + ) && + i + 1 < args.length + ) options[args[i].slice(2)] = args[++i]; else targets.push(args[i]); } const outFile = options.out; if (!outFile) { console.error( - "usage: commitRecords.js --out [--pr ] [--title ] ...", + "usage: commitRecords.js --out [--pr ] [--title ] [--markdown [--url ] [--since ]] ...", ); process.exit(1); } @@ -76,7 +86,34 @@ const heading = { pr: (options.pr ?? "").replace(/\D/g, ""), title: options.title ?? "", }; -const body = pageBody(targets.map(group), heading); +const groups = targets.map(group); +const body = pageBody(groups, heading); +if (options.markdown) { + const since = options.since; + // The commits of the targets that `since` does not reach. + const fresh = new Set( + since + ? targets.flatMap(t => + git(["rev-list", "--abbrev-commit", "--abbrev=9", t, `^${since}`]) + .split("\n") + .filter(Boolean), + ) + : [], + ); + const shown = since + ? groups.map(g => ({ + ...g, + records: g.records.filter(r => fresh.has(r.sha)), + })) + : groups; + if (shown.some(g => g.records.length)) { + writeFileSync( + options.markdown, + commentMarkdown(shown, { url: options.url, fresh: !!since }), + ); + console.log(options.markdown); + } +} const head = `${esc(pageTitle(heading))} `; diff --git a/scripts/commitRecords.test.js b/scripts/commitRecords.test.js index 0d8e57075f0..47462eea124 100644 --- a/scripts/commitRecords.test.js +++ b/scripts/commitRecords.test.js @@ -132,9 +132,8 @@ test("a minimal record draws its identity and subject", async () => { has(html, "

Fix a typo

"); assert.match( html, - /
[0-9a-f]{9}<\/code> · A Person · [A-Z][a-z]{2} \d{2}, \d{4} · #7<\/a><\/div>/, + /
[0-9a-f]{9}<\/code> · A Person with B Person · [A-Z][a-z]{2} \d{2}, \d{4} · #7<\/a><\/div>/, ); - has(html, '
with B Person
'); lacks(html, '
'); lacks(html, "Signed-off-by"); lacks(html, "Co-Authored-By"); @@ -446,7 +445,7 @@ test("Breaking and Review sections right above the sign-off are drawn", async () ); has(html, '
  • Is -bs-x still read? No.
    • '); - has(html, '
    with B Person
    '); + has(html, "     · A Person with B Person · "); lacks(html, "Signed-off-by"); }); @@ -493,7 +492,7 @@ test("trailers of any key end the message and are not drawn", async () => { const html = await renderOne( `Fix a thing\n\nCo-Authored-By: Claude Opus 5.5 \nClaude-Session: https://claude.ai/code/session_x\n${SIGNED}\n`, ); - has(html, '
    with Claude Opus 5.5
    '); + has(html, " · A Person with Claude Opus 5.5 · "); lacks(html, "Claude-Session"); lacks(html, "session_x"); lacks(html, '
    … '); has(html, ''); }); + +test("--since leaves the comment the commits a push added, the page every commit", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "commit-records-")); + const git = (/** @type {string[]} */ ...a) => + execFileSync("git", a, { cwd: dir, encoding: "utf8" }).trim(); + git("init", "-q", "-b", "main"); + git("config", "user.name", "A Person"); + git("config", "user.email", "a@example.com"); + await commit(git, dir, { "a.txt": "a\n" }, "base"); + await commit(git, dir, {}, `First change\n\n${SIGNED}`); + const before = git("rev-parse", "HEAD"); + await commit(git, dir, {}, `Second change\n\n${SIGNED}`); + const out = path.join(dir, "page.html"); + const md = path.join(dir, "comment.md"); + const run = (/** @type {string} */ since) => { + const r = spawnSync( + "node", + [ + script, + "--out", + out, + "--markdown", + md, + "--since", + since, + "HEAD~2..HEAD", + ], + { cwd: dir, encoding: "utf8" }, + ); + assert.equal(r.status, 0, r.stderr); + }; + run(before); + const comment = await fs.readFile(md, "utf8"); + has(comment, "1 new commit"); + has(comment, "Second change"); + lacks(comment, "First change"); + const html = await fs.readFile(out, "utf8"); + has(html, "First change"); + has(html, "Second change"); + await fs.rm(md); + run("HEAD"); + await assert.rejects(fs.access(md)); +}); diff --git a/scripts/recordMarkdown.js b/scripts/recordMarkdown.js new file mode 100644 index 00000000000..d8908b39649 --- /dev/null +++ b/scripts/recordMarkdown.js @@ -0,0 +1,453 @@ +// @ts-check + +/* Commit records drawn as GitHub Markdown, for the Semantic Diff comment on + * a pull request. GitHub strips styles from a comment, so each form is drawn + * in what its Markdown renders natively: lines of code, trees, text, + * matrices, mappings and reanalyze reports as a unified diff, typing rules + * as mathematics. A comment holds at most 65536 characters; past a budget + * below that, the largest commits are drawn without their forms, and the + * page has them all. recordRender.js parses the records and draws the page. + */ + +import { + alignLines, + atomDiff, + BELOW, + BLOCK, + cellsOf, + columnWidths, + dedent, + esc, + FORMS, + formEntries, + mappingWidth, + mapsTo, + matchLines, + mathAtoms, + mathGroups, + mathTokens, + NOTE, + proseChunks, + reportEntries, + sectionGroups, + treeLine, +} from "./recordRender.js"; + +/** @import { CommitRecord, Group, Section, FormEntry } from "./recordRender.js" */ + +/** The characters a comment holds at most, with room for the action's tag. */ +export const BUDGET = 60000; + +// --------------------------------------------------------------------------- +// Inline text + +/** + * Prose split into its code spans and attributes (odd indices) and the + * text between (even ones). An attribute such as `@react.component` written + * outside backticks is drawn as code, so that GitHub does not take it for a + * mention of a user; so is any other `@name`. + * @param {string} s + */ +const spans = s => + s.split(/(`[^`]+`|(? + spans(s) + .map((part, i) => + i % 2 + ? part.startsWith("`") + ? part + : `\`${part}\`` + : part.replace(/&/g, "&").replace(/[\\`*_[\]<>|~$]/g, "\\$&"), + ) + .join("") + .replace(/^[#+-]/, "\\$&") + .replace(/^(\d+)([.)])/, "$1\\$2"); + +/** + * Prose as HTML, for a ``, where GitHub reads no Markdown: code + * spans and attributes as ``. + * @param {string} s + */ +const inlineHtml = s => + spans(s) + .map((part, i) => + i % 2 ? `${esc(part.replace(/^`|`$/g, ""))}` : esc(part), + ) + .join(""); + +/** + * A fenced code block: the fence longer than any run of backticks in the + * lines, so that no line closes it. + * @param {string} lang + * @param {string[]} lines + */ +export function fence(lang, lines) { + const longest = Math.max( + 0, + ...lines.flatMap(l => (l.match(/`+/g) ?? []).map(r => r.length)), + ); + const f = "`".repeat(Math.max(3, longest + 1)); + return `${f}${lang}\n${lines.join("\n")}\n${f}`; +} + +/** + * Prose: paragraphs, and ReScript code blocks (`proseChunks`). + * @param {string[]} lines + */ +const prose = lines => + proseChunks(lines).map(c => + c.code ? fence("rescript", c.lines) : inline(c.lines[0]), + ); + +// --------------------------------------------------------------------------- +// Mathematics + +/** Characters TeX reads as markup, escaped. @param {string} s */ +const tex = s => + s.replace(/[#$%&_{}\\^~]/g, c => + c === "\\" + ? "\\backslash " + : c === "^" + ? "\\hat{}" + : c === "~" + ? "\\sim " + : `\\${c}`, + ); + +/** + * A formula in TeX, as the page sets it: one-letter names, with their + * primes, italic variables, longer names upright; `_x`, `_{…}` and `^{…}` + * sub- and superscripts; `code` in backticks as typewriter text; primes as + * `'`. A space between two words (a name, code or a subscripted name) is + * kept. + * @param {string} text + * @returns {string} + */ +export function texMath(text) { + const toks = [...mathTokens(text)]; + const word = (/** @type {RegExpMatchArray | undefined} */ m) => + !!(m?.groups?.name || m?.groups?.code || m?.groups?.script); + return toks + .map((m, k) => { + const g = m.groups ?? {}; + if (g.code) return `\\texttt{${tex(g.code.slice(1, -1))}}`; + if (g.script) + return `${g.script[0]}{${texMath(g.script.replace(/^[_^]\{?|\}$/g, ""))}}`; + if (g.name) + return /^.[0-9'′]*$/.test(g.name) + ? g.name.replace(/′/g, "'") + : `\\mathrm{${g.name.replace(/′/g, "'")}}`; + if (/\s/.test(m[0])) + return word(toks[k - 1]) && word(toks[k + 1]) ? "\\ " : ""; + return m[0] === "′" ? "'" : tex(m[0]); + }) + .join(""); +} + +/** + * Mathematics in TeX (`mathGroups`): inference rules as fractions, their + * names after them, and rows of formulas; the marked parts (numbered as + * `mathAtoms` lists them) coloured. + * @param {string[]} block + * @param {{ marked?: Set, color?: string }} [marks] + */ +export function texRules(block, marks = {}) { + let n = 0; + const part = (/** @type {string} */ t) => + marks.marked?.has(n++) ? `\\color{${marks.color}}{${t}}` : t; + const out = mathGroups(block).map(g => { + if ("rows" in g) + return `\\begin{array}{l}${g.rows + .map(r => r.map(p => part(texMath(p))).join(" \\qquad ")) + .join(" \\\\ ")}\\end{array}`; + const premises = g.premises.map(p => part(texMath(p))).join(" \\qquad "); + const conclusion = g.conclusion.map(l => part(texMath(l))).join(" \\\\ "); + const name = g.name ? ` \\quad ${part(`(\\textsf{${tex(g.name)}})`)}` : ""; + return `\\dfrac{${premises}}{\\begin{array}{c}${conclusion}\\end{array}}${name}`; + }); + return out.length > 1 + ? `\\begin{array}{l}${out.join(" \\\\[1em] ")}\\end{array}` + : (out[0] ?? ""); +} + +// --------------------------------------------------------------------------- +// Forms + +/** + * Each drawing's fenced block language for one version: GitHub highlights + * ReScript and JavaScript; trees, Lambda, text and reports are plain. + * @type {Record} + */ +const LANG = { rescript: "rescript", js: "js", math: "math" }; + +/** + * The lines of one version of a form as drawn in a fenced block: a tree's + * list brackets dropped, a matrix's cells and a mapping's arrows aligned + * across both versions (`both`). + * @param {string} drawing + * @param {string[]} ls + * @param {string[]} both + */ +function formLines(drawing, ls, both) { + if (drawing === "tree") return ls.filter(treeLine); + if (drawing === "matrix") { + const widths = columnWidths(both); + return ls.map(l => + NOTE.test(l) + ? l.trim() + : cellsOf(l) + .map((p, c) => p.padEnd(widths[c] ?? 0)) + .join(" ") + .trimEnd(), + ); + } + if (drawing === "mapping") { + const width = mappingWidth(both); + return ls.map(l => { + const m = mapsTo(l); + return m ? `${m[0].padEnd(width)} ↦ ${m[1]}` : l.trim(); + }); + } + return ls; +} + +/** + * Two versions of a form's lines as a unified diff: a line common to both + * is context; between two common lines, the lines only Before has, then the + * lines only After has. + * @param {string[]} b + * @param {string[]} a + */ +function diffLines(b, a) { + /** @type {string[]} */ + const out = []; + /** @type {string[]} */ + let adds = []; + for (const row of alignLines(b, a)) { + if (row.same) { + out.push(...adds); + adds = []; + out.push(` ${b[row.a ?? 0]}`); + continue; + } + if (row.a !== null) out.push(`-${b[row.a]}`); + if (row.b !== null) adds.push(`+${a[row.b]}`); + } + return [...out, ...adds]; +} + +/** + * Two reanalyze reports as a unified diff of their entries, aligned by + * identity: a finding one report lacks is removed or added, a run of + * findings in both is one context line counting them, and a note in both, + * such as the closing count, is context. + * @param {string[]} b + * @param {string[]} a + */ +function reportDiff(b, a) { + const before = reportEntries(b); + const after = reportEntries(a); + const pairs = matchLines( + before.map(e => e.key), + after.map(e => e.key), + k => k, + ); + /** @type {string[][]} */ + const out = []; + let same = 0; + const collapse = () => { + if (same) + out.push([` … ${same} unchanged finding${same === 1 ? "" : "s"}`]); + same = 0; + }; + const lines = (/** @type {string} */ key, /** @type {string} */ sign) => + key.split("\n").map(l => sign + l); + let i = 0; + let j = 0; + for (const [x, y] of [...pairs, [before.length, after.length]]) { + const dels = before.slice(i, x); + const adds = after.slice(j, y); + if (dels.length || adds.length) collapse(); + for (const e of dels) out.push(lines(e.key, e.elision ? " " : "-")); + for (const e of adds) out.push(lines(e.key, e.elision ? " " : "+")); + if (x < before.length) { + if (before[x].analysis) same++; + else { + collapse(); + out.push(lines(before[x].key, " ")); + } + } + i = x + 1; + j = y + 1; + } + collapse(); + return out.flatMap((e, k) => (k ? [" ", ...e] : e)); +} + +/** + * One form of an object block: its name and captions, then the versions + * drawn. Two versions are a diff, or, for typing rules, two figures side by + * side with the changed parts coloured; equal versions and a lone version + * are drawn once. + * @param {Extract} e + */ +function form(e) { + const drawing = FORMS[e.form] ?? "text"; + const { before, after } = e; + const hasBlock = (/** @type {Section} */ s) => s.block.some(l => l.trim()); + const caption = (/** @type {Section | undefined} */ s) => + s && hasBlock(s) && s.text.length ? inline(s.text.join(" ")) : ""; + const lines = (/** @type {Section} */ s) => + hasBlock(s) ? dedent(s.block) : s.text; + const label = (/** @type {string[]} */ notes) => + [`\`${e.form}\``, ...notes.filter(Boolean)].join(" · "); + const one = (/** @type {string[]} */ ls) => + drawing === "math" + ? fence("math", [texRules(ls)]) + : fence(LANG[drawing] ?? "", formLines(drawing, ls, ls)); + if (before && after) { + const [b, a] = [lines(before), lines(after)]; + const [cb, ca] = [caption(before), caption(after)]; + if (b.length && b.join("\n") === a.join("\n")) + return `${label(["Before and After are equal", cb, ca === cb ? "" : ca])}\n\n${one(a)}`; + const head = label( + cb === ca ? [cb] : [cb && `Before: ${cb}`, ca && `After: ${ca}`], + ); + if (drawing === "math") { + const [del, add] = atomDiff(mathAtoms(b), mathAtoms(a)); + const cell = (/** @type {string} */ t) => `\n\n${t}\n\n`; + return `${head}\n\n${cell( + fence("math", [texRules(b, { marked: del, color: "red" })]), + )}${cell(fence("math", [texRules(a, { marked: add, color: "green" })]))}
    BeforeAfter
    `; + } + const diff = + drawing === "report" + ? reportDiff(b, a) + : diffLines( + formLines(drawing, b, [...b, ...a]), + formLines(drawing, a, [...b, ...a]), + ); + return `${head}\n\n${fence("diff", diff)}`; + } + const only = /** @type {Section} */ (before ?? after); + return `${label([before ? "Before" : "After", caption(only)])}\n\n${one(lines(only))}`; +} + +// --------------------------------------------------------------------------- +// Commits + +/** + * One section with the example and forms it holds; without `full`, the + * forms are left to the page. + * @param {Section} s + * @param {Section[]} parts + * @param {boolean} full + */ +function section(s, parts, full) { + /** @type {string[]} */ + const out = []; + const block = BLOCK.exec(s.name); + const text = s.text.length ? [inline(s.text.join(" "))] : []; + const lines = dedent(s.block); + const body = !lines.length + ? [] + : s.name + ? prose(lines) + : [fence("rescript", lines)]; + if (s.name === "Why") + out.push( + [`**Why**`, ...text, ...body] + .join("\n\n") + .split("\n") + .map(l => (l ? `> ${l}` : ">")) + .join("\n"), + ); + else { + if (s.name) + out.push( + block + ? `**${block[1]} ${block[2]}**${block[3] ? ` (${inline(block[3])})` : ""}` + : `**${inline(s.name)}**`, + ); + out.push(...text, ...body); + } + const entries = formEntries(parts); + if (full) + for (const e of entries) + out.push("example" in e ? fence("rescript", e.example) : form(e)); + else if (entries.length) out.push("Forms on the page."); + return out.join("\n\n"); +} + +/** + * A commit as a collapsible block: its subject, identity and message drawn; + * without `full`, its forms are left to the page. + * @param {CommitRecord} r + * @param {boolean} full + */ +export function commitMarkdown(r, full) { + const pr = /\(#(\d+)\)\s*$/.exec(r.subject)?.[1] ?? ""; + const subject = r.subject.replace(/\s*\(#\d+\)\s*$/, ""); + const joined = (/** @type {Section} */ s) => + [...s.text, ...s.block.map(l => l.trim())].join(" ").trim(); + const out = [ + `
    ${inlineHtml(subject)}`, + `${esc(r.sha)} · ${inline(r.author)}${r.coauthors.length ? ` with ${inline(r.coauthors.join(", "))}` : ""}${r.date ? ` · ${esc(r.date)}` : ""}${pr ? ` · #${pr}` : ""}`, + ...sectionGroups(r.sections.filter(s => !BELOW.has(s.name))).map(g => + section(g.section, g.parts, full), + ), + ...r.sections + .filter(s => s.name === "Breaking") + .map(s => `> [!CAUTION]\n> **Breaking:** ${inline(joined(s))}`), + ]; + const reviews = r.sections.filter(s => s.name === "Review"); + if (reviews.length) + out.push( + `**Review**\n\n${reviews.map(s => `- ${inline(joined(s))}`).join("\n")}`, + ); + out.push("
    "); + return out.filter(Boolean).join("\n\n"); +} + +/** + * The comment: a heading linking the page, then each commit. Past the + * budget, the commits with the most characters are drawn without their + * forms, and if that is not enough, the last commits are left to the page. + * `fresh` counts the commits as new ones, those a push added. + * @param {Group[]} groups + * @param {{ url?: string, budget?: number, fresh?: boolean }} [options] + */ +export function commentMarkdown( + groups, + { url = "", budget = BUDGET, fresh = false } = {}, +) { + const records = groups.flatMap(g => g.records); + const n = records.length; + const head = `### Semantic Diff\n\n${n}${fresh ? " new" : ""} commit${n === 1 ? "" : "s"}${url ? ` · [Full page](${url})` : ""}`; + const drawn = records.map(r => commitMarkdown(r, true)); + const size = () => drawn.reduce((t, d) => t + d.length + 2, head.length); + const order = drawn + .map((d, k) => ({ k, length: d.length })) + .sort((x, y) => y.length - x.length); + for (const { k } of order) { + if (size() <= budget) break; + drawn[k] = commitMarkdown(records[k], false); + } + let left = 0; + while (drawn.length && size() > budget) { + drawn.pop(); + left++; + } + if (left) + drawn.push( + `${left} more commit${left === 1 ? "" : "s"} on the page.`, + ); + return `${[head, ...drawn].join("\n\n")}\n`; +} diff --git a/scripts/recordMarkdown.test.js b/scripts/recordMarkdown.test.js new file mode 100644 index 00000000000..5dda5a1a6b1 --- /dev/null +++ b/scripts/recordMarkdown.test.js @@ -0,0 +1,232 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { commentMarkdown, fence, inline, texMath } from "./recordMarkdown.js"; +import { parseRecord } from "./recordRender.js"; + +const SIGNED = "Signed-off-by: A Person "; + +/** + * The comment drawn from commit messages, one commit each. + * @param {string[]} messages + * @param {{ url?: string, budget?: number }} [options] + */ +const comment = (messages, options) => + commentMarkdown( + [ + { + records: messages.map((message, k) => + parseRecord({ + sha: `${k}`.padStart(9, "a"), + author: "A Person", + date: "Oct 22, 2026", + message, + }), + ), + }, + ], + options, + ); + +/** + * Asserts that the comment contains the text; a failure names the text. + * @param {string} md + * @param {string} text + */ +const has = (md, text) => + assert.ok(md.includes(text), `comment lacks ${JSON.stringify(text)}`); + +test("a commit is a block with its identity, sections and co-authors", () => { + const md = comment( + [ + `Represent raw extensions as their own node (#12) + +Why: + A raw extension carries JavaScript. + +AST modified (raw extensions): +Example: + let f = %raw(\`(a) => a\`) +Before (parse): + [ + expression + Pexp_extension "raw" + ] +After (parse): + [ + expression + Pexp_raw "raw" + ] + +Review: Does a PPX still see it? Yes. +Breaking: PPXs must match Pexp_raw. + +Co-Authored-By: B Person +${SIGNED} +`, + ], + { url: "https://cdn.example/pr-12/index.html?v=1" }, + ); + has( + md, + "### Semantic Diff\n\n1 commit · [Full page](https://cdn.example/pr-12/index.html?v=1)", + ); + has( + md, + "
    Represent raw extensions as their own node", + ); + has(md, "aaaaaaaa0 · A Person with B Person · Oct 22, 2026 · #12"); + has(md, "> **Why**\n>\n> A raw extension carries JavaScript."); + has(md, "**AST modified** (raw extensions)"); + has(md, "```rescript\nlet f = %raw(`(a) => a`)\n```"); + // the tree's list brackets are dropped and the changed line is a diff + has( + md, + '`parse`\n\n```diff\n expression\n- Pexp_extension "raw"\n+ Pexp_raw "raw"\n```', + ); + has(md, "> [!CAUTION]\n> **Breaking:** PPXs must match Pexp\\_raw."); + has(md, "**Review**\n\n- Does a PPX still see it? Yes."); + has(md, "Yes.\n\n
    "); + assert.ok(!md.includes("Signed-off-by")); +}); + +test("a typing rule is mathematics, the changed parts coloured", () => { + const md = comment([ + `Check applications + +Typing modified (application): +Before (rule): + Γ ⊢ e_1 : τ_{1} → τ_{2} Γ ⊢ e_2 : τ_{1} + ─────────────── (App) + Γ ⊢ e_1 e_2 : τ_{2} +After (rule): + Γ ⊢ e_1 : τ_{1} → τ_{2} Γ ⊢ e_2 : τ_{1}′ + ─────────────── (App) + Γ ⊢ e_1 e_2 : τ_{2} + +${SIGNED} +`, + ]); + has(md, "
    BeforeAfter
    \n\n```math\n"); + has( + md, + "\\dfrac{Γ⊢e_{1}:τ_{1}→τ_{2} \\qquad \\color{red}{Γ⊢e_{2}:τ_{1}}}{\\begin{array}{c}Γ⊢e_{1}\\ e_{2}:τ_{2}\\end{array}} \\quad (\\textsf{App})", + ); + has(md, "\\color{green}{Γ⊢e_{2}:τ_{1}'}"); + assert.equal(md.split("\\color").length - 1, 2); +}); + +test("a formula's words, code and TeX characters", () => { + assert.equal( + texMath("fresh x `f(a, ...)` & {p}"), + "\\mathrm{fresh}\\ x\\texttt{f(a, ...)}\\&\\{p\\}", + ); +}); + +test("matrices and mappings align their columns in the diff", () => { + const md = comment([ + `Unbox options + +Representation modified (option): +Before (representation): boxed + Some(x) ↦ {_0: x} + None ↦ undefined +After (representation): unboxed + Some(x) ↦ x + None ↦ undefined + +Matching modified (option split): +Before (matrix): + Some(x) e1 + None e2 +After (matrix): + Some(x) _ e1 + None e2 + order: Some first + +JS modified (f): +Before (js): same output + let f = 1; +After (js): same output + let f = 1; + +${SIGNED} +`, + ]); + has( + md, + "`representation` · Before: boxed · After: unboxed\n\n```diff\n-Some(x) ↦ {_0: x}\n+Some(x) ↦ x\n None ↦ undefined\n```", + ); + has( + md, + "```diff\n-Some(x) e1\n+Some(x) _ e1\n None e2\n+order: Some first\n```", + ); + has( + md, + "`js` · Before and After are equal · same output\n\n```js\nlet f = 1;\n```", + ); +}); + +const DIVIDE = `Exception Analysis +Division.res:1:5-10 +divide might throw Division_by_zero`; +const DIVIDE_BY = DIVIDE.replace(/divide/g, "divideBy").replace(":1:", ":2:"); + +test("a reanalyze report is a diff of findings, unchanged ones counted", () => { + const indented = (/** @type {string} */ s) => + s + .split("\n") + .map(l => (l ? ` ${l}` : "")) + .join("\n"); + const md = comment([ + `Skip literal divisors + +Reanalyze modified (division): +Before (reanalyze): +${indented(`${DIVIDE}\n\n${DIVIDE_BY}\n\n${DIVIDE_BY.replaceAll("divideBy", "divideTo")}\n\nAnalysis reported 3 issues`)} +After (reanalyze): +${indented(`${DIVIDE_BY}\n\n${DIVIDE_BY.replaceAll("divideBy", "divideTo")}\n\nAnalysis reported 2 issues`)} + +${SIGNED} +`, + ]); + has( + md, + "```diff\n-Exception Analysis\n-Division.res:1:5-10\n-divide might throw Division_by_zero\n \n … 2 unchanged findings\n \n-Analysis reported 3 issues\n \n+Analysis reported 2 issues\n```", + ); +}); + +test("prose escapes markup and draws attributes and mentions as code", () => { + assert.equal( + inline("Use @react.component and `a*b` for in $x_1 * 2, cc @someone"), + "Use `@react.component` and `a*b` for \\ in \\$x\\_1 \\* 2, cc `@someone`", + ); + assert.equal(inline("# not a heading"), "\\# not a heading"); + assert.equal(inline("1. not a list"), "1\\. not a list"); + assert.equal(inline("mail a@example.com"), "mail a@example.com"); +}); + +test("a fence is longer than any run of backticks in its lines", () => { + assert.equal(fence("js", ["let s = ```;"]), "````js\nlet s = ```;\n````"); +}); + +test("past the budget, the largest commits leave their forms to the page", () => { + const big = `Change output + +JS modified (f): +Before (js): +${Array.from({ length: 200 }, (_, k) => ` let a${k} = ${k};`).join("\n")} +After (js): +${Array.from({ length: 200 }, (_, k) => ` let b${k} = ${k};`).join("\n")} + +${SIGNED} +`; + const small = `Fix a typo\n\nWhy:\n It was wrong.\n\n${SIGNED}\n`; + const md = comment([small, big, small], { budget: 4000 }); + assert.ok(md.length <= 4000, `${md.length}`); + has(md, "**JS modified** (f)\n\nForms on the page."); + assert.equal(md.split("> It was wrong.").length - 1, 2); + const tiny = comment([small, big, small], { budget: 400 }); + assert.ok(tiny.length <= 500, `${tiny.length}`); + has(tiny, "more commit"); +}); diff --git a/scripts/recordRender.js b/scripts/recordRender.js index d298ddffa96..cb8d5ae0882 100644 --- a/scripts/recordRender.js +++ b/scripts/recordRender.js @@ -36,7 +36,7 @@ export const esc = s => * of another name is drawn as text. * @type {Record} */ -const FORMS = { +export const FORMS = { parse: "tree", expanded: "tree", typed: "tree", @@ -55,11 +55,11 @@ const FORMS = { * An object block's heading: ` ()`, * the kind one word, the object's parentheses optional. */ -const BLOCK = +export const BLOCK = /^([A-Z][A-Za-z]*) (added|modified|removed)(?: \(((?:[^()]|\([^()]*\))*)\))?$/; /** A section an object block holds: its example or one version of a form. */ -const PART = /^(?:Example|(Before|After) \(([a-z]+)\))$/; +export const PART = /^(?:Example|(Before|After) \(([a-z]+)\))$/; // --------------------------------------------------------------------------- // Records @@ -113,7 +113,7 @@ const untab = l => }); /** Whether a line is indented by 4 spaces or more. @param {string} l */ -const isIndented = l => /^ {4,}\S/.test(l); +export const isIndented = l => /^ {4,}\S/.test(l); /** * A record's subject, sections and trailers. A section starts at a line @@ -259,7 +259,7 @@ function highlight(code, lang) { * @param {(line: string) => string} key * @returns {[number, number][]} */ -function matchLines(a, b, key) { +export function matchLines(a, b, key) { const x = a.map(key); const y = b.map(key); /** @type {number[][]} */ @@ -292,7 +292,7 @@ function matchLines(a, b, key) { * @param {string[]} b * @returns {{ a: number | null, b: number | null, same: boolean }[]} */ -function alignLines(a, b) { +export function alignLines(a, b) { /** @type {{ a: number | null, b: number | null, same: boolean }[]} */ const rows = []; let i = 0; @@ -340,7 +340,7 @@ const indentOf = l => /^ */.exec(l)?.[0].length ?? 0; const codeHtml = (lines, draw) => draw(lines.join("\n")).split("\n"); /** An elision line, `…` and any text, marking lines left out. @param {string} l */ -const elided = l => l.trimStart().startsWith("…"); +export const elided = l => l.trimStart().startsWith("…"); /** * Lines of code, each drawn by `draw`, the marked ones shaded and the @@ -366,7 +366,7 @@ function codeLines(lines, draw, marks = {}) { } /** A note line of a drawing: `label: text`. */ -const NOTE = /^\s*[a-z][\w ]{0,20}:\s/; +export const NOTE = /^\s*[a-z][\w ]{0,20}:\s/; /** * The parts of a figure (a typing rule or a rewrite) that a Before/After @@ -382,7 +382,7 @@ const NOTE = /^\s*[a-z][\w ]{0,20}:\s/; * @param {Atom[]} b * @returns {[Set, Set]} */ -function atomDiff(a, b) { +export function atomDiff(a, b) { const del = new Set(); const add = new Set(); for (const slot of new Set([...a, ...b].map(x => x.slot))) { @@ -555,7 +555,7 @@ const inline = s => * `_{…}`, `^{…}`), a name, or any other character. * @param {string} text */ -const mathTokens = text => +export const mathTokens = text => text.matchAll( /(?`[^`]+`)|(?