Query based formatter with max line width support - #12768
Draft
LeonMatthes wants to merge 42 commits into
Draft
Conversation
This is inspired by the Topiary Crate which uses Tree-sitter syntax nodes that it annotates with specific formatting annotations that it then uses to format the entire document. We now use a similar approach here but based on our own grammar which is based on the Rowan Crate.
Comments in a gap now split it into sub-gaps that are resolved independently instead of the whole gap being kept verbatim. Trailing comments hang on their line, own-line comments re-indent to the current level (except at column 0, which compiler syntax tests rely on), blank lines above comments are preserved and capped at one, and re-indented multiline block comments shift their continuation lines to keep internal alignment. Rules can now re-indent a comment but never move it onto a different line. Also convert the rules.rs integration tests to multi-line string literals for readability, and reconcile API_DESIGN.md's Instruction section with the implemented slot-indexed shape.
…er, at) Positional and conditional rule logic stays in plain Rust: children() yields the significant direct children, iter() exposes them as NodeOrToken, and at() re-enters the selection API from whatever the iteration found. token_matching() covers kind-set matches (e.g. the operator of a BinaryExpression); token() now delegates to it. child_tokens() and at() also structurally exclude the Eof terminator, which would otherwise anchor atoms past the last real gap.
any_node() registers rules that run for every node at Tier::Wildcard, between Token and Node in the override order. Dispatch reuses the node's selection across both tiers: atoms record their tier at attach time, so retargeting the selection afterwards is invisible to what the wildcard rules produced. InputSoftline at a boundary with no input newline now abstains instead of pushing a tier-bearing Nothing. A Node-tier Nothing would otherwise veto a weaker-tier Space (e.g. a wildcard adjacent-node space), gluing single-line constructs together. It still counts toward gap engagement.
A rule can mark a node as a leaf (Selection::leaf()) to emit its interior verbatim: the gaps strictly inside its significant span are kept as written no matter what rules fired there, while the boundary gaps just before and after still resolve so the leaf spaces against its surroundings. Indentation still accrues across a leaf so a balanced brace pair inside it keeps the running level exact. The sink now collects markers alongside boundary atoms (Annotations); resolve normalizes the leaf ranges to outermost disjoint spans and suppresses interior gaps. make_rules leafs @rust-attr contents and string-template interpolations, whose foreign tokens the global colon/comma rules would otherwise mangle.
A `// slint-fmt:ignore` comment leafs the construct starting at the next significant token, emitting it verbatim. apply_ignore_directives runs between annotate and finish, producing the same Marker::Leaf the leaf rules do; leaf normalization dedups it against any overlapping rule-leaf. ignored_span walks the token's ancestors for the outermost node that still begins at that token, capped below the Document so a top-level directive ignores one item rather than the whole file. The match tolerates trailing whitespace (invisible in a line comment) but is otherwise exact, mirroring the reference ruleset.
Atom::Literal(String) injects fixed text at a boundary (append-literals
hug the left token, prepend-literals the right); Selection::delete()
marks a token for removal. Together they manage a list's trailing comma:
append Literal(",") to the last argument when the list breaks across
lines, delete the comma when it collapses onto one line.
A deleted token emits nothing (still passing the writer once) and its
gap collapses; the next surviving gap sources its append-side atoms from
the last emitted token, so the two gaps around a deleted token merge
without any structural change to the sub-gap machinery. A delete inside
a leaf range defers to the leaf.
Adding Literal costs Atom/AtomInstance/Instruction their Copy (Clone is
kept); new EmitLiteral/DeleteToken instructions render the two effects.
SyntaxToken::next_token relied on rowan's first_token() to descend into
the next node, but rowan only descends into the FIRST child: a node
whose first child is an empty node (e.g. an empty leading QualifiedName
from `inherits {`, or the empty leading Expression of a wildcard match
case with no prior case) reported no first token, so the walk skipped
the entire subtree — panicking the query formatter's linearization in
debug and silently dropping that code in release.
Replace the shallow first_token() with a properly recursive first-token
search that skips empty children. Regression test covers both error-tree
shapes.
EmitLiteral instructions for append/prepend literals were pushed unconditionally, before the leaf_internal check, so a Literal attached to a token strictly inside a Leaf range (including ranges leafed by // slint-fmt:ignore) leaked into supposedly-verbatim output — unlike the paired delete(), which was already suppressed. Gate both literal loops on !leaf_internal so all boundary effects are suppressed uniformly inside a leaf. Also adds the two next_token error-tree shapes to linearize_visits_every_token as engine-level regressions.
API_DESIGN.md phase-2 step 8 / constraint 5 require that no Hardline resolve strictly inside a softline measure span that itself resolved single-line — such a newline would flip the span to multiline on the next run, breaking format(format(x)) == format(x). resolve now records (debug builds only) the single-line-resolved softline spans and the positions where a Hardline actually produced a newline, then panics if any Hardline newline lands strictly inside a single-line span. All tracking is #[cfg(debug_assertions)], so release builds are unaffected. A #[should_panic] test with a deliberately violating ruleset confirms the guard fires.
The formatter is no longer a no-op except where rules fire. Every gap is now resolved: a rule's atoms decide the boundary, or a default applies — a single space between two tokens, nothing at the document edges (before the first token and before Eof). The only input-preserving paths left are the deliberate Leaf feature (@rust-attr, string templates, // slint-fmt:ignore) and the comment never-move policy. This makes the formatter diff-testable against the repository: it reformats everything, so a run shows exactly what the (still minimal) ruleset does and does not yet cover, and the diff shrinks as rules are added. resolve drops the has_spacing_atom gate; resolve_gap takes a default Strength and loses its 'keep as written' branch; Instruction::KeepSubGap is removed (KeepGap remains only for leaf interiors and a deleted token's comment-bearing gap). Tests, API_DESIGN.md and comments updated to drop the concept.
Port the Topiary-experiment's corpus-testing harness (origin/experiment-slint-fmt-topiary) to the new query-based formatter, as a `slint-lsp` example so it stays out of normal builds and installs. `diff` reports whole-file line churn against a corpus; `canonicalize` checks that indentation-perturbed multiline blocks reconverge to the same formatted output. Both call format_document_query through the existing writer API without touching the formatter itself.
Translate the highest-signal slint.scm layout rules onto the query engine: top-level items on their own lines, element/code-block bodies breaking one item per line, struct/enum/object-literal/array bodies, and comma-driven list breaks. Fast-corpus churn drops from 37477 to 22994.
Dot/member-access gluing, property angle brackets, call/index/callback parentheses, unary operators, repeated-element indices, and ternary operator spacing. Fast-corpus churn drops from 22994 to 10690.
Element and nested bodies now break across lines, so the prototype tests' expected outputs are updated to the fully-formatted results. Comment, leaf and ignore-directive behaviors are unchanged.
An empty element/struct/enum body now formats as {} inline (the dominant
corpus convention) while a body already broken across lines keeps its open
pair. Fast-corpus churn drops from 9493 to 9379.
The global comma softline measured the comma's parent, which for functions, callbacks and callback connections includes a multiline body — wrongly breaking inline parameter lists. Commas now keep a newline only where the input had one; container-wide breaks come from each body's per-item rule. Fast-corpus churn drops from 9379 to 9011.
Fast-corpus churn drops from 9011 to 8280.
The LSP formatting request and the format subcommand now both use the query-based formatter; the SLINT_FMT_QUERY opt-in gate is gone. The old imperative formatter stays temporarily as dead code until its tests are ported. Two behavior fixes surfaced by the switch: the Document rule now ends every non-empty file with exactly one newline, and the nothing-at-the-document-edge default is scoped to the edge-touching sub-gap so a hanging comment before Eof (or a leading comment before the first token) keeps its space instead of gluing to the code.
The old imperative formatter's 52 end-to-end tests move from fmt.rs to the new fmt/tests.rs, now running against format_document_query. The expected outputs are re-blessed where the two formatters deliberately disagree (input-driven layout instead of forced breaking, no line-width breaking, no trailing-comma management, tight empty braces, dropped leading blank lines, guaranteed trailing newline), so the suite records the before/after difference. The query formatter's own overlapping end-to-end tests in rules.rs are removed; only its two leaf-rule tests survive, appended to the ported suite, because no old test covers those constructs. Two rule fixes surfaced by the port: a dot no longer glues onto a bare integer literal (42.log(x) re-lexes as `42.` `log` and would change the expression), and stray error-recovery token children of a document or braced body keep their input line structure instead of being broken one token per line.
The query-based formatter is the default everywhere and the test suite is ported, so fmt/fmt.rs goes. TokenWriter loses insert_before — only the old formatter called it — and the docs drop their not-yet- implemented and kept-until-parity qualifiers.
First step of the max-line-width search (MAX_LINE_WIDTH_DESIGN.md): the PAGE_WIDTH/COMPUTATION_WIDTH constants, the group vocabulary (GroupId, Variant), and the lexicographic cost triple (overflow, deviation, height) with the splitting contract the search's pruning relies on. Not wired into the pipeline yet; covered by unit tests.
The candidate bookkeeping from MAX_LINE_WIDTH_DESIGN.md: Measure (cost + last-line width + decisions), the O(1)-append persistent DecisionList (with an iterative Drop — the chains grow one link per decided group), lazily evaluated tainted fallbacks, and the dedup/merge pair that keeps only the Pareto frontier with the deterministic left tie-break the idempotency argument relies on. Not wired into the pipeline yet; covered by unit tests.
The search core from MAX_LINE_WIDTH_DESIGN.md: the Doc tree (text runs, fixed newlines, concatenation, per-group either/or choices) in an arena, and the memoized resolver that finds the cheapest variant per group — per-prefix suffix resolution merged Set-preferring, the deviation penalty applied on branch-local copies, and lazily bundled greedy fallbacks once a layout blows past the computation width. Not wired into the pipeline yet; covered by unit tests, including mutation-verified cases for per-prefix tainting and the greedy fallback's author-layout bias.
Behavior-preserving refactor that opens the seam the width search needs. route_atom gains a SoftlineMode: measured softlines resolve either from the input layout (as today) or from a group's chosen variant; InputSoftline keeps its input meaning in both. resolve_gap now takes the indentation level by value and returns a GapResolution instead of mutating the caller's vector and counter, so the document builder can run it once per (gap, variant) and the emitter can replay it. Output is byte-for-byte unchanged (verified against the fmt-corpus).
Step 5a of the document builder: collect every distinct measured-softline span into a Group, map it to the token slots it covers, and link the groups into a containment forest (the spans nest or are disjoint). Each group records whether its input was multiline, which fixes the variant the search penalizes for deviating. Not consumed yet; the document builder that walks the forest is next.
Walk the group forest and emit a Doc per token, gap and group: each group becomes a choice between its flat and multiline bodies, or just the multiline body when a newline (a comment, say) forbids flattening. Gaps resolve through the engine's own gap resolver, so the search measures the same layout the renderer will produce -- including the column shift applied to a re-indented multi-line comment's continuation lines. The builder needs several of the engine's phase-2 helpers, so expose them pub(crate) rather than move the builder into engine.rs. Not wired into the pipeline yet.
Switch the formatter's second phase from resolving every gap against the input layout to build -> search -> emit: build a choice document of groups, let the width search pick single-line or multiline per group to keep lines within PAGE_WIDTH (100), then emit the plan replaying those decisions. A group absent from the search's decisions was either inlined into an ancestor's chosen flat body (render single-line) or never had a single-line body (render multiline); BuiltDocument now reports the latter set. The IdempotencyGuard classifies a span single-line by the search's decision rather than by input measurement. A body whose flat layout would contain a newline (a comment, a Hardline) has no single-line variant, so it is forced multiline -- which removes the old Hardline-inside-a-single-line-span idempotency hazard rather than panicking on it.
Let a rule hand the width search both layouts of a dynamic construct
instead of branching on input multilineness at annotate time. An
Atom::Literal and the delete() marker now carry an optional Condition
{ span, variant }: the literal is emitted, or the token deleted, only when
the group spanning `span` resolves to `variant`. Selection sugar
`literal_if_multiline` / `delete_if_single_line` pairs the condition with
the group's softlines so they flip together.
Builder and emitter resolve a condition through one shared `condition_active`
keyed on the atom's controlling-group variant, so the width the search
optimizes is the width emitted. The builder now collapses a deleted token's
gap the way the emitter does, and debug assertions enforce the invariants
the shared resolution relies on: a condition shares a group's span, and a
deleted token sits strictly inside its group carrying no append atoms.
The trailing-comma engine test is re-expressed with the conditional API —
same four outputs, now decided by the search. The shipped ruleset does not
use conditional atoms yet, so real output is unchanged.
One call on a Selection expands to the primitives: the first-to-last-item span becomes a group, each inter-item separator gains the group's softline, and the trailing separator follows the chosen layout — deleted when the list collapses, added after the last item when it breaks. The trailing separator sits textually after the group's measure span, so the builder widens each group's slot range into an extent covering the tokens its conditional atoms occupy. Conditional literals anchored past the extent (the separator a broken list gains) are emitted at the end of the group's region, keeping the group's width and its choice one unit. The emitter resolves each condition by the group its span identifies; debug assertions enforce the placement invariant that keeps the builder's region-mode accounting in agreement — a debug run is the ruleset validator, as the design document now spells out. A list holding a token-less error-recovery item (a doubled comma) is left as written: which separator is trailing is ambiguous there.
Migrate the shipped ruleset to the width-search list primitives: arrays, call arguments, callback-declaration and callback-connection parameters and function parameters go through break_delimited_list — the delimiters and the items form two separate groups, so a moderately wide list breaks at its delimiters alone while a too-wide one goes on to one item per line, gaining a trailing comma. The delimiter span, not the node's, forms the group: a declaration's multiline body must not force its parameter list open. The trailing separator the ruleset now conditionally deletes carries the global comma rule's InputSoftline, so the emitter no longer discards a deleted token's append atoms — it carries them to the next surviving gap. They then act in the same physical gap whether or not the token survived, which is exactly where the deletion-unaware builder already counts them; the corresponding clause of debug_assert_deletable is obsolete. Deleted tokens' prepend indent atoms remain unsound (collapsed unapplied) and are now asserted against; both design documents are updated to the carry semantics. A newline inside one list item counts as the author breaking the list, so the whole list goes one item per line — the last multiline argument is not hugged against the parens; a test pins this deliberate departure from other formatters.
This ensures that if the function arguments are split onto multiple
lines, the codeblock is as well, so we don't end up with:
function foo(
a: int,
b: int,
) { return 5; }
Which will now become:
function foo(
a: int,
b: int,
) {
return 5;
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a variant of #12451 that also adds maximum line width support.
It basically uses the current rule set and extends it by allowing the formatter to decide whether a given set of soft lines should be broken onto multiple lines, not just by determining whether the soft line group was multi-line in the input, but also whether it is needed to break it over multiple lines to prevent the document from going over the maximum line width.
To decide which output is optimal in terms of what needs line breaks and what does not, it uses this algorithm here, which is also linked in the corresponding issue in the topiary repository (topiary/topiary#700):
The score of every part of the document is lexicographical by:
(lower is better)
This ensures as long as the number of characters is less than the max line width, we strictly follow the authors layout (as 1. is 0, so we compare by 2. first and then 3.).