From e8b1d6842c1d19a74265ac112b0b2c8518937ac1 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 21:33:29 -0800 Subject: [PATCH 01/30] docs: add task list --- newline-query-optimization.todo | 152 ++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 newline-query-optimization.todo diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo new file mode 100644 index 0000000..b80de2e --- /dev/null +++ b/newline-query-optimization.todo @@ -0,0 +1,152 @@ +# Newline Query + Query Engine Optimization Plan + +Goal: make `Query.Newline` extremely optimized and semantically correct for "node occurs AFTER a newline", +while reducing allocations/overhead in the hottest Query paths (small scope). + +Confirmed semantics: +- "After a newline" means either: + - The node’s leading trivia contains a newline, OR + - The previous sibling’s trailing trivia contains a newline. + +Constraints: +- Keep change surface minimal: rewrite only the hottest LINQ paths. +- Preserve document order and all existing query semantics. +- Keep newline semantics identical in red matching and green matching. + +------------------------------------------------------------------------ + +## Phase 0 — Baseline + Safety Nets + +- [ ] Identify current implementation + hotspots + - [ ] Locate `NewlineNodeQuery` and its `TryMatchGreen(...)` implementation + - [ ] Find the selection-mode code path (`SelectModes.Last` / `ApplyMode`) + - [ ] Identify the hottest `Select(...)` implementations/combinators currently using LINQ + +- [ ] Add/confirm semantic tests for newline behavior + - [ ] Leading trivia newline qualifies as `Query.Newline` + - [ ] Previous sibling trailing trivia newline qualifies as `Query.Newline` + - [ ] Both cases work at top-level and inside blocks + - [ ] `Query.NotNewline` is the exact negation of `Query.Newline` for the same contexts + - [ ] Edge cases: first sibling (no previous sibling), empty blocks/regions + +Acceptance criteria: +- Existing behavior is captured in tests so perf refactors don’t change semantics. + +------------------------------------------------------------------------ + +## Phase 1 — Align Newline Semantics (Green Matching) + +- [ ] Update green matching for newline + - [ ] Implement both checks in `NewlineNodeQuery.TryMatchGreen(parent, childIndex, out consumedCount)`: + - [ ] Current node leading trivia contains newline + - [ ] Previous sibling trailing trivia contains newline (when `childIndex > 0`) + - [ ] Use only `(parent, childIndex)` context; do not allocate red siblings + - [ ] Keep `consumedCount` behavior identical (should remain 1) + +- [ ] Validate parity with red matching + - [ ] Ensure red matching logic (if separate) uses the same semantics + - [ ] Add a regression test that validates both red and green paths (if both are reachable) + +Acceptance criteria: +- `Query.Newline` matches exactly the confirmed semantics in all test cases. + +------------------------------------------------------------------------ + +## Phase 2 — Remove LINQ from Selection Modes (Start with `SelectModes.Last`) + +- [ ] Rewrite selection-mode handling without LINQ + - [ ] Replace `LastOrDefault()` usage with a simple scan that tracks the last match + - [ ] Ensure ordering/behavior matches existing semantics + - [ ] Avoid buffering unless semantics strictly require it + +- [ ] Expand to other modes only if they are on the hot path + - [ ] Audit remaining modes for LINQ/buffering + - [ ] Rewrite only those that show up in profiles/benchmarks + +Acceptance criteria: +- `SelectModes.Last` returns the same result as before, with fewer allocations. + +------------------------------------------------------------------------ + +## Phase 3 — Replace LINQ in the Hottest `Select(...)` Implementations (Small Scope) + +- [ ] Identify top `Select` hot paths + - [ ] Common kind queries (e.g., `KindNodeQuery.Select(...)`) + - [ ] Block queries and leaf queries used heavily by editor/regions + +- [ ] Replace common LINQ patterns with tight loops + - [ ] Replace `.Where(...)`, `.SelectMany(...)`, `.LastOrDefault()` in hot paths + - [ ] Preserve document order and short-circuiting behavior + - [ ] Avoid iterator/closure allocations where possible + +Acceptance criteria: +- Query results are byte-for-byte identical in ordering and content; allocations reduced in benchmarks. + +------------------------------------------------------------------------ + +## Phase 4 — Replace LINQ in the Hottest Combinators (Small Scope) + +- [ ] Remove LINQ where it forces buffering/extra iterators + - [ ] OR / AnyOf-style combinators + - [ ] AND / sequence-style combinators + - [ ] Any other combinator used by newline queries or editor/region resolution + +- [ ] Preserve semantics and ordering + - [ ] Document order remains stable + - [ ] No duplicate matches unless already part of the semantics + +Acceptance criteria: +- Combinator behavior remains unchanged; perf improves in existing suites. + +------------------------------------------------------------------------ + +## Phase 5 — Optimize Region Resolution (Materialize-on-Match) + +- [ ] Refactor region traversal to avoid per-visited-node allocations + - [ ] Maintain an incremental slot-index stack while walking the tree + - [ ] Construct `NodePath` only when a match is found (snapshot stack) + - [ ] Avoid `NodePath.FromNode(...)` and repeated `.ToArray()` allocations in scans + +- [ ] Validate correctness + - [ ] Query regions (`IRegionQuery` + match-based fallback) return identical regions + - [ ] Editor operations that depend on regions remain stable + +Acceptance criteria: +- Region-heavy operations allocate less and remain semantically identical. + +------------------------------------------------------------------------ + +## Phase 6 — Documentation Sanity + +- [ ] Update docs/comments to explicitly state newline semantics + - [ ] Clarify "node after newline" definition (leading trivia OR previous sibling trailing trivia) + - [ ] Remove/correct any mention of "newline whitespace tokens" if newline is trivia-only + - [ ] Ensure docs match actual implementation + +Suggested doc touchpoints: +- [ ] TinyTokenizer.wiki/Query-API.md +- [ ] TinyTokenizer.wiki/Trivia.md +- [ ] TinyTokenizer.wiki/TreeWalker.md (only if it mentions newline semantics) + +Acceptance criteria: +- Public docs and XML comments match runtime behavior. + +------------------------------------------------------------------------ + +## Phase 7 — Validation / Perf Smoke + +- [ ] Run unit tests + - [ ] `dotnet test TinyTokenizer.Tests` + +- [ ] Run benchmarks (baseline + comparison) + - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks` + - [ ] Filter relevant suites: + - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxTreeBenchmarks*` + - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxEditorBenchmarks*` + +- [ ] (Optional) Add a targeted benchmark if needed + - [ ] Benchmark `SyntaxTree.Select(Query.Newline)` + - [ ] Benchmark `SelectModes.Last` on a newline-heavy query + +Acceptance criteria: +- Tests pass; perf smoke shows reduced allocations or faster execution in the affected scenarios. From 3c4a69d705689538c49b3a763dc50fb892e3891c Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 21:40:48 -0800 Subject: [PATCH 02/30] [phase 0]: Baseline + Safety Nets --- TinyTokenizer.Tests/NewlineQueryTests.cs | 87 ++++++++++++++++++++++++ newline-query-optimization.todo | 22 +++--- 2 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 TinyTokenizer.Tests/NewlineQueryTests.cs diff --git a/TinyTokenizer.Tests/NewlineQueryTests.cs b/TinyTokenizer.Tests/NewlineQueryTests.cs new file mode 100644 index 0000000..79a7500 --- /dev/null +++ b/TinyTokenizer.Tests/NewlineQueryTests.cs @@ -0,0 +1,87 @@ +using System.Linq; +using TinyTokenizer.Ast; +using Xunit; + +namespace TinyTokenizer.Tests; + +[Trait("Category", "Query")] +public sealed class NewlineQueryTests +{ + [Fact] + public void Newline_MatchesNodeWithLeadingTriviaNewline_TopLevelFirstSibling() + { + var tree = SyntaxTree.Parse("\nfoo"); + var foo = tree.Root.Children.OfType().First(n => n.Kind == NodeKind.Ident); + + Assert.True(Query.Newline.Matches(foo)); + Assert.False(Query.NotNewline.Matches(foo)); + } + + [Fact] + public void Newline_MatchesNodeAfterNewlineViaPreviousSiblingTrailingTrivia_TopLevel() + { + var tree = SyntaxTree.Parse("x\ny"); + var idents = tree.Root.Children.OfType().Where(n => n.Kind == NodeKind.Ident).ToList(); + + Assert.Equal(2, idents.Count); + Assert.False(Query.Newline.Matches(idents[0])); + Assert.True(Query.Newline.Matches(idents[1])); + } + + [Fact] + public void Newline_MatchesFirstInnerNodeAfterOpenerViaPreviousSiblingTrailingTrivia_InBlock() + { + var tree = SyntaxTree.Parse("{\na}"); + var block = tree.Root.Children.OfType().Single(); + var a = block.InnerChildren.OfType().Single(n => n.Kind == NodeKind.Ident); + + Assert.True(Query.Newline.Matches(a)); + Assert.False(Query.NotNewline.Matches(a)); + } + + [Fact] + public void Newline_MatchesInnerNodeAfterNewlineBetweenSiblings_InBlock() + { + var tree = SyntaxTree.Parse("{a\nb}"); + var block = tree.Root.Children.OfType().Single(); + var idents = block.InnerChildren.OfType().Where(n => n.Kind == NodeKind.Ident).ToList(); + + Assert.Equal(2, idents.Count); + Assert.False(Query.Newline.Matches(idents[0])); + Assert.True(Query.Newline.Matches(idents[1])); + } + + [Fact] + public void NotNewline_IsExactNegationOfNewline_ForIdentifiersInSameTree() + { + var tree = SyntaxTree.Parse("a b\nc\n\nd"); + + var allIdents = tree.Select(Query.AnyIdent).ToList(); + var newlineIdents = tree.Select(Query.AnyIdent & Query.Newline).ToList(); + var notNewlineIdents = tree.Select(Query.AnyIdent & Query.NotNewline).ToList(); + + Assert.All(allIdents, n => Assert.True(newlineIdents.Contains(n) ^ notNewlineIdents.Contains(n))); + Assert.Empty(newlineIdents.Intersect(notNewlineIdents)); + Assert.Equal(allIdents.Count, newlineIdents.Count + notNewlineIdents.Count); + } + + [Fact] + public void Newline_DoesNotThrow_OnEmptyBlockOrEmptyTree() + { + var emptyTree = SyntaxTree.Parse(string.Empty); + Assert.Empty(emptyTree.Select(Query.Newline)); + + var emptyBlockTree = SyntaxTree.Parse("{}"); + Assert.Empty(emptyBlockTree.Select(Query.Newline)); + } + + [Fact] + public void Newline_MatchesCloserAfterNewlineInEmptyBlock() + { + var tree = SyntaxTree.Parse("{\n}"); + var block = tree.Root.Children.OfType().Single(); + + Assert.True(Query.Newline.Matches(block.CloserNode)); + Assert.False(Query.Newline.Matches(block.OpenerNode)); + } +} diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index b80de2e..88b66ff 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -17,17 +17,17 @@ Constraints: ## Phase 0 — Baseline + Safety Nets -- [ ] Identify current implementation + hotspots - - [ ] Locate `NewlineNodeQuery` and its `TryMatchGreen(...)` implementation - - [ ] Find the selection-mode code path (`SelectModes.Last` / `ApplyMode`) - - [ ] Identify the hottest `Select(...)` implementations/combinators currently using LINQ - -- [ ] Add/confirm semantic tests for newline behavior - - [ ] Leading trivia newline qualifies as `Query.Newline` - - [ ] Previous sibling trailing trivia newline qualifies as `Query.Newline` - - [ ] Both cases work at top-level and inside blocks - - [ ] `Query.NotNewline` is the exact negation of `Query.Newline` for the same contexts - - [ ] Edge cases: first sibling (no previous sibling), empty blocks/regions +- [x] Identify current implementation + hotspots + - [x] Locate `NewlineNodeQuery` and its `TryMatchGreen(...)` implementation + - [x] Find the selection-mode code path (`SelectModes.Last` / `ApplyMode`) + - [x] Identify the hottest `Select(...)` implementations/combinators currently using LINQ + +- [x] Add/confirm semantic tests for newline behavior + - [x] Leading trivia newline qualifies as `Query.Newline` + - [x] Previous sibling trailing trivia newline qualifies as `Query.Newline` + - [x] Both cases work at top-level and inside blocks + - [x] `Query.NotNewline` is the exact negation of `Query.Newline` for the same contexts + - [x] Edge cases: first sibling (no previous sibling), empty blocks/regions Acceptance criteria: - Existing behavior is captured in tests so perf refactors don’t change semantics. From d1e77856740e6de2e88e5bfd6d67b251b158b47f Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:31:59 -0800 Subject: [PATCH 03/30] docs: add sidequest task list --- newline-flags-token-centric.todo | 114 +++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 newline-flags-token-centric.todo diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo new file mode 100644 index 0000000..0819c43 --- /dev/null +++ b/newline-flags-token-centric.todo @@ -0,0 +1,114 @@ +# Token-Centric Newline Flags Refactor Plan + +Goal: Make newline-related `GreenNodeFlags` strictly reflect token-centric `Query.Newline` semantics: +- `Query.Newline` matches nodes that FOLLOW after a newline. +- Token-centric definition: newline is detected via: + - current node leading trivia contains a newline, OR + - previous sibling trailing trivia contains a newline. +- Avoid container-node “stacked matches” (e.g., `GreenList`, `GreenSyntaxNode` should not match just because their first child starts after a newline). + +Primary drivers: +- Flags should be cheap to read (ideally non-virtual field access). +- Flags should have strict meaning; no accidental propagation that changes query semantics. + +------------------------------------------------------------------------ + +## Phase 0 — Baseline + Guardrails + +- [ ] Confirm intended semantics in tests + - [ ] Add explicit tests proving `Query.Newline` does NOT match container nodes (root list / syntax containers) + - [ ] Example: on "\nfoo", `Query.Newline` matches `foo` token, but NOT `tree.Root` + - [ ] Example (schema/binding): on "\nfoo()" with syntax binding enabled, `Query.Newline` matches the first token after newline, but NOT the bound syntax node container unless it is itself a token/block boundary + - [ ] Add/confirm tests for existing newline cases remain unchanged (top-level + inside blocks) + +Acceptance criteria: +- Tests explicitly lock in token-centric behavior (no container matches). + +------------------------------------------------------------------------ + +## Phase 1 — Define Flag Semantics (Strict Meaning) + +- [ ] Decide/document strict meaning of newline-related flags + - [ ] `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia`: + - [ ] MUST mean: this node *owns* newline trivia on its boundary (not inherited from children) + - [ ] Valid for: `GreenLeaf` trivia boundaries, `GreenBlock` delimiter boundaries + - [ ] `ContainsNewlineTrivia`: + - [ ] MUST mean: newline trivia exists anywhere inside the node’s subtree (including children) + - [ ] Update inline comments in [TinyTokenizer/Ast/GreenNodeFlags.cs] to match this strict meaning + +Acceptance criteria: +- “Boundary” flags are never used to represent “first child boundary”. + +------------------------------------------------------------------------ + +## Phase 2 — Fix Boundary Flag Propagation in Container Nodes + +- [ ] Stop propagating boundary newline flags from children in container nodes + - [ ] Update `GreenList` flags computation + - [ ] Remove: boundary = (first.Flags & LeadingBoundary) | (last.Flags & TrailingBoundary) + - [ ] Replace with: boundary is always `None` for lists (or only set if list truly owns boundary trivia, which it currently does not) + - [ ] Keep: `contains` = OR of children `Contains` masks + - [ ] Update `GreenSyntaxNode` flags computation similarly + - [ ] Remove boundary propagation from first/last child + - [ ] Keep contains propagation + - [ ] Double-check any other `GreenContainer` types that compute flags similarly + - [ ] Search for `LeadingBoundary` / `TrailingBoundary` usage outside `GreenLeaf`/`GreenBlock` + +Acceptance criteria: +- Container nodes no longer report `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia` via child propagation. +- New token-centric tests from Phase 0 pass. + +------------------------------------------------------------------------ + +## Phase 3 — Make Flags a Field (Remove Virtual Property Overhead) + +- [ ] Refactor `GreenNode.Flags` from virtual property to base field storage + - [ ] Introduce an `internal readonly GreenNodeFlags Flags;` field in `GreenNode` + - [ ] Ensure it is set during construction of every concrete green node + - [ ] Remove `internal virtual GreenNodeFlags Flags => ...` and all overrides + - [ ] Update constructors: + - [ ] `GreenLeaf` sets boundary/contains flags based on trivia/kind + - [ ] `GreenBlock` sets boundary flags based on delimiter trivia, and contains flags across opener/children/closer + - [ ] `GreenList` and `GreenSyntaxNode` set contains flags (no boundary propagation) + - [ ] Any other green node types set flags consistently + - [ ] Ensure any green-node caches/factories still work (e.g., delimiter cache) + +Acceptance criteria: +- Flags reads are non-virtual and O(1) field access. +- All tests pass. + +------------------------------------------------------------------------ + +## Phase 4 — Update Newline Query to Use Flags First (No Type Checks) + +- [ ] Update `NewlineNodeQuery` to rely on strict flags + - [ ] `HasGreenNewline(node)` becomes: `(node.Flags & HasLeadingNewlineTrivia) != 0` + - [ ] `HasPreviousSiblingTrailingNewline(...)` becomes: `(prev.Flags & HasTrailingNewlineTrivia) != 0` + - [ ] Keep sibling-context logic in `TryMatchGreen` (can’t be encoded as single-node flag) + - [ ] Confirm no container nodes accidentally match due to flag propagation fixes (Phase 2) + +Acceptance criteria: +- `Query.Newline` behavior remains identical for tokens/blocks. +- Token-centric tests confirming “no container matches” pass. + +------------------------------------------------------------------------ + +## Phase 5 — Validate + Perf Smoke + +- [ ] Run full tests + - [ ] `dotnet test TinyTokenizer.Tests` + +- [ ] Add/update a targeted micro-benchmark (optional, if needed) + - [ ] Benchmark `SyntaxTree.Select(Query.Newline)` on newline-heavy source + - [ ] Benchmark green-path matching (`IGreenNodeQuery.TryMatchGreen`) for newline scanning + +Acceptance criteria: +- Tests pass. +- Allocations are reduced (or at least no regression) for newline-heavy selection. + +------------------------------------------------------------------------ + +## Notes / Risks + +- Changing boundary propagation semantics may affect any existing code that incorrectly relied on container boundary flags. Any such cases should switch to `ContainsNewlineTrivia` or explicit token queries. +- Making flags a field will require touching most green node constructors; keep the refactor mechanical and well-tested. From f61b1b462994eb80fc5059a0d45f7a9818ae20a0 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:37:30 -0800 Subject: [PATCH 04/30] [phase 0]: Baseline + Guardrails --- TinyTokenizer.Tests/NewlineQueryTests.cs | 34 ++++++++++++++++++++++++ newline-flags-token-centric.todo | 10 +++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/TinyTokenizer.Tests/NewlineQueryTests.cs b/TinyTokenizer.Tests/NewlineQueryTests.cs index 79a7500..90b0505 100644 --- a/TinyTokenizer.Tests/NewlineQueryTests.cs +++ b/TinyTokenizer.Tests/NewlineQueryTests.cs @@ -7,6 +7,15 @@ namespace TinyTokenizer.Tests; [Trait("Category", "Query")] public sealed class NewlineQueryTests { + private static Schema CreateSyntaxBindingSchema() + { + return Schema.Create() + .DefineSyntax(Syntax.Define("FunctionCall") + .Match(Query.AnyIdent, Query.ParenBlock) + .Build()) + .Build(); + } + [Fact] public void Newline_MatchesNodeWithLeadingTriviaNewline_TopLevelFirstSibling() { @@ -84,4 +93,29 @@ public void Newline_MatchesCloserAfterNewlineInEmptyBlock() Assert.True(Query.Newline.Matches(block.CloserNode)); Assert.False(Query.Newline.Matches(block.OpenerNode)); } + + [Fact] + public void Newline_DoesNotMatchRootContainerNode() + { + var tree = SyntaxTree.Parse("\nfoo"); + + Assert.False(Query.Newline.Matches(tree.Root)); + Assert.DoesNotContain(tree.Root, tree.Select(Query.Newline)); + } + + [Fact] + public void Newline_DoesNotMatchBoundSyntaxContainerNode() + { + var schema = CreateSyntaxBindingSchema(); + var tree = SyntaxTree.Parse("\nfoo()", schema); + + var funcCall = tree.Root.Children.OfType().First(); + + // Token-centric newline: matches tokens, not the bound syntax container. + Assert.False(Query.Newline.Matches(funcCall)); + Assert.DoesNotContain(funcCall, tree.Select(Query.Newline)); + + // Still matches the first token after newline. + Assert.True(Query.Newline.Matches(funcCall.NameNode)); + } } diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index 0819c43..23ed832 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -15,11 +15,11 @@ Primary drivers: ## Phase 0 — Baseline + Guardrails -- [ ] Confirm intended semantics in tests - - [ ] Add explicit tests proving `Query.Newline` does NOT match container nodes (root list / syntax containers) - - [ ] Example: on "\nfoo", `Query.Newline` matches `foo` token, but NOT `tree.Root` - - [ ] Example (schema/binding): on "\nfoo()" with syntax binding enabled, `Query.Newline` matches the first token after newline, but NOT the bound syntax node container unless it is itself a token/block boundary - - [ ] Add/confirm tests for existing newline cases remain unchanged (top-level + inside blocks) +- [x] Confirm intended semantics in tests + - [x] Add explicit tests proving `Query.Newline` does NOT match container nodes (root list / syntax containers) + - [x] Example: on "\nfoo", `Query.Newline` matches `foo` token, but NOT `tree.Root` + - [x] Example (schema/binding): on "\nfoo()" with syntax binding enabled, `Query.Newline` matches the first token after newline, but NOT the bound syntax node container unless it is itself a token/block boundary + - [x] Add/confirm tests for existing newline cases remain unchanged (top-level + inside blocks) Acceptance criteria: - Tests explicitly lock in token-centric behavior (no container matches). From ed46aa04cf10af249b92d8a85e5a57b9b8ccd929 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:40:05 -0800 Subject: [PATCH 05/30] [phase 1]: Define Flag Semantics --- TinyTokenizer/Ast/GreenNodeFlags.cs | 33 +++++++++++++++++++++++++++-- newline-flags-token-centric.todo | 14 ++++++------ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/TinyTokenizer/Ast/GreenNodeFlags.cs b/TinyTokenizer/Ast/GreenNodeFlags.cs index 89124a7..1c6a0b0 100644 --- a/TinyTokenizer/Ast/GreenNodeFlags.cs +++ b/TinyTokenizer/Ast/GreenNodeFlags.cs @@ -6,12 +6,41 @@ namespace TinyTokenizer.Ast; /// Bitflags describing trivia and content properties of a green node. /// Intended for O(1) query checks and subtree pruning. /// +/// +/// +/// These flags intentionally distinguish between boundary trivia and subtree contains. +/// +/// +/// +/// +/// Boundary flags (, +/// , etc.) mean the node itself +/// owns trivia on its boundary. For token-centric newline queries, this is the only +/// correct interpretation. +/// +/// +/// +/// +/// Contains flags (, etc.) mean the +/// trivia exists somewhere within the node's subtree (including children). +/// +/// +/// +/// +/// Important: boundary flags MUST NOT be used to represent "first child has boundary trivia". +/// Container nodes should not automatically inherit boundary flags from their first/last child. +/// +/// [Flags] internal enum GreenNodeFlags : uint { None = 0, - // Boundary trivia (left/right edge of the node's text span) + // Boundary trivia (owned by this node's boundary) + // + // For leaves: these correspond to the leaf's own leading/trailing trivia. + // For blocks: these correspond to opener leading and closer trailing trivia. + // For containers/lists: these should remain None unless the container explicitly owns trivia. HasLeadingNewlineTrivia = 1u << 0, HasTrailingNewlineTrivia = 1u << 1, @@ -21,7 +50,7 @@ internal enum GreenNodeFlags : uint HasLeadingCommentTrivia = 1u << 4, HasTrailingCommentTrivia = 1u << 5, - // Subtree flags (anywhere within the node's subtree) + // Subtree flags (anywhere within the node's subtree, including children) ContainsNewlineTrivia = 1u << 8, ContainsWhitespaceTrivia = 1u << 9, ContainsCommentTrivia = 1u << 10, diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index 23ed832..eb70f49 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -28,13 +28,13 @@ Acceptance criteria: ## Phase 1 — Define Flag Semantics (Strict Meaning) -- [ ] Decide/document strict meaning of newline-related flags - - [ ] `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia`: - - [ ] MUST mean: this node *owns* newline trivia on its boundary (not inherited from children) - - [ ] Valid for: `GreenLeaf` trivia boundaries, `GreenBlock` delimiter boundaries - - [ ] `ContainsNewlineTrivia`: - - [ ] MUST mean: newline trivia exists anywhere inside the node’s subtree (including children) - - [ ] Update inline comments in [TinyTokenizer/Ast/GreenNodeFlags.cs] to match this strict meaning +- [x] Decide/document strict meaning of newline-related flags + - [x] `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia`: + - [x] MUST mean: this node *owns* newline trivia on its boundary (not inherited from children) + - [x] Valid for: `GreenLeaf` trivia boundaries, `GreenBlock` delimiter boundaries + - [x] `ContainsNewlineTrivia`: + - [x] MUST mean: newline trivia exists anywhere inside the node’s subtree (including children) + - [x] Update inline comments in [TinyTokenizer/Ast/GreenNodeFlags.cs] to match this strict meaning Acceptance criteria: - “Boundary” flags are never used to represent “first child boundary”. From e947bfa713c1b2f7391381037b47d40ef6d4d081 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:43:26 -0800 Subject: [PATCH 06/30] [phase 2]: Fix Boundary Flag Propagation in Container Nodes --- TinyTokenizer.Tests/GreenNodeFlagsTests.cs | 10 +++++----- TinyTokenizer/Ast/GreenList.cs | 9 +++------ TinyTokenizer/Ast/GreenSyntaxNode.cs | 9 +++------ newline-flags-token-centric.todo | 20 ++++++++++---------- 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/TinyTokenizer.Tests/GreenNodeFlagsTests.cs b/TinyTokenizer.Tests/GreenNodeFlagsTests.cs index e0a8973..cf50f54 100644 --- a/TinyTokenizer.Tests/GreenNodeFlagsTests.cs +++ b/TinyTokenizer.Tests/GreenNodeFlagsTests.cs @@ -139,7 +139,7 @@ public void GreenBlock_Flags_AggregateContains_AndUseOpenerLeadingPlusCloserTrai } [Fact] - public void GreenList_Flags_UseFirstLeadingAndLastTrailingAsBoundary() + public void GreenList_Flags_DoNotPropagateBoundaryFlags_FromChildren() { var first = new GreenLeaf(NodeKind.Ident, "a", leadingTrivia: ImmutableArray.Create(GreenTrivia.Whitespace(" "))); var middle = new GreenLeaf(NodeKind.Ident, "b", trailingTrivia: ImmutableArray.Create(GreenTrivia.SingleLineComment("// c"))); @@ -147,8 +147,8 @@ public void GreenList_Flags_UseFirstLeadingAndLastTrailingAsBoundary() var list = new GreenList(ImmutableArray.Create(first, middle, last)); - AssertHas(list.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); - AssertHas(list.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + // Token-centric boundary semantics: lists do not own boundary trivia. + AssertNotHas(list.Flags, GreenNodeFlagMasks.Boundary); AssertHas(list.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); AssertHas(list.Flags, GreenNodeFlags.ContainsCommentTrivia); @@ -165,8 +165,8 @@ public void GreenSyntaxNode_Flags_AggregateLikeList() var node = new GreenSyntaxNode(kind, first, last); - AssertHas(node.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); - AssertHas(node.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + // Token-centric boundary semantics: syntax containers do not own boundary trivia. + AssertNotHas(node.Flags, GreenNodeFlagMasks.Boundary); AssertHas(node.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); AssertHas(node.Flags, GreenNodeFlags.ContainsNewlineTrivia); } diff --git a/TinyTokenizer/Ast/GreenList.cs b/TinyTokenizer/Ast/GreenList.cs index c6a3ab5..c2063ed 100644 --- a/TinyTokenizer/Ast/GreenList.cs +++ b/TinyTokenizer/Ast/GreenList.cs @@ -53,12 +53,9 @@ public GreenList(ImmutableArray children) } else { - var first = _children[0]; - var last = _children[^1]; - - var boundary = - (first.Flags & GreenNodeFlagMasks.LeadingBoundary) | - (last.Flags & GreenNodeFlagMasks.TrailingBoundary); + // Token-centric boundary semantics: + // Lists do not own boundary trivia, so boundary flags are not propagated from children. + var boundary = GreenNodeFlags.None; var contains = GreenNodeFlags.None; foreach (var child in _children) diff --git a/TinyTokenizer/Ast/GreenSyntaxNode.cs b/TinyTokenizer/Ast/GreenSyntaxNode.cs index ed21515..585a009 100644 --- a/TinyTokenizer/Ast/GreenSyntaxNode.cs +++ b/TinyTokenizer/Ast/GreenSyntaxNode.cs @@ -51,12 +51,9 @@ public GreenSyntaxNode(NodeKind kind, ImmutableArray children) } else { - var first = _children[0]; - var last = _children[^1]; - - var boundary = - (first.Flags & GreenNodeFlagMasks.LeadingBoundary) | - (last.Flags & GreenNodeFlagMasks.TrailingBoundary); + // Token-centric boundary semantics: + // Syntax containers do not own boundary trivia, so boundary flags are not propagated from children. + var boundary = GreenNodeFlags.None; var contains = GreenNodeFlags.None; foreach (var child in _children) diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index eb70f49..be76084 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -43,16 +43,16 @@ Acceptance criteria: ## Phase 2 — Fix Boundary Flag Propagation in Container Nodes -- [ ] Stop propagating boundary newline flags from children in container nodes - - [ ] Update `GreenList` flags computation - - [ ] Remove: boundary = (first.Flags & LeadingBoundary) | (last.Flags & TrailingBoundary) - - [ ] Replace with: boundary is always `None` for lists (or only set if list truly owns boundary trivia, which it currently does not) - - [ ] Keep: `contains` = OR of children `Contains` masks - - [ ] Update `GreenSyntaxNode` flags computation similarly - - [ ] Remove boundary propagation from first/last child - - [ ] Keep contains propagation - - [ ] Double-check any other `GreenContainer` types that compute flags similarly - - [ ] Search for `LeadingBoundary` / `TrailingBoundary` usage outside `GreenLeaf`/`GreenBlock` +- [x] Stop propagating boundary newline flags from children in container nodes + - [x] Update `GreenList` flags computation + - [x] Remove: boundary = (first.Flags & LeadingBoundary) | (last.Flags & TrailingBoundary) + - [x] Replace with: boundary is always `None` for lists (or only set if list truly owns boundary trivia, which it currently does not) + - [x] Keep: `contains` = OR of children `Contains` masks + - [x] Update `GreenSyntaxNode` flags computation similarly + - [x] Remove boundary propagation from first/last child + - [x] Keep contains propagation + - [x] Double-check any other `GreenContainer` types that compute flags similarly + - [x] Search for `LeadingBoundary` / `TrailingBoundary` usage outside `GreenLeaf`/`GreenBlock` Acceptance criteria: - Container nodes no longer report `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia` via child propagation. From 2f51de02341fcc038a17776e345f0b1a80bfce80 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:48:48 -0800 Subject: [PATCH 07/30] [phase 3]: Make Flags a Field --- TinyTokenizer/Ast/GreenBlock.cs | 72 +++++++++++++-------- TinyTokenizer/Ast/GreenContainer.cs | 5 ++ TinyTokenizer/Ast/GreenLeaf.cs | 93 +++++++++++++++++++--------- TinyTokenizer/Ast/GreenList.cs | 73 +++++++++++----------- TinyTokenizer/Ast/GreenNode.cs | 9 ++- TinyTokenizer/Ast/GreenSyntaxNode.cs | 52 ++++++++-------- newline-flags-token-centric.todo | 20 +++--- 7 files changed, 193 insertions(+), 131 deletions(-) diff --git a/TinyTokenizer/Ast/GreenBlock.cs b/TinyTokenizer/Ast/GreenBlock.cs index 115f71d..6a09b47 100644 --- a/TinyTokenizer/Ast/GreenBlock.cs +++ b/TinyTokenizer/Ast/GreenBlock.cs @@ -24,14 +24,10 @@ internal sealed record GreenBlock : GreenContainer private readonly ImmutableArray _children; private readonly int _width; - private readonly GreenNodeFlags _flags; private readonly int[]? _childOffsets; // Pre-computed for ≥10 children (O(1) lookup) /// public override NodeKind Kind { get; } - - /// - internal override GreenNodeFlags Flags => _flags; /// The opening delimiter node (e.g., '{', '[', '(') with its trivia. public GreenLeaf OpenerNode { get; } @@ -67,44 +63,66 @@ public GreenBlock( GreenLeaf openerNode, GreenLeaf closerNode, ImmutableArray children) + : this( + openerNode, + closerNode, + children.IsDefault ? ImmutableArray.Empty : children, + Compute(openerNode, closerNode, children.IsDefault ? ImmutableArray.Empty : children)) + { + } + + private GreenBlock( + GreenLeaf openerNode, + GreenLeaf closerNode, + ImmutableArray children, + BlockComputed computed) + : base(computed.Flags) { OpenerNode = openerNode; CloserNode = closerNode; Kind = GetBlockKind(Opener); - _children = children.IsDefault ? ImmutableArray.Empty : children; - - // Compute width: opener (with trivia) + children + closer (with trivia) + _children = children; + _width = computed.Width; + _childOffsets = computed.ChildOffsets; + } + + private static BlockComputed Compute(GreenLeaf openerNode, GreenLeaf closerNode, ImmutableArray children) + { int childrenWidth = 0; - foreach (var child in _children) + + int[]? offsets = null; + if (children.Length >= 10) + offsets = new int[children.Length]; + + int offset = openerNode.Width; + for (int i = 0; i < children.Length; i++) + { + if (offsets != null) + offsets[i] = offset; + + var child = children[i]; childrenWidth += child.Width; - - _width = OpenerNode.Width + childrenWidth + CloserNode.Width; + offset += child.Width; + } + + int width = openerNode.Width + childrenWidth + closerNode.Width; // Flags // - Boundary comes only from the first/last leaf boundaries (opener leading, closer trailing). // - Subtree contains flags are ORed across opener/inner/closer (excluding boundary bits). var boundary = - (OpenerNode.Flags & GreenNodeFlagMasks.LeadingBoundary) | - (CloserNode.Flags & GreenNodeFlagMasks.TrailingBoundary); + (openerNode.Flags & GreenNodeFlagMasks.LeadingBoundary) | + (closerNode.Flags & GreenNodeFlagMasks.TrailingBoundary); - var contains = (OpenerNode.Flags | CloserNode.Flags) & GreenNodeFlagMasks.Contains; - foreach (var child in _children) + var contains = (openerNode.Flags | closerNode.Flags) & GreenNodeFlagMasks.Contains; + foreach (var child in children) contains |= child.Flags & GreenNodeFlagMasks.Contains; - _flags = boundary | contains; - - // Pre-compute child offsets for large blocks - if (_children.Length >= 10) - { - _childOffsets = new int[_children.Length]; - int offset = OpenerNode.Width; // After opener (including its trivia) - for (int i = 0; i < _children.Length; i++) - { - _childOffsets[i] = offset; - offset += _children[i].Width; - } - } + var flags = boundary | contains; + return new BlockComputed(width, flags, offsets); } + + private readonly record struct BlockComputed(int Width, GreenNodeFlags Flags, int[]? ChildOffsets); /// /// Creates a new block node with automatic opener/closer creation. diff --git a/TinyTokenizer/Ast/GreenContainer.cs b/TinyTokenizer/Ast/GreenContainer.cs index 2df38dc..40859a4 100644 --- a/TinyTokenizer/Ast/GreenContainer.cs +++ b/TinyTokenizer/Ast/GreenContainer.cs @@ -18,6 +18,11 @@ namespace TinyTokenizer.Ast; [DebuggerDisplay("{DebuggerDisplay,nq}")] internal abstract record GreenContainer : GreenNode { + protected GreenContainer(GreenNodeFlags flags) + : base(flags) + { + } + /// protected override string DebuggerDisplay => $"{Kind}[{Width}] ({SlotCount} children)"; diff --git a/TinyTokenizer/Ast/GreenLeaf.cs b/TinyTokenizer/Ast/GreenLeaf.cs index bc1f0a6..503193f 100644 --- a/TinyTokenizer/Ast/GreenLeaf.cs +++ b/TinyTokenizer/Ast/GreenLeaf.cs @@ -17,13 +17,9 @@ internal sealed record GreenLeaf : GreenNode $"{Kind}[{Width}] \"{Truncate(Text, 20)}\""; private readonly int _width; - private readonly GreenNodeFlags _flags; /// public override NodeKind Kind { get; } - - /// - internal override GreenNodeFlags Flags => _flags; /// The text content of this token (excluding trivia). public string Text { get; } @@ -60,36 +56,34 @@ public GreenLeaf( string text, ImmutableArray leadingTrivia = default, ImmutableArray trailingTrivia = default) + : this( + kind, + text, + leadingTrivia.IsDefault ? ImmutableArray.Empty : leadingTrivia, + trailingTrivia.IsDefault ? ImmutableArray.Empty : trailingTrivia, + Compute(kind, + text, + leadingTrivia.IsDefault ? ImmutableArray.Empty : leadingTrivia, + trailingTrivia.IsDefault ? ImmutableArray.Empty : trailingTrivia)) + { + } + + private GreenLeaf( + NodeKind kind, + string text, + ImmutableArray leadingTrivia, + ImmutableArray trailingTrivia, + LeafComputed computed) + : base(computed.Flags) { Kind = kind; Text = text; - LeadingTrivia = leadingTrivia.IsDefault ? ImmutableArray.Empty : leadingTrivia; - TrailingTrivia = trailingTrivia.IsDefault ? ImmutableArray.Empty : trailingTrivia; - - LeadingTriviaWidth = ComputeTriviaWidthAndFlags( - LeadingTrivia, - isLeading: true, - out var leadingBoundaryFlags, - out var leadingContainsFlags); - - TrailingTriviaWidth = ComputeTriviaWidthAndFlags( - TrailingTrivia, - isLeading: false, - out var trailingBoundaryFlags, - out var trailingContainsFlags); - - _width = LeadingTriviaWidth + Text.Length + TrailingTriviaWidth; - - // Subtree flags based on node kind - var kindFlags = GreenNodeFlags.None; - if (kind == NodeKind.Error) - kindFlags |= GreenNodeFlags.ContainsErrorNode; - if (kind == NodeKind.TaggedIdent) - kindFlags |= GreenNodeFlags.ContainsTaggedIdent; - if (kind.IsKeyword()) - kindFlags |= GreenNodeFlags.ContainsKeyword; + LeadingTrivia = leadingTrivia; + TrailingTrivia = trailingTrivia; - _flags = leadingBoundaryFlags | trailingBoundaryFlags | leadingContainsFlags | trailingContainsFlags | kindFlags; + LeadingTriviaWidth = computed.LeadingTriviaWidth; + TrailingTriviaWidth = computed.TrailingTriviaWidth; + _width = computed.Width; } /// @@ -138,6 +132,45 @@ public GreenLeaf WithTrailingTrivia(ImmutableArray trivia) /// public GreenLeaf WithText(string text) => new(Kind, text, LeadingTrivia, TrailingTrivia); + + private static LeafComputed Compute( + NodeKind kind, + string text, + ImmutableArray leadingTrivia, + ImmutableArray trailingTrivia) + { + int leadingWidth = ComputeTriviaWidthAndFlags( + leadingTrivia, + isLeading: true, + out var leadingBoundaryFlags, + out var leadingContainsFlags); + + int trailingWidth = ComputeTriviaWidthAndFlags( + trailingTrivia, + isLeading: false, + out var trailingBoundaryFlags, + out var trailingContainsFlags); + + int width = leadingWidth + text.Length + trailingWidth; + + // Subtree flags based on node kind + var kindFlags = GreenNodeFlags.None; + if (kind == NodeKind.Error) + kindFlags |= GreenNodeFlags.ContainsErrorNode; + if (kind == NodeKind.TaggedIdent) + kindFlags |= GreenNodeFlags.ContainsTaggedIdent; + if (kind.IsKeyword()) + kindFlags |= GreenNodeFlags.ContainsKeyword; + + var flags = leadingBoundaryFlags | trailingBoundaryFlags | leadingContainsFlags | trailingContainsFlags | kindFlags; + return new LeafComputed(leadingWidth, trailingWidth, width, flags); + } + + private readonly record struct LeafComputed( + int LeadingTriviaWidth, + int TrailingTriviaWidth, + int Width, + GreenNodeFlags Flags); private static int ComputeTriviaWidthAndFlags( ImmutableArray trivia, diff --git a/TinyTokenizer/Ast/GreenList.cs b/TinyTokenizer/Ast/GreenList.cs index c2063ed..89dac24 100644 --- a/TinyTokenizer/Ast/GreenList.cs +++ b/TinyTokenizer/Ast/GreenList.cs @@ -18,7 +18,6 @@ internal sealed record GreenList : GreenContainer private readonly ImmutableArray _children; private readonly int _width; - private readonly GreenNodeFlags _flags; private readonly int[]? _childOffsets; /// @@ -26,9 +25,6 @@ internal sealed record GreenList : GreenContainer /// public override int Width => _width; - - /// - internal override GreenNodeFlags Flags => _flags; /// public override ImmutableArray Children => _children; @@ -37,45 +33,50 @@ internal sealed record GreenList : GreenContainer /// Creates a new token list. /// public GreenList(ImmutableArray children) + : this( + children.IsDefault ? ImmutableArray.Empty : children, + Compute(children.IsDefault ? ImmutableArray.Empty : children)) { - _children = children.IsDefault ? ImmutableArray.Empty : children; - - // Compute width - int width = 0; - foreach (var child in _children) - width += child.Width; - _width = width; + } - // Flags - if (_children.Length == 0) - { - _flags = GreenNodeFlags.None; - } - else - { - // Token-centric boundary semantics: - // Lists do not own boundary trivia, so boundary flags are not propagated from children. - var boundary = GreenNodeFlags.None; + private GreenList(ImmutableArray children, ListComputed computed) + : base(computed.Flags) + { + _children = children; + _width = computed.Width; + _childOffsets = computed.ChildOffsets; + } - var contains = GreenNodeFlags.None; - foreach (var child in _children) - contains |= child.Flags & GreenNodeFlagMasks.Contains; + private static ListComputed Compute(ImmutableArray children) + { + if (children.Length == 0) + return new ListComputed(Width: 0, Flags: GreenNodeFlags.None, ChildOffsets: null); - _flags = boundary | contains; - } - - // Pre-compute offsets for large lists - if (_children.Length >= 10) + int width = 0; + var contains = GreenNodeFlags.None; + + int[]? offsets = null; + if (children.Length >= 10) + offsets = new int[children.Length]; + + int offset = 0; + for (int i = 0; i < children.Length; i++) { - _childOffsets = new int[_children.Length]; - int offset = 0; - for (int i = 0; i < _children.Length; i++) - { - _childOffsets[i] = offset; - offset += _children[i].Width; - } + if (offsets != null) + offsets[i] = offset; + + var child = children[i]; + width += child.Width; + offset += child.Width; + contains |= child.Flags & GreenNodeFlagMasks.Contains; } + + // Token-centric boundary semantics: lists do not own boundary trivia. + var flags = contains; + return new ListComputed(width, flags, offsets); } + + private readonly record struct ListComputed(int Width, GreenNodeFlags Flags, int[]? ChildOffsets); /// public override GreenNode? GetSlot(int index) diff --git a/TinyTokenizer/Ast/GreenNode.cs b/TinyTokenizer/Ast/GreenNode.cs index a9e65e5..6809d31 100644 --- a/TinyTokenizer/Ast/GreenNode.cs +++ b/TinyTokenizer/Ast/GreenNode.cs @@ -14,6 +14,11 @@ namespace TinyTokenizer.Ast; [DebuggerDisplay("{DebuggerDisplay,nq}")] internal abstract record GreenNode : IFormattable, ITextSerializable { + protected GreenNode(GreenNodeFlags flags) + { + Flags = flags; + } + /// /// Gets the debugger display string for this node. /// Override in derived classes for specialized display. @@ -39,9 +44,9 @@ protected static string Truncate(string text, int maxLength) /// /// Cached flags describing trivia/content properties for fast queries. - /// Concrete green node types override this once flags are computed. + /// Stored directly on the green node for O(1) access. /// - internal virtual GreenNodeFlags Flags => GreenNodeFlags.None; + internal readonly GreenNodeFlags Flags; /// /// Total character width of this node, including any trivia. diff --git a/TinyTokenizer/Ast/GreenSyntaxNode.cs b/TinyTokenizer/Ast/GreenSyntaxNode.cs index 585a009..36d8e58 100644 --- a/TinyTokenizer/Ast/GreenSyntaxNode.cs +++ b/TinyTokenizer/Ast/GreenSyntaxNode.cs @@ -24,7 +24,6 @@ internal sealed record GreenSyntaxNode : GreenContainer private readonly ImmutableArray _children; private readonly NodeKind _kind; private readonly int _width; - private readonly GreenNodeFlags _flags; /// /// Creates a green syntax node wrapping the specified children. @@ -32,36 +31,40 @@ internal sealed record GreenSyntaxNode : GreenContainer /// The semantic NodeKind for this syntax construct. /// The child green nodes that make up this syntax construct. public GreenSyntaxNode(NodeKind kind, ImmutableArray children) + : this( + kind, + children.IsDefault ? ImmutableArray.Empty : children, + Compute(children.IsDefault ? ImmutableArray.Empty : children)) + { + } + + private GreenSyntaxNode(NodeKind kind, ImmutableArray children, SyntaxNodeComputed computed) + : base(computed.Flags) { _kind = kind; - _children = children.IsDefault ? ImmutableArray.Empty : children; - - // Calculate total width + _children = children; + _width = computed.Width; + } + + private static SyntaxNodeComputed Compute(ImmutableArray children) + { + if (children.Length == 0) + return new SyntaxNodeComputed(Width: 0, Flags: GreenNodeFlags.None); + int width = 0; - foreach (var child in _children) + var contains = GreenNodeFlags.None; + foreach (var child in children) { width += child.Width; + contains |= child.Flags & GreenNodeFlagMasks.Contains; } - _width = width; - // Flags - if (_children.Length == 0) - { - _flags = GreenNodeFlags.None; - } - else - { - // Token-centric boundary semantics: - // Syntax containers do not own boundary trivia, so boundary flags are not propagated from children. - var boundary = GreenNodeFlags.None; - - var contains = GreenNodeFlags.None; - foreach (var child in _children) - contains |= child.Flags & GreenNodeFlagMasks.Contains; - - _flags = boundary | contains; - } + // Token-centric boundary semantics: syntax containers do not own boundary trivia. + var flags = contains; + return new SyntaxNodeComputed(width, flags); } + + private readonly record struct SyntaxNodeComputed(int Width, GreenNodeFlags Flags); /// /// Creates a green syntax node from params array of children. @@ -76,9 +79,6 @@ public GreenSyntaxNode(NodeKind kind, params GreenNode[] children) /// public override int Width => _width; - - /// - internal override GreenNodeFlags Flags => _flags; /// public override ImmutableArray Children => _children; diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index be76084..1060cd6 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -62,16 +62,16 @@ Acceptance criteria: ## Phase 3 — Make Flags a Field (Remove Virtual Property Overhead) -- [ ] Refactor `GreenNode.Flags` from virtual property to base field storage - - [ ] Introduce an `internal readonly GreenNodeFlags Flags;` field in `GreenNode` - - [ ] Ensure it is set during construction of every concrete green node - - [ ] Remove `internal virtual GreenNodeFlags Flags => ...` and all overrides - - [ ] Update constructors: - - [ ] `GreenLeaf` sets boundary/contains flags based on trivia/kind - - [ ] `GreenBlock` sets boundary flags based on delimiter trivia, and contains flags across opener/children/closer - - [ ] `GreenList` and `GreenSyntaxNode` set contains flags (no boundary propagation) - - [ ] Any other green node types set flags consistently - - [ ] Ensure any green-node caches/factories still work (e.g., delimiter cache) +- [x] Refactor `GreenNode.Flags` from virtual property to base field storage + - [x] Introduce an `internal readonly GreenNodeFlags Flags;` field in `GreenNode` + - [x] Ensure it is set during construction of every concrete green node + - [x] Remove `internal virtual GreenNodeFlags Flags => ...` and all overrides + - [x] Update constructors: + - [x] `GreenLeaf` sets boundary/contains flags based on trivia/kind + - [x] `GreenBlock` sets boundary flags based on delimiter trivia, and contains flags across opener/children/closer + - [x] `GreenList` and `GreenSyntaxNode` set contains flags (no boundary propagation) + - [x] Any other green node types set flags consistently + - [x] Ensure any green-node caches/factories still work (e.g., delimiter cache) Acceptance criteria: - Flags reads are non-virtual and O(1) field access. From 03797ecdd86b655e7d265b4a95d38e40453137d8 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:55:08 -0800 Subject: [PATCH 08/30] [phase 4]: Update Newline Query to Use Flags First --- TinyTokenizer/Ast/NodeQueryTypes.cs | 90 ++++++++++++++--------------- newline-flags-token-centric.todo | 10 ++-- 2 files changed, 47 insertions(+), 53 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index 0648bf3..ea691bf 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -681,9 +681,8 @@ protected override LeafNodeQuery CreateFiltered(Func predicate /// /// Matches nodes that represent or are preceded by a newline. /// Checks: -/// 1. The node itself is a whitespace token containing newline characters. -/// 2. The node's leading trivia contains a newline. -/// 3. The previous sibling's trailing trivia contains a newline. +/// 1. The node's leading boundary trivia contains a newline. +/// 2. The previous sibling's trailing boundary trivia contains a newline. /// /// /// This query is particularly useful for line-based pattern matching, such as @@ -744,61 +743,56 @@ internal override bool MatchesGreen(GreenNode node) bool hasNewline = HasGreenNewline(node); return _negated ? !hasNewline : hasNewline; } - - private static bool HasGreenNewline(GreenNode node) + + internal override bool TryMatchGreen(IReadOnlyList siblings, int startIndex, out int consumedCount) { - // Check leading trivia for newline - var leadingTrivia = node switch + if ((uint)startIndex >= (uint)siblings.Count) { - GreenLeaf gl => gl.LeadingTrivia, - GreenBlock gb => gb.LeadingTrivia, - _ => System.Collections.Immutable.ImmutableArray.Empty - }; - - foreach (var t in leadingTrivia) + consumedCount = 0; + return false; + } + + var node = siblings[startIndex]; + + // Token-centric newline semantics: + // - current node matches if it owns leading newline trivia + // - OR if previous sibling owns trailing newline trivia + bool hasNewline = HasGreenNewline(node); + if (!hasNewline && startIndex > 0) { - if (t.Kind == TriviaKind.Newline) - return true; + hasNewline = HasGreenTrailingNewline(siblings[startIndex - 1]); } - + + bool matched = _negated ? !hasNewline : hasNewline; + if (matched) + { + consumedCount = 1; + return true; + } + + consumedCount = 0; return false; } + private static bool HasGreenNewline(GreenNode node) + { + return (node.Flags & GreenNodeFlags.HasLeadingNewlineTrivia) != 0; + } + + private static bool HasGreenTrailingNewline(GreenNode node) + { + return (node.Flags & GreenNodeFlags.HasTrailingNewlineTrivia) != 0; + } + private static bool HasNewline(SyntaxNode node) { - // Check 1: Does leading trivia contain newline? - var leadingTrivia = node.Green switch - { - GreenLeaf gl => gl.LeadingTrivia, - GreenBlock gb => gb.LeadingTrivia, - _ => System.Collections.Immutable.ImmutableArray.Empty - }; - - foreach (var t in leadingTrivia) - { - if (t.Kind == TriviaKind.Newline) - return true; - } - - // Check 2: Does previous sibling's trailing trivia contain newline? + // Check 1: does this node own leading newline trivia? + if ((node.Green.Flags & GreenNodeFlags.HasLeadingNewlineTrivia) != 0) + return true; + + // Check 2: does previous sibling own trailing newline trivia? var prev = node.PreviousSibling(); - if (prev != null) - { - var trailingTrivia = prev.Green switch - { - GreenLeaf gl => gl.TrailingTrivia, - GreenBlock gb => gb.TrailingTrivia, - _ => System.Collections.Immutable.ImmutableArray.Empty - }; - - foreach (var t in trailingTrivia) - { - if (t.Kind == TriviaKind.Newline) - return true; - } - } - - return false; + return prev != null && (prev.Green.Flags & GreenNodeFlags.HasTrailingNewlineTrivia) != 0; } protected override NewlineNodeQuery CreateFiltered(Func predicate) => diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index 1060cd6..5673373 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -81,11 +81,11 @@ Acceptance criteria: ## Phase 4 — Update Newline Query to Use Flags First (No Type Checks) -- [ ] Update `NewlineNodeQuery` to rely on strict flags - - [ ] `HasGreenNewline(node)` becomes: `(node.Flags & HasLeadingNewlineTrivia) != 0` - - [ ] `HasPreviousSiblingTrailingNewline(...)` becomes: `(prev.Flags & HasTrailingNewlineTrivia) != 0` - - [ ] Keep sibling-context logic in `TryMatchGreen` (can’t be encoded as single-node flag) - - [ ] Confirm no container nodes accidentally match due to flag propagation fixes (Phase 2) +- [x] Update `NewlineNodeQuery` to rely on strict flags + - [x] `HasGreenNewline(node)` becomes: `(node.Flags & HasLeadingNewlineTrivia) != 0` + - [x] `HasPreviousSiblingTrailingNewline(...)` becomes: `(prev.Flags & HasTrailingNewlineTrivia) != 0` + - [x] Keep sibling-context logic in `TryMatchGreen` (can’t be encoded as single-node flag) + - [x] Confirm no container nodes accidentally match due to flag propagation fixes (Phase 2) Acceptance criteria: - `Query.Newline` behavior remains identical for tokens/blocks. From c484681b4440512d1c5668d9ca932ee887a811d8 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 22:59:20 -0800 Subject: [PATCH 09/30] [phase 5]: Validate --- newline-flags-token-centric.todo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index 5673373..695ff43 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -95,8 +95,8 @@ Acceptance criteria: ## Phase 5 — Validate + Perf Smoke -- [ ] Run full tests - - [ ] `dotnet test TinyTokenizer.Tests` +- [x] Run full tests + - [x] `dotnet test TinyTokenizer.Tests` - [ ] Add/update a targeted micro-benchmark (optional, if needed) - [ ] Benchmark `SyntaxTree.Select(Query.Newline)` on newline-heavy source From add76fe8e4afaac0dfccce2834eead77d861d1fa Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 23:44:12 -0800 Subject: [PATCH 10/30] [phase 5]: Perf tests --- .../NewlineQueryBenchmarks.cs | 72 +++++++++++++++++++ TinyTokenizer.Benchmarks/Program.cs | 4 +- newline-flags-token-centric.todo | 6 +- 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 TinyTokenizer.Benchmarks/NewlineQueryBenchmarks.cs diff --git a/TinyTokenizer.Benchmarks/NewlineQueryBenchmarks.cs b/TinyTokenizer.Benchmarks/NewlineQueryBenchmarks.cs new file mode 100644 index 0000000..b071863 --- /dev/null +++ b/TinyTokenizer.Benchmarks/NewlineQueryBenchmarks.cs @@ -0,0 +1,72 @@ +using System.Text; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using TinyTokenizer.Ast; +using Q = TinyTokenizer.Ast.Query; + +namespace TinyTokenizer.Benchmarks; + +/// +/// Benchmarks for token-centric newline detection via Query.Newline. +/// Measures the cost of scanning a newline-heavy tree and matching nodes +/// that follow a newline (current leading newline OR previous sibling trailing newline). +/// +[MemoryDiagnoser] +[GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)] +[CategoriesColumn] +public class NewlineQueryBenchmarks +{ + [Params(1_000, 10_000)] + public int Lines { get; set; } + + private SyntaxTree _tree = null!; + + [GlobalSetup] + public void Setup() + { + var source = GenerateNewlineHeavyInput(Lines); + _tree = SyntaxTree.Parse(source); + } + + [Benchmark(Description = "Select(Query.Newline) - Count")] + [BenchmarkCategory("Query", "Newline")] + public int SelectNewline_Count() + { + int count = 0; + foreach (var _ in _tree.Select(Q.Newline)) + count++; + return count; + } + + [Benchmark(Description = "Select(Query.Newline.First()) - First match")] + [BenchmarkCategory("Query", "Newline")] + public int SelectNewline_First() + { + foreach (var node in _tree.Select(Q.Newline.First())) + return node.Position; + return -1; + } + + private static string GenerateNewlineHeavyInput(int lines) + { + // Intentionally mixes: trailing-newline ownership (end-of-line) and + // leading-newline ownership (own-line comments become leading trivia). + var builder = new StringBuilder(capacity: lines * 32); + + for (int i = 0; i < lines; i++) + { + builder.Append("x"); + builder.Append(i); + builder.Append(" = "); + builder.Append(i); + builder.Append(";\n"); + + if ((i & 7) == 0) + { + builder.Append("// comment\n"); + } + } + + return builder.ToString(); + } +} diff --git a/TinyTokenizer.Benchmarks/Program.cs b/TinyTokenizer.Benchmarks/Program.cs index 5996970..aff6351 100644 --- a/TinyTokenizer.Benchmarks/Program.cs +++ b/TinyTokenizer.Benchmarks/Program.cs @@ -1,4 +1,6 @@ using BenchmarkDotNet.Running; using TinyTokenizer.Benchmarks; -BenchmarkSwitcher.FromAssembly(typeof(LexerBenchmarks).Assembly).Run(args); +BenchmarkSwitcher.FromTypes([ + typeof(NewlineQueryBenchmarks) +]).Run(args); diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo index 695ff43..a59e52e 100644 --- a/newline-flags-token-centric.todo +++ b/newline-flags-token-centric.todo @@ -98,9 +98,9 @@ Acceptance criteria: - [x] Run full tests - [x] `dotnet test TinyTokenizer.Tests` -- [ ] Add/update a targeted micro-benchmark (optional, if needed) - - [ ] Benchmark `SyntaxTree.Select(Query.Newline)` on newline-heavy source - - [ ] Benchmark green-path matching (`IGreenNodeQuery.TryMatchGreen`) for newline scanning +- [x] Add/update a targeted micro-benchmark (optional, if needed) + - [x] Benchmark `SyntaxTree.Select(Query.Newline)` on newline-heavy source + - [x] Benchmark green-path matching (`IGreenNodeQuery.TryMatchGreen`) for newline scanning Acceptance criteria: - Tests pass. From 3473cae84afb87c5d782410759a6a44854b521b7 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sat, 10 Jan 2026 23:44:34 -0800 Subject: [PATCH 11/30] chore: remove completed task list --- newline-flags-token-centric.todo | 114 ------------------------------- 1 file changed, 114 deletions(-) delete mode 100644 newline-flags-token-centric.todo diff --git a/newline-flags-token-centric.todo b/newline-flags-token-centric.todo deleted file mode 100644 index a59e52e..0000000 --- a/newline-flags-token-centric.todo +++ /dev/null @@ -1,114 +0,0 @@ -# Token-Centric Newline Flags Refactor Plan - -Goal: Make newline-related `GreenNodeFlags` strictly reflect token-centric `Query.Newline` semantics: -- `Query.Newline` matches nodes that FOLLOW after a newline. -- Token-centric definition: newline is detected via: - - current node leading trivia contains a newline, OR - - previous sibling trailing trivia contains a newline. -- Avoid container-node “stacked matches” (e.g., `GreenList`, `GreenSyntaxNode` should not match just because their first child starts after a newline). - -Primary drivers: -- Flags should be cheap to read (ideally non-virtual field access). -- Flags should have strict meaning; no accidental propagation that changes query semantics. - ------------------------------------------------------------------------- - -## Phase 0 — Baseline + Guardrails - -- [x] Confirm intended semantics in tests - - [x] Add explicit tests proving `Query.Newline` does NOT match container nodes (root list / syntax containers) - - [x] Example: on "\nfoo", `Query.Newline` matches `foo` token, but NOT `tree.Root` - - [x] Example (schema/binding): on "\nfoo()" with syntax binding enabled, `Query.Newline` matches the first token after newline, but NOT the bound syntax node container unless it is itself a token/block boundary - - [x] Add/confirm tests for existing newline cases remain unchanged (top-level + inside blocks) - -Acceptance criteria: -- Tests explicitly lock in token-centric behavior (no container matches). - ------------------------------------------------------------------------- - -## Phase 1 — Define Flag Semantics (Strict Meaning) - -- [x] Decide/document strict meaning of newline-related flags - - [x] `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia`: - - [x] MUST mean: this node *owns* newline trivia on its boundary (not inherited from children) - - [x] Valid for: `GreenLeaf` trivia boundaries, `GreenBlock` delimiter boundaries - - [x] `ContainsNewlineTrivia`: - - [x] MUST mean: newline trivia exists anywhere inside the node’s subtree (including children) - - [x] Update inline comments in [TinyTokenizer/Ast/GreenNodeFlags.cs] to match this strict meaning - -Acceptance criteria: -- “Boundary” flags are never used to represent “first child boundary”. - ------------------------------------------------------------------------- - -## Phase 2 — Fix Boundary Flag Propagation in Container Nodes - -- [x] Stop propagating boundary newline flags from children in container nodes - - [x] Update `GreenList` flags computation - - [x] Remove: boundary = (first.Flags & LeadingBoundary) | (last.Flags & TrailingBoundary) - - [x] Replace with: boundary is always `None` for lists (or only set if list truly owns boundary trivia, which it currently does not) - - [x] Keep: `contains` = OR of children `Contains` masks - - [x] Update `GreenSyntaxNode` flags computation similarly - - [x] Remove boundary propagation from first/last child - - [x] Keep contains propagation - - [x] Double-check any other `GreenContainer` types that compute flags similarly - - [x] Search for `LeadingBoundary` / `TrailingBoundary` usage outside `GreenLeaf`/`GreenBlock` - -Acceptance criteria: -- Container nodes no longer report `HasLeadingNewlineTrivia` / `HasTrailingNewlineTrivia` via child propagation. -- New token-centric tests from Phase 0 pass. - ------------------------------------------------------------------------- - -## Phase 3 — Make Flags a Field (Remove Virtual Property Overhead) - -- [x] Refactor `GreenNode.Flags` from virtual property to base field storage - - [x] Introduce an `internal readonly GreenNodeFlags Flags;` field in `GreenNode` - - [x] Ensure it is set during construction of every concrete green node - - [x] Remove `internal virtual GreenNodeFlags Flags => ...` and all overrides - - [x] Update constructors: - - [x] `GreenLeaf` sets boundary/contains flags based on trivia/kind - - [x] `GreenBlock` sets boundary flags based on delimiter trivia, and contains flags across opener/children/closer - - [x] `GreenList` and `GreenSyntaxNode` set contains flags (no boundary propagation) - - [x] Any other green node types set flags consistently - - [x] Ensure any green-node caches/factories still work (e.g., delimiter cache) - -Acceptance criteria: -- Flags reads are non-virtual and O(1) field access. -- All tests pass. - ------------------------------------------------------------------------- - -## Phase 4 — Update Newline Query to Use Flags First (No Type Checks) - -- [x] Update `NewlineNodeQuery` to rely on strict flags - - [x] `HasGreenNewline(node)` becomes: `(node.Flags & HasLeadingNewlineTrivia) != 0` - - [x] `HasPreviousSiblingTrailingNewline(...)` becomes: `(prev.Flags & HasTrailingNewlineTrivia) != 0` - - [x] Keep sibling-context logic in `TryMatchGreen` (can’t be encoded as single-node flag) - - [x] Confirm no container nodes accidentally match due to flag propagation fixes (Phase 2) - -Acceptance criteria: -- `Query.Newline` behavior remains identical for tokens/blocks. -- Token-centric tests confirming “no container matches” pass. - ------------------------------------------------------------------------- - -## Phase 5 — Validate + Perf Smoke - -- [x] Run full tests - - [x] `dotnet test TinyTokenizer.Tests` - -- [x] Add/update a targeted micro-benchmark (optional, if needed) - - [x] Benchmark `SyntaxTree.Select(Query.Newline)` on newline-heavy source - - [x] Benchmark green-path matching (`IGreenNodeQuery.TryMatchGreen`) for newline scanning - -Acceptance criteria: -- Tests pass. -- Allocations are reduced (or at least no regression) for newline-heavy selection. - ------------------------------------------------------------------------- - -## Notes / Risks - -- Changing boundary propagation semantics may affect any existing code that incorrectly relied on container boundary flags. Any such cases should switch to `ContainsNewlineTrivia` or explicit token queries. -- Making flags a field will require touching most green node constructors; keep the refactor mechanical and well-tested. From 988bbed2e721d36cc849039650763f4c4473cd62 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 00:19:05 -0800 Subject: [PATCH 12/30] chore: add new phase --- newline-query-optimization.todo | 49 ++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 88b66ff..73adcde 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -8,6 +8,11 @@ Confirmed semantics: - The node’s leading trivia contains a newline, OR - The previous sibling’s trailing trivia contains a newline. +Implementation note (post node-flags work): +- These semantics are implemented via green-node *boundary* flags (token-centric): + - `HasLeadingNewline` on the current node, OR + - `HasTrailingNewline` on the previous sibling. + Constraints: - Keep change surface minimal: rewrite only the hottest LINQ paths. - Preserve document order and all existing query semantics. @@ -36,19 +41,42 @@ Acceptance criteria: ## Phase 1 — Align Newline Semantics (Green Matching) -- [ ] Update green matching for newline - - [ ] Implement both checks in `NewlineNodeQuery.TryMatchGreen(parent, childIndex, out consumedCount)`: - - [ ] Current node leading trivia contains newline - - [ ] Previous sibling trailing trivia contains newline (when `childIndex > 0`) - - [ ] Use only `(parent, childIndex)` context; do not allocate red siblings - - [ ] Keep `consumedCount` behavior identical (should remain 1) +- [x] Update green matching for newline + - [x] Implement both checks in `NewlineNodeQuery.TryMatchGreen(parent, childIndex, out consumedCount)`: + - [x] Current node `HasLeadingNewline` + - [x] Previous sibling `HasTrailingNewline` (when `childIndex > 0`) + - [x] Use only `(parent, childIndex)` context; do not allocate red siblings + - [x] Keep `consumedCount` behavior identical (should remain 1) -- [ ] Validate parity with red matching - - [ ] Ensure red matching logic (if separate) uses the same semantics - - [ ] Add a regression test that validates both red and green paths (if both are reachable) +- [x] Validate parity with red matching + - [x] Ensure red matching logic uses the same semantics (leading OR previous sibling trailing) Acceptance criteria: - `Query.Newline` matches exactly the confirmed semantics in all test cases. +- Green matching uses boundary flags (`HasLeadingNewline`/`HasTrailingNewline`) and does not scan trivia. + +------------------------------------------------------------------------ + +## Phase 1b — SyntaxEditor Flag Mutation Tests (Green Flags) + +Goal: ensure green-node flags remain correct after `SyntaxEditor` mutations and undo/redo. + +- [ ] Add SyntaxEditor tests that assert *green* flag values after mutations + - [ ] Assert boundary flags (token-centric ownership) + - [ ] `Replace(...)` preserves `HasLeadingNewline` on the replaced token when leading newline trivia is preserved + - [ ] `InsertAfter(...)` with a trailing newline causes the next sibling to match newline semantics via previous sibling `HasTrailingNewline` + - [ ] `Remove(...)` of a token that owns trailing newline removes the `HasTrailingNewline` boundary from the tree + - [ ] Assert subtree "contains" flags remain correct + - [ ] `ContainsNewline` propagates correctly through blocks/lists after insert/replace/remove + - [ ] Undo/Redo restores flag state + - [ ] After `Commit()`, `Undo()` restores the original green flags + - [ ] After `Undo()`, `Redo()` restores the mutated green flags + - [ ] Prefer stable selection + direct assertions + - [ ] Select the target node via `Query`/positions, then assert `node.Green` flags (`GreenNodeFlags`) + - [ ] (Optional) also assert `Query.Newline` results as a behavioral cross-check + +Acceptance criteria: +- Tests directly assert green flag bits (not just query behavior) for the above scenarios. ------------------------------------------------------------------------ @@ -121,6 +149,7 @@ Acceptance criteria: - [ ] Update docs/comments to explicitly state newline semantics - [ ] Clarify "node after newline" definition (leading trivia OR previous sibling trailing trivia) - [ ] Remove/correct any mention of "newline whitespace tokens" if newline is trivia-only + - [ ] Call out token-centric boundary ownership (containers do not "own" boundary newline flags) - [ ] Ensure docs match actual implementation Suggested doc touchpoints: @@ -145,7 +174,7 @@ Acceptance criteria: - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxEditorBenchmarks*` - [ ] (Optional) Add a targeted benchmark if needed - - [ ] Benchmark `SyntaxTree.Select(Query.Newline)` + - [x] Benchmark `SyntaxTree.Select(Query.Newline)` - [ ] Benchmark `SelectModes.Last` on a newline-heavy query Acceptance criteria: From 9f15304619b5f35871378a4195d033f72a8300ac Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 00:24:52 -0800 Subject: [PATCH 13/30] [phase 1b]: SyntaxEditor Flag Mutation Tests --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 112 +++++++++++++++++++++++ newline-query-optimization.todo | 24 ++--- 2 files changed, 124 insertions(+), 12 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index d800955..182a107 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1614,6 +1614,118 @@ public void InsertAfter_InsertedTextWithTrailingNewline_PreservesFollowingTrivia Assert.Equal(2, bAfter.LeadingTriviaWidth); // b retains its " " leading trivia } + #region Green Flag Mutation Tests + + private static void AssertHasFlags(GreenNodeFlags actual, GreenNodeFlags expected) + { + Assert.True((actual & expected) == expected, $"Expected flags to include {expected} but was {actual}"); + } + + private static void AssertNotHasFlags(GreenNodeFlags actual, GreenNodeFlags unexpected) + { + Assert.True((actual & unexpected) == 0, $"Expected flags to NOT include {unexpected} but was {actual}"); + } + + [Fact] + public void Replace_OwnLineCommentLeadingTrivia_PreservesGreenBoundaryFlags_OnReplacement() + { + var options = TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine); + var tree = SyntaxTree.Parse("a\n// c\nb", options); + + var bBefore = Assert.Single(tree.Select(Q.Ident("b")).OfType()); + AssertHasFlags(bBefore.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia | GreenNodeFlags.HasLeadingCommentTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("b"), "X") + .Commit(); + + var xAfter = Assert.Single(tree.Select(Q.Ident("X")).OfType()); + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia | GreenNodeFlags.HasLeadingCommentTrivia); + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.ContainsNewlineTrivia | GreenNodeFlags.ContainsCommentTrivia); + } + + [Fact] + public void InsertAfter_InsertedTextWithTrailingNewline_SetsGreenFlags_OnInsertedAndFollowingTokens() + { + var tree = SyntaxTree.Parse("a b"); + var aNode = Assert.Single(tree.Select(Q.Ident("a")).OfType()); + + tree.CreateEditor() + .InsertAfter(aNode, " X\n") + .Commit(); + + var xAfter = Assert.Single(tree.Select(Q.Ident("X")).OfType()); + var bAfter = Assert.Single(tree.Select(Q.Ident("b")).OfType()); + + // Inserted token owns the trailing newline; following token should NOT gain leading newline ownership. + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia | GreenNodeFlags.ContainsNewlineTrivia); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia); + + // Root list should reflect subtree contains. + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void Remove_TokenOwningTrailingNewline_RemovesNewlineBoundaryAndContainsFlags() + { + var tree = SyntaxTree.Parse("a\nb"); + var aNode = Assert.Single(tree.Select(Q.Ident("a")).OfType()); + + // Sanity: 'a' owns the trailing newline in the token-centric trivia model. + AssertHasFlags(aNode.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + + tree.CreateEditor() + .Remove(Q.Ident("a")) + .Commit(); + + Assert.Equal("b", tree.ToText()); + var bAfter = Assert.Single(tree.Select(Q.Ident("b")).OfType()); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() + { + var tree = SyntaxTree.Parse("{a}"); + var blockBefore = (SyntaxBlock)Assert.Single(tree.Select(Q.BraceBlock)); + + AssertNotHasFlags(blockBefore.Green.Flags, GreenNodeFlags.ContainsNewlineTrivia); + + var aNode = Assert.Single(tree.Select(Q.Ident("a")).OfType()); + tree.CreateEditor() + .InsertAfter(aNode, "X\n") + .Commit(); + + var blockAfter = (SyntaxBlock)Assert.Single(tree.Select(Q.BraceBlock)); + AssertHasFlags(blockAfter.Green.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void GreenFlags_UndoRedo_RestoreFlagState_AfterMutation() + { + var tree = SyntaxTree.Parse("a b"); + var before = tree.GreenRoot.Flags; + AssertNotHasFlags(before, GreenNodeFlags.ContainsNewlineTrivia); + + var aNode = Assert.Single(tree.Select(Q.Ident("a")).OfType()); + tree.CreateEditor() + .InsertAfter(aNode, "X\n") + .Commit(); + + var after = tree.GreenRoot.Flags; + AssertHasFlags(after, GreenNodeFlags.ContainsNewlineTrivia); + + Assert.True(tree.Undo()); + Assert.Equal(before, tree.GreenRoot.Flags); + + Assert.True(tree.Redo()); + Assert.Equal(after, tree.GreenRoot.Flags); + } + + #endregion + /// /// Tests that block opener trivia follows the correct trivia model: /// - Trailing trivia: up to AND INCLUDING the newline diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 73adcde..eabda5c 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -61,18 +61,18 @@ Acceptance criteria: Goal: ensure green-node flags remain correct after `SyntaxEditor` mutations and undo/redo. -- [ ] Add SyntaxEditor tests that assert *green* flag values after mutations - - [ ] Assert boundary flags (token-centric ownership) - - [ ] `Replace(...)` preserves `HasLeadingNewline` on the replaced token when leading newline trivia is preserved - - [ ] `InsertAfter(...)` with a trailing newline causes the next sibling to match newline semantics via previous sibling `HasTrailingNewline` - - [ ] `Remove(...)` of a token that owns trailing newline removes the `HasTrailingNewline` boundary from the tree - - [ ] Assert subtree "contains" flags remain correct - - [ ] `ContainsNewline` propagates correctly through blocks/lists after insert/replace/remove - - [ ] Undo/Redo restores flag state - - [ ] After `Commit()`, `Undo()` restores the original green flags - - [ ] After `Undo()`, `Redo()` restores the mutated green flags - - [ ] Prefer stable selection + direct assertions - - [ ] Select the target node via `Query`/positions, then assert `node.Green` flags (`GreenNodeFlags`) +- [x] Add SyntaxEditor tests that assert *green* flag values after mutations + - [x] Assert boundary flags (token-centric ownership) + - [x] `Replace(...)` preserves `HasLeadingNewline` on the replaced token when leading newline trivia is preserved + - [x] `InsertAfter(...)` with a trailing newline causes the next sibling to match newline semantics via previous sibling `HasTrailingNewline` + - [x] `Remove(...)` of a token that owns trailing newline removes the `HasTrailingNewline` boundary from the tree + - [x] Assert subtree "contains" flags remain correct + - [x] `ContainsNewline` propagates correctly through blocks/lists after insert/replace/remove + - [x] Undo/Redo restores flag state + - [x] After `Commit()`, `Undo()` restores the original green flags + - [x] After `Undo()`, `Redo()` restores the mutated green flags + - [x] Prefer stable selection + direct assertions + - [x] Select the target node via `Query`/positions, then assert `node.Green` flags (`GreenNodeFlags`) - [ ] (Optional) also assert `Query.Newline` results as a behavioral cross-check Acceptance criteria: From 1db38910915cc728969147eb78a8ea2741ead2c2 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 00:35:23 -0800 Subject: [PATCH 14/30] [phase 2]: Remove LINQ from Selection Modes --- TinyTokenizer/Ast/NodeQueryTypes.cs | 229 ++++++++++--------- TinyTokenizer/Ast/SemanticMatchExtensions.cs | 22 +- newline-query-optimization.todo | 16 +- 3 files changed, 132 insertions(+), 135 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index ea691bf..6f6cc73 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -7,16 +7,95 @@ namespace TinyTokenizer.Ast; /// internal static class SelectionModeHelper { - public static IEnumerable Apply(IEnumerable regions, SelectionMode mode, int modeArg) => - mode switch + public static IEnumerable Apply(IEnumerable source, SelectionMode mode, int modeArg) + { + return mode switch { - SelectionMode.First => regions.Take(1), - SelectionMode.Last => regions.TakeLast(1), - SelectionMode.Nth => regions.Skip(modeArg).Take(1), - SelectionMode.Skip => regions.Skip(modeArg), - SelectionMode.Take => regions.Take(modeArg), - _ => regions + SelectionMode.First => TakeFirst(source), + SelectionMode.Last => TakeLast(source), + SelectionMode.Nth => TakeNth(source, modeArg), + SelectionMode.Skip => Skip(source, modeArg), + SelectionMode.Take => Take(source, modeArg), + _ => source }; + } + + private static IEnumerable TakeFirst(IEnumerable source) + { + foreach (var item in source) + { + yield return item; + yield break; + } + } + + private static IEnumerable TakeLast(IEnumerable source) + { + T? last = default; + var found = false; + + foreach (var item in source) + { + last = item; + found = true; + } + + if (found) + yield return last!; + } + + private static IEnumerable TakeNth(IEnumerable source, int n) + { + if (n < 0) + yield break; + + var index = 0; + foreach (var item in source) + { + if (index == n) + { + yield return item; + yield break; + } + index++; + } + } + + private static IEnumerable Skip(IEnumerable source, int count) + { + if (count <= 0) + { + foreach (var item in source) + yield return item; + yield break; + } + + var skipped = 0; + foreach (var item in source) + { + if (skipped < count) + { + skipped++; + continue; + } + yield return item; + } + } + + private static IEnumerable Take(IEnumerable source, int count) + { + if (count <= 0) + yield break; + + var taken = 0; + foreach (var item in source) + { + yield return item; + taken++; + if (taken >= count) + yield break; + } + } } /// @@ -69,16 +148,8 @@ public override IEnumerable Select(SyntaxNode root) { var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf().Where(Matches); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -191,16 +262,8 @@ public override IEnumerable Select(SyntaxNode root) { var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf().Where(Matches); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -303,9 +366,6 @@ private IEnumerable SelectRegionsFromWalker(PathTrackingWalker walk /// /// // Replace content between braces /// editor.Replace(Query.BraceBlock.Inner(), "new content") - /// - /// // Works with empty blocks too - /// editor.Replace(Query.BraceBlock.Inner(), "inserted into empty") /// /// public InnerContentQuery Inner() => new InnerContentQuery(this); @@ -327,12 +387,9 @@ public enum BoundarySide } /// -/// A query that selects the boundary (start or end) of container nodes. -/// For blocks, this returns the opener or closer token. -/// For lists/containers without delimiters, this returns first/last child (or empty for empty containers). +/// A query that selects the boundary node (start or end) of containers matched by an inner query. /// /// -/// This query carries metadata about which container and boundary is being targeted. /// uses this metadata to compute insertion positions, /// even for empty containers where returns no results. /// @@ -532,7 +589,12 @@ private IEnumerable SelectRegionsCore(SyntaxNode root) if (container is SyntaxBlock block) { var innerCount = block.ChildCount; - var firstInner = block.InnerChildren.FirstOrDefault(); + SyntaxNode? firstInner = null; + foreach (var child in block.InnerChildren) + { + firstInner = child; + break; + } yield return new QueryRegion( parent: block, @@ -578,15 +640,7 @@ public override IEnumerable Select(SyntaxNode root) if (_predicate != null) matches = matches.Where(_predicate); - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -641,15 +695,7 @@ public override IEnumerable Select(SyntaxNode root) var walker = new TreeWalker(root, NodeFilter.Leaves); var matches = walker.DescendantsAndSelf().Where(Matches); - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -719,15 +765,7 @@ public override IEnumerable Select(SyntaxNode root) var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf().Where(Matches); - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -1156,9 +1194,11 @@ public sealed record BeginningOfFileQuery : INodeQuery, IGreenNodeQuery public IEnumerable Select(SyntaxNode root) { // BOF only matches the first node in the file - var firstChild = root.Children.FirstOrDefault(); - if (firstChild != null) - yield return firstChild; + foreach (var child in root.Children) + { + yield return child; + yield break; + } } /// @@ -1214,9 +1254,14 @@ public sealed record EndOfFileQuery : INodeQuery, IGreenNodeQuery public IEnumerable Select(SyntaxNode root) { // EOF only matches the last node in the file - var lastChild = root.Children.LastOrDefault(); - if (lastChild != null) - yield return lastChild; + SyntaxNode? last = null; + foreach (var child in root.Children) + { + last = child; + } + + if (last != null) + yield return last; } /// @@ -1563,15 +1608,7 @@ public override IEnumerable Select(SyntaxNode root) var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf().Where(Matches); - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -1679,16 +1716,8 @@ public override IEnumerable Select(SyntaxNode root) var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf() .Where(n => n.Kind == targetKind && (_predicate == null || _predicate(n))); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -1837,16 +1866,8 @@ public override IEnumerable Select(SyntaxTree tree) var walker = new TreeWalker(tree.Root); var matches = walker.DescendantsAndSelf() .Where(n => kindSet.Contains(n.Kind) && (_predicate == null || _predicate(n))); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// @@ -1856,16 +1877,8 @@ public override IEnumerable Select(SyntaxNode root) var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf() .Where(n => n.Kind.IsKeyword() && (_predicate == null || _predicate(n))); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// diff --git a/TinyTokenizer/Ast/SemanticMatchExtensions.cs b/TinyTokenizer/Ast/SemanticMatchExtensions.cs index d28b1c4..9d48dfe 100644 --- a/TinyTokenizer/Ast/SemanticMatchExtensions.cs +++ b/TinyTokenizer/Ast/SemanticMatchExtensions.cs @@ -137,16 +137,8 @@ public override IEnumerable Select(SyntaxNode root) { var walker = new TreeWalker(root); var matches = walker.DescendantsAndSelf().Where(Matches); - - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } public override bool Matches(SyntaxNode node) => @@ -225,15 +217,7 @@ public override IEnumerable Select(SyntaxNode root) private IEnumerable ApplyMode(IEnumerable matches) { - return _mode switch - { - SelectionMode.First => matches.Take(1), - SelectionMode.Last => matches.TakeLast(1), - SelectionMode.Nth => matches.Skip(_modeArg).Take(1), - SelectionMode.Skip => matches.Skip(_modeArg), - SelectionMode.Take => matches.Take(_modeArg), - _ => matches - }; + return SelectionModeHelper.Apply(matches, _mode, _modeArg); } /// diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index eabda5c..11506a0 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -82,14 +82,14 @@ Acceptance criteria: ## Phase 2 — Remove LINQ from Selection Modes (Start with `SelectModes.Last`) -- [ ] Rewrite selection-mode handling without LINQ - - [ ] Replace `LastOrDefault()` usage with a simple scan that tracks the last match - - [ ] Ensure ordering/behavior matches existing semantics - - [ ] Avoid buffering unless semantics strictly require it - -- [ ] Expand to other modes only if they are on the hot path - - [ ] Audit remaining modes for LINQ/buffering - - [ ] Rewrite only those that show up in profiles/benchmarks +- [x] Rewrite selection-mode handling without LINQ + - [x] Replace `LastOrDefault()` usage with a simple scan that tracks the last match + - [x] Ensure ordering/behavior matches existing semantics + - [x] Avoid buffering unless semantics strictly require it + +- [x] Expand to other modes only if they are on the hot path + - [x] Audit remaining modes for LINQ/buffering + - [x] Rewrite only those that show up in profiles/benchmarks Acceptance criteria: - `SelectModes.Last` returns the same result as before, with fewer allocations. From eaa1e627caf6e297f75207c41193c82734f646b6 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 00:45:03 -0800 Subject: [PATCH 15/30] [phase 3]: Replace LINQ in the Hottest `Select(...)` Implementations --- TinyTokenizer/Ast/NodeQueryTypes.cs | 134 ++++++++++++++----- TinyTokenizer/Ast/SemanticMatchExtensions.cs | 66 +++++++-- newline-query-optimization.todo | 16 +-- 3 files changed, 164 insertions(+), 52 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index 6f6cc73..394b5e6 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -146,10 +146,17 @@ private KindNodeQuery(NodeKind kind, Func? predicate, Selectio /// public override IEnumerable Select(SyntaxNode root) { - var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + private IEnumerable EnumerateMatches(SyntaxNode root) + { + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind == Kind && (_predicate == null || _predicate(node))) + yield return node; + } } /// @@ -259,11 +266,26 @@ private protected BlockNodeQuery(char? opener, Func? predicate /// public override IEnumerable Select(SyntaxNode root) + { + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } + + private IEnumerable EnumerateMatches(SyntaxNode root) { var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node is not SyntaxBlock block) + continue; - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + if (_opener != null && block.Opener != _opener.Value) + continue; + + if (_predicate != null && !_predicate(node)) + continue; + + yield return node; + } } /// @@ -636,11 +658,20 @@ private AnyNodeQuery(Func? predicate, SelectionMode mode, int /// public override IEnumerable Select(SyntaxNode root) { - var matches = new TreeWalker(root).DescendantsAndSelf(); - if (_predicate != null) - matches = matches.Where(_predicate); - - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + if (_predicate == null) + return SelectionModeHelper.Apply(new TreeWalker(root).DescendantsAndSelf(), _mode, _modeArg); + + return SelectionModeHelper.Apply(EnumerateMatches(root, _predicate), _mode, _modeArg); + } + + private static IEnumerable EnumerateMatches(SyntaxNode root, Func predicate) + { + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (predicate(node)) + yield return node; + } } /// @@ -691,11 +722,18 @@ private LeafNodeQuery(Func? predicate, SelectionMode mode, int /// public override IEnumerable Select(SyntaxNode root) + { + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } + + private IEnumerable EnumerateMatches(SyntaxNode root) { var walker = new TreeWalker(root, NodeFilter.Leaves); - var matches = walker.DescendantsAndSelf().Where(Matches); - - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node is SyntaxToken && (_predicate == null || _predicate(node))) + yield return node; + } } /// @@ -761,11 +799,18 @@ private NewlineNodeQuery(Func? predicate, SelectionMode mode, /// public override IEnumerable Select(SyntaxNode root) + { + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } + + private IEnumerable EnumerateMatches(SyntaxNode root) { var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); - - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + foreach (var node in walker.DescendantsAndSelf()) + { + if (Matches(node)) + yield return node; + } } /// @@ -1604,11 +1649,18 @@ private AnyKeywordQuery(Func? predicate, SelectionMode mode, i /// public override IEnumerable Select(SyntaxNode root) + { + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } + + private IEnumerable EnumerateMatches(SyntaxNode root) { var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); - - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind.IsKeyword() && (_predicate == null || _predicate(node))) + yield return node; + } } /// @@ -1712,12 +1764,18 @@ public override IEnumerable Select(SyntaxNode root) if (!_isResolved || _resolvedKind == null) return []; - var targetKind = _resolvedKind.Value; - var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf() - .Where(n => n.Kind == targetKind && (_predicate == null || _predicate(n))); + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + private IEnumerable EnumerateMatches(SyntaxNode root) + { + var targetKind = _resolvedKind!.Value; + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind == targetKind && (_predicate == null || _predicate(node))) + yield return node; + } } /// @@ -1863,22 +1921,34 @@ public override IEnumerable Select(SyntaxTree tree) return []; var kindSet = categoryKinds.ToHashSet(); - var walker = new TreeWalker(tree.Root); - var matches = walker.DescendantsAndSelf() - .Where(n => kindSet.Contains(n.Kind) && (_predicate == null || _predicate(n))); + return SelectionModeHelper.Apply(EnumerateMatchesByKindSet(tree.Root, kindSet), _mode, _modeArg); + } - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + private IEnumerable EnumerateMatchesByKindSet(SyntaxNode root, HashSet kindSet) + { + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (kindSet.Contains(node.Kind) && (_predicate == null || _predicate(node))) + yield return node; + } } /// public override IEnumerable Select(SyntaxNode root) { // Without tree context, we can't resolve category - match any keyword - var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf() - .Where(n => n.Kind.IsKeyword() && (_predicate == null || _predicate(n))); + return SelectionModeHelper.Apply(EnumerateMatchesAnyKeyword(root), _mode, _modeArg); + } - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + private IEnumerable EnumerateMatchesAnyKeyword(SyntaxNode root) + { + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind.IsKeyword() && (_predicate == null || _predicate(node))) + yield return node; + } } /// diff --git a/TinyTokenizer/Ast/SemanticMatchExtensions.cs b/TinyTokenizer/Ast/SemanticMatchExtensions.cs index 9d48dfe..21e3b0b 100644 --- a/TinyTokenizer/Ast/SemanticMatchExtensions.cs +++ b/TinyTokenizer/Ast/SemanticMatchExtensions.cs @@ -135,10 +135,17 @@ private SyntaxNodeQuery(NodeKind kind, Func? predicate, Select public override IEnumerable Select(SyntaxNode root) { - var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); + return SelectionModeHelper.Apply(EnumerateMatches(root), _mode, _modeArg); + } - return SelectionModeHelper.Apply(matches, _mode, _modeArg); + private IEnumerable EnumerateMatches(SyntaxNode root) + { + var walker = new TreeWalker(root); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind == _kind && (_predicate == null || _predicate(node))) + yield return node; + } } public override bool Matches(SyntaxNode node) => @@ -198,21 +205,42 @@ public override IEnumerable Select(SyntaxTree tree) } private IEnumerable SelectWithKind(SyntaxNode root, NodeKind kind) + { + return ApplyMode(EnumerateMatchesByKind(root, kind)); + } + + private IEnumerable EnumerateMatchesByKind(SyntaxNode root, NodeKind kind) { var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf() - .Where(n => n.Kind == kind && (_predicate == null || _predicate((T)n))); - - return ApplyMode(matches); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node.Kind != kind) + continue; + + if (node is not T typed) + continue; + + if (_predicate != null && !_predicate(typed)) + continue; + + yield return node; + } } public override IEnumerable Select(SyntaxNode root) { // Match by C# type (no schema available) + return ApplyMode(EnumerateMatchesByType(root)); + } + + private IEnumerable EnumerateMatchesByType(SyntaxNode root) + { var walker = new TreeWalker(root); - var matches = walker.DescendantsAndSelf().Where(Matches); - - return ApplyMode(matches); + foreach (var node in walker.DescendantsAndSelf()) + { + if (node is T typed && (_predicate == null || _predicate(typed))) + yield return node; + } } private IEnumerable ApplyMode(IEnumerable matches) @@ -223,12 +251,26 @@ private IEnumerable ApplyMode(IEnumerable matches) /// /// Selects and casts to the strongly-typed syntax node. /// - public IEnumerable SelectTyped(SyntaxTree tree) => Select(tree).Cast(); + public IEnumerable SelectTyped(SyntaxTree tree) + { + foreach (var node in Select(tree)) + { + if (node is T typed) + yield return typed; + } + } /// /// Selects and casts to the strongly-typed syntax node. /// - public IEnumerable SelectTyped(SyntaxNode root) => Select(root).Cast(); + public IEnumerable SelectTyped(SyntaxNode root) + { + foreach (var node in Select(root)) + { + if (node is T typed) + yield return typed; + } + } public override bool Matches(SyntaxNode node) => node is T typed && (_predicate == null || _predicate(typed)); diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 11506a0..48d3fb9 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -98,14 +98,14 @@ Acceptance criteria: ## Phase 3 — Replace LINQ in the Hottest `Select(...)` Implementations (Small Scope) -- [ ] Identify top `Select` hot paths - - [ ] Common kind queries (e.g., `KindNodeQuery.Select(...)`) - - [ ] Block queries and leaf queries used heavily by editor/regions - -- [ ] Replace common LINQ patterns with tight loops - - [ ] Replace `.Where(...)`, `.SelectMany(...)`, `.LastOrDefault()` in hot paths - - [ ] Preserve document order and short-circuiting behavior - - [ ] Avoid iterator/closure allocations where possible +- [x] Identify top `Select` hot paths + - [x] Common kind queries (e.g., `KindNodeQuery.Select(...)`) + - [x] Block queries and leaf queries used heavily by editor/regions + +- [x] Replace common LINQ patterns with tight loops + - [x] Replace `.Where(...)`, `.SelectMany(...)`, `.LastOrDefault()` in hot paths + - [x] Preserve document order and short-circuiting behavior + - [x] Avoid iterator/closure allocations where possible Acceptance criteria: - Query results are byte-for-byte identical in ordering and content; allocations reduced in benchmarks. From cc8921bb0427d01055ec7da5c9db7f059d5eef65 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 00:52:20 -0800 Subject: [PATCH 16/30] [phase 4]: Replace LINQ in the Hottest Combinators --- TinyTokenizer/Ast/NodeQueryTypes.cs | 82 +++++++++++++++++++++++++-- TinyTokenizer/Ast/QueryCombinators.cs | 14 ++++- newline-query-optimization.todo | 16 +++--- 3 files changed, 99 insertions(+), 13 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index 394b5e6..6054fe8 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -1032,10 +1032,26 @@ public sealed record AnyOfQuery : INodeQuery, IGreenNodeQuery, ISchemaResolvable public AnyOfQuery(params INodeQuery[] queries) => _queries = queries; /// Creates a query that matches any of the specified queries. - public AnyOfQuery(IEnumerable queries) => _queries = queries.ToArray(); + public AnyOfQuery(IEnumerable queries) + { + ArgumentNullException.ThrowIfNull(queries); + _queries = MaterializeQueries(queries); + } /// - public bool IsResolved => _queries.All(q => q is not ISchemaResolvableQuery r || r.IsResolved); + public bool IsResolved + { + get + { + foreach (var query in _queries) + { + if (query is ISchemaResolvableQuery r && !r.IsResolved) + return false; + } + + return true; + } + } /// public void ResolveWithSchema(Schema schema) @@ -1112,6 +1128,27 @@ bool IGreenNodeQuery.TryMatchGreen(IReadOnlyList siblings, int startI consumedCount = 0; return false; } + + private static INodeQuery[] MaterializeQueries(IEnumerable queries) + { + if (queries is INodeQuery[] arr) + return arr; + + if (queries is ICollection col) + { + if (col.Count == 0) + return []; + + var buffer = new INodeQuery[col.Count]; + col.CopyTo(buffer, 0); + return buffer; + } + + var list = new List(); + foreach (var q in queries) + list.Add(q); + return list.ToArray(); + } } #endregion @@ -1130,10 +1167,26 @@ public sealed record NoneOfQuery : INodeQuery, IGreenNodeQuery, ISchemaResolvabl public NoneOfQuery(params INodeQuery[] queries) => _queries = queries; /// Creates a query that matches when none of the specified queries match. - public NoneOfQuery(IEnumerable queries) => _queries = queries.ToArray(); + public NoneOfQuery(IEnumerable queries) + { + ArgumentNullException.ThrowIfNull(queries); + _queries = MaterializeQueries(queries); + } /// - public bool IsResolved => _queries.All(q => q is not ISchemaResolvableQuery r || r.IsResolved); + public bool IsResolved + { + get + { + foreach (var query in _queries) + { + if (query is ISchemaResolvableQuery r && !r.IsResolved) + return false; + } + + return true; + } + } /// public void ResolveWithSchema(Schema schema) @@ -1220,6 +1273,27 @@ bool IGreenNodeQuery.TryMatchGreen(IReadOnlyList siblings, int startI consumedCount = 1; return true; } + + private static INodeQuery[] MaterializeQueries(IEnumerable queries) + { + if (queries is INodeQuery[] arr) + return arr; + + if (queries is ICollection col) + { + if (col.Count == 0) + return []; + + var buffer = new INodeQuery[col.Count]; + col.CopyTo(buffer, 0); + return buffer; + } + + var list = new List(); + foreach (var q in queries) + list.Add(q); + return list.ToArray(); + } } #endregion diff --git a/TinyTokenizer/Ast/QueryCombinators.cs b/TinyTokenizer/Ast/QueryCombinators.cs index 6d2373d..354f4a5 100644 --- a/TinyTokenizer/Ast/QueryCombinators.cs +++ b/TinyTokenizer/Ast/QueryCombinators.cs @@ -270,7 +270,19 @@ public SequenceQuery(params INodeQuery[] parts) } /// - public bool IsResolved => _parts.All(p => p is not ISchemaResolvableQuery r || r.IsResolved); + public bool IsResolved + { + get + { + foreach (var part in _parts) + { + if (part is ISchemaResolvableQuery r && !r.IsResolved) + return false; + } + + return true; + } + } /// public void ResolveWithSchema(Schema schema) diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 48d3fb9..8b1f993 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -114,14 +114,14 @@ Acceptance criteria: ## Phase 4 — Replace LINQ in the Hottest Combinators (Small Scope) -- [ ] Remove LINQ where it forces buffering/extra iterators - - [ ] OR / AnyOf-style combinators - - [ ] AND / sequence-style combinators - - [ ] Any other combinator used by newline queries or editor/region resolution - -- [ ] Preserve semantics and ordering - - [ ] Document order remains stable - - [ ] No duplicate matches unless already part of the semantics +- [x] Remove LINQ where it forces buffering/extra iterators + - [x] OR / AnyOf-style combinators + - [x] AND / sequence-style combinators + - [x] Any other combinator used by newline queries or editor/region resolution + +- [x] Preserve semantics and ordering + - [x] Document order remains stable + - [x] No duplicate matches unless already part of the semantics Acceptance criteria: - Combinator behavior remains unchanged; perf improves in existing suites. From 46d0d148d8e90d6a8f4a3d5c049517b3c0f23861 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 01:14:51 -0800 Subject: [PATCH 17/30] [phase 5]: Optimize Region Resolution --- TinyTokenizer/Ast/NodeQuery.cs | 26 +---- TinyTokenizer/Ast/NodeQueryTypes.cs | 60 +++------- TinyTokenizer/Ast/QueryCombinators.cs | 132 ++++------------------ TinyTokenizer/Ast/QueryRegion.cs | 156 ++++++++++++-------------- newline-query-optimization.todo | 16 +-- 5 files changed, 124 insertions(+), 266 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQuery.cs b/TinyTokenizer/Ast/NodeQuery.cs index ac62315..97d2f17 100644 --- a/TinyTokenizer/Ast/NodeQuery.cs +++ b/TinyTokenizer/Ast/NodeQuery.cs @@ -124,9 +124,9 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) => SelectRegionsCore(root); /// - /// Default region resolution: traverses tree with PathTrackingWalker, calls TryMatch once per node, + /// Default region resolution: traverses tree with a stack-based walker, calls TryMatch once per node, /// then applies selection filtering (First/Last/Nth via ApplyRegionFilter). - /// Uses incremental path tracking for O(1) per node instead of O(depth). + /// Snapshots a NodePath only when a match is found. /// internal virtual IEnumerable SelectRegionsCore(SyntaxNode root) { @@ -135,29 +135,11 @@ internal virtual IEnumerable SelectRegionsCore(SyntaxNode root) /// /// Traverses tree and yields a region for each matching node. - /// Uses PathTrackingWalker for O(1) path computation per node. + /// Uses RegionTraversal to avoid per-visited-node allocations. /// private IEnumerable SelectAllRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) - { - if (TryMatch(node, out var consumedCount)) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } - } + return RegionTraversal.SelectRegions(root, TryMatch); } /// diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index 6054fe8..a43a049 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -197,35 +197,23 @@ protected override KindNodeQuery CreateFiltered(Func predicate /// /// Optimized region resolution: single-pass traversal that checks Kind directly /// and applies selection mode inline for efficient First()/Take() short-circuit. - /// Uses PathTrackingWalker for O(1) path computation per node. + /// Uses RegionTraversal to avoid per-visited-node allocations. /// internal override IEnumerable SelectRegionsCore(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - var regions = SelectRegionsFromWalker(walker); + var regions = RegionTraversal.SelectRegions(root, TryGetRegion); return ApplyRegionFilter(regions); - } - - private IEnumerable SelectRegionsFromWalker(PathTrackingWalker walker) - { - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) + + bool TryGetRegion(SyntaxNode node, out int consumedCount) { - // Inline match check - avoids virtual TryMatch call if (node.Kind == Kind && (_predicate == null || _predicate(node))) { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + 1, // KindNodeQuery always consumes 1 - firstNode: node, - position: node.Position - ); - } + consumedCount = 1; + return true; } + + consumedCount = 0; + return false; } } @@ -318,37 +306,25 @@ protected override BlockNodeQuery CreateFiltered(Func predicat /// /// Optimized region resolution: single-pass traversal that checks block type directly /// and applies selection mode inline for efficient First()/Take() short-circuit. - /// Uses PathTrackingWalker for O(1) path computation per node. + /// Uses RegionTraversal to avoid per-visited-node allocations. /// internal override IEnumerable SelectRegionsCore(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - var regions = SelectRegionsFromWalker(walker); + var regions = RegionTraversal.SelectRegions(root, TryGetRegion); return ApplyRegionFilter(regions); - } - - private IEnumerable SelectRegionsFromWalker(PathTrackingWalker walker) - { - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) + + bool TryGetRegion(SyntaxNode node, out int consumedCount) { - // Inline match check - avoids virtual TryMatch call if (node is SyntaxBlock block && (_opener == null || block.Opener == _opener.Value) && (_predicate == null || _predicate(node))) { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + 1, // BlockNodeQuery always consumes 1 - firstNode: node, - position: node.Position - ); - } + consumedCount = 1; + return true; } + + consumedCount = 0; + return false; } } diff --git a/TinyTokenizer/Ast/QueryCombinators.cs b/TinyTokenizer/Ast/QueryCombinators.cs index 354f4a5..6744a35 100644 --- a/TinyTokenizer/Ast/QueryCombinators.cs +++ b/TinyTokenizer/Ast/QueryCombinators.cs @@ -215,25 +215,7 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) - { - if (TryMatch(node, out var consumedCount)) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } - } + return RegionTraversal.SelectRegions(root, TryMatch); } } @@ -393,25 +375,7 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) - { - if (TryMatch(node, out var consumedCount)) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } - } + return RegionTraversal.SelectRegions(root, TryMatch); } } @@ -478,25 +442,16 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - // Optional delegates to inner query - traverse and match inner directly - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) + // Optional delegates to inner query - only yield when inner consumes > 0 + return RegionTraversal.SelectRegions(root, TryGetInnerRegion); + + bool TryGetInnerRegion(SyntaxNode node, out int consumedCount) { - if (_inner.TryMatch(node, out var consumedCount) && consumedCount > 0) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } + if (_inner.TryMatch(node, out consumedCount) && consumedCount > 0) + return true; + + consumedCount = 0; + return false; } } } @@ -625,24 +580,15 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) + return RegionTraversal.SelectRegions(root, TryGetNonEmptyRegion); + + bool TryGetNonEmptyRegion(SyntaxNode node, out int consumedCount) { - if (TryMatch(node, out var consumedCount) && consumedCount > 0) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } + if (TryMatch(node, out consumedCount) && consumedCount > 0) + return true; + + consumedCount = 0; + return false; } } } @@ -846,25 +792,7 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) - { - if (TryMatch(node, out var consumedCount)) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } - } + return RegionTraversal.SelectRegions(root, TryMatch); } } @@ -1002,25 +930,7 @@ IEnumerable IRegionQuery.SelectRegions(SyntaxTree tree) /// IEnumerable IRegionQuery.SelectRegions(SyntaxNode root) { - var walker = new PathTrackingWalker(root); - foreach (var (node, parentPath) in walker.DescendantsAndSelfWithPath()) - { - if (TryMatch(node, out var consumedCount)) - { - var parent = node.Parent; - if (parent != null) - { - yield return new QueryRegion( - parentPath: parentPath, - parent: parent, - startSlot: node.SiblingIndex, - endSlot: node.SiblingIndex + consumedCount, - firstNode: node, - position: node.Position - ); - } - } - } + return RegionTraversal.SelectRegions(root, TryMatch); } } diff --git a/TinyTokenizer/Ast/QueryRegion.cs b/TinyTokenizer/Ast/QueryRegion.cs index 63c7ad1..616b2d2 100644 --- a/TinyTokenizer/Ast/QueryRegion.cs +++ b/TinyTokenizer/Ast/QueryRegion.cs @@ -133,113 +133,103 @@ internal interface IRegionQuery } /// -/// A tree walker that incrementally tracks the path during traversal. -/// O(1) per traversal step instead of O(depth) for NodePath.FromNode(). +/// Low-allocation traversal helper for region resolution. +/// Maintains an incremental slot-index stack while walking the tree and only snapshots +/// a when a match is found. /// -internal sealed class PathTrackingWalker +internal static class RegionTraversal { - private readonly SyntaxNode _root; - private readonly List _pathStack; - private SyntaxNode _current; - - public PathTrackingWalker(SyntaxNode root) - { - _root = root; - _current = root; - _pathStack = new List(8); // Pre-allocate for typical tree depth - } - - /// The current node. - public SyntaxNode Current => _current; - - /// - /// Gets the current path as a NodePath. - /// The path leads to the CURRENT node (not its parent). - /// - public NodePath CurrentPath => new NodePath(ImmutableArray.CreateRange(_pathStack)); - - /// - /// Gets the path to the parent of the current node. - /// Returns Root path if current is the root. - /// - public NodePath ParentPath + internal delegate bool TryGetRegionDelegate(SyntaxNode node, out int consumedCount); + + internal static IEnumerable SelectRegions(SyntaxNode root, TryGetRegionDelegate tryGetRegion) { - get + ArgumentNullException.ThrowIfNull(tryGetRegion); + + var pathStack = new List(8); + var current = root; + + while (true) { - if (_pathStack.Count == 0) - return NodePath.Root; - - // Return path without the last index (current node's sibling index) - return new NodePath(ImmutableArray.CreateRange(_pathStack.Take(_pathStack.Count - 1))); + if (current.Parent != null && tryGetRegion(current, out var consumedCount)) + { + var parent = current.Parent; + var startSlot = current.SiblingIndex; + yield return new QueryRegion( + parentPath: CreateParentPath(pathStack), + parent: parent, + startSlot: startSlot, + endSlot: startSlot + consumedCount, + firstNode: current, + position: current.Position + ); + } + + if (TryMoveToFirstChild(ref current, pathStack)) + continue; + + if (!TryMoveToNextSiblingOrAncestor(ref current, pathStack)) + break; } } - - /// - /// Enumerates all descendants of the root in document order, - /// yielding each node along with its parent path. - /// - public IEnumerable<(SyntaxNode Node, NodePath ParentPath)> DescendantsAndSelfWithPath() + + private static NodePath CreateParentPath(List pathStack) { - // Yield root first - yield return (_root, NodePath.Root); - - // Reset state for traversal - _current = _root; - _pathStack.Clear(); - - while (MoveNext()) - { - yield return (_current, ParentPath); - } + // pathStack is the path to the CURRENT node; parent path is pathStack without the last element. + var parentDepth = pathStack.Count - 1; + if (parentDepth <= 0) + return NodePath.Root; + + var builder = ImmutableArray.CreateBuilder(parentDepth); + for (int i = 0; i < parentDepth; i++) + builder.Add(pathStack[i]); + + return new NodePath(builder.ToImmutable()); } - - /// - /// Moves to the next node in document order (depth-first pre-order). - /// Returns true if moved, false if at end. - /// - private bool MoveNext() + + private static bool TryMoveToFirstChild(ref SyntaxNode current, List pathStack) { - // Try first child - if (_current.SlotCount > 0) + if (current.SlotCount == 0) + return false; + + for (int i = 0; i < current.SlotCount; i++) { - for (int i = 0; i < _current.SlotCount; i++) + var child = current.GetChild(i); + if (child != null) { - var child = _current.GetChild(i); - if (child != null) - { - _pathStack.Add(i); - _current = child; - return true; - } + pathStack.Add(i); + current = child; + return true; } } - - // Try next sibling or ancestor's next sibling - while (_pathStack.Count > 0) + + return false; + } + + private static bool TryMoveToNextSiblingOrAncestor(ref SyntaxNode current, List pathStack) + { + while (pathStack.Count > 0) { - var parent = _current.Parent; + var parent = current.Parent; if (parent == null) - break; - - var currentIndex = _pathStack[_pathStack.Count - 1]; - _pathStack.RemoveAt(_pathStack.Count - 1); - - // Try next sibling + return false; + + var currentIndex = pathStack[^1]; + pathStack.RemoveAt(pathStack.Count - 1); + for (int i = currentIndex + 1; i < parent.SlotCount; i++) { var sibling = parent.GetChild(i); if (sibling != null) { - _pathStack.Add(i); - _current = sibling; + pathStack.Add(i); + current = sibling; return true; } } - - // Move up to try parent's siblings - _current = parent; + + current = parent; } - + return false; } } diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 8b1f993..7da4feb 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -130,14 +130,14 @@ Acceptance criteria: ## Phase 5 — Optimize Region Resolution (Materialize-on-Match) -- [ ] Refactor region traversal to avoid per-visited-node allocations - - [ ] Maintain an incremental slot-index stack while walking the tree - - [ ] Construct `NodePath` only when a match is found (snapshot stack) - - [ ] Avoid `NodePath.FromNode(...)` and repeated `.ToArray()` allocations in scans - -- [ ] Validate correctness - - [ ] Query regions (`IRegionQuery` + match-based fallback) return identical regions - - [ ] Editor operations that depend on regions remain stable +- [x] Refactor region traversal to avoid per-visited-node allocations + - [x] Maintain an incremental slot-index stack while walking the tree + - [x] Construct `NodePath` only when a match is found (snapshot stack) + - [x] Avoid `NodePath.FromNode(...)` and repeated `.ToArray()` allocations in scans + +- [x] Validate correctness + - [x] Query regions (`IRegionQuery` + match-based fallback) return identical regions + - [x] Editor operations that depend on regions remain stable Acceptance criteria: - Region-heavy operations allocate less and remain semantically identical. From dd562eaad5460201297675a677732d038be918d9 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 01:23:10 -0800 Subject: [PATCH 18/30] [phase 6]: Documentation Sanity --- TinyTokenizer/Ast/NodeQueryTypes.cs | 11 +++++++---- TinyTokenizer/Ast/Query.cs | 8 ++++++-- newline-query-optimization.todo | 14 +++++++------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/TinyTokenizer/Ast/NodeQueryTypes.cs b/TinyTokenizer/Ast/NodeQueryTypes.cs index a43a049..de15079 100644 --- a/TinyTokenizer/Ast/NodeQueryTypes.cs +++ b/TinyTokenizer/Ast/NodeQueryTypes.cs @@ -739,12 +739,15 @@ protected override LeafNodeQuery CreateFiltered(Func predicate #region Newline Query /// -/// Matches nodes that represent or are preceded by a newline. -/// Checks: -/// 1. The node's leading boundary trivia contains a newline. -/// 2. The previous sibling's trailing boundary trivia contains a newline. +/// Matches nodes that occur after a newline. +/// A node matches when either: +/// 1) The node owns leading newline trivia, OR +/// 2) The previous sibling owns trailing newline trivia. /// /// +/// Newline detection is token-centric: the newline boundary is owned by a token's +/// leading/trailing trivia, not by container nodes. +/// /// This query is particularly useful for line-based pattern matching, such as /// matching directive lines that should consume tokens until a newline. /// diff --git a/TinyTokenizer/Ast/Query.cs b/TinyTokenizer/Ast/Query.cs index 8b430f1..56e88e3 100644 --- a/TinyTokenizer/Ast/Query.cs +++ b/TinyTokenizer/Ast/Query.cs @@ -104,13 +104,17 @@ public static class Query public static LeafNodeQuery Leaf => new LeafNodeQuery(); /// - /// Matches nodes that are preceded by a newline (in trivia or as whitespace token). + /// Matches nodes that occur after a newline. + /// A node matches when either: + /// - The node owns leading newline trivia, OR + /// - The previous sibling owns trailing newline trivia. /// Useful for line-based pattern matching. /// public static NewlineNodeQuery Newline => new NewlineNodeQuery(); /// - /// Matches nodes that are NOT preceded by a newline. + /// Matches nodes that do NOT occur after a newline. + /// This is the exact negation of under the same context. /// Useful for matching tokens on the same line. /// public static NewlineNodeQuery NotNewline => new NewlineNodeQuery(negated: true); diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index 7da4feb..ec36872 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -146,15 +146,15 @@ Acceptance criteria: ## Phase 6 — Documentation Sanity -- [ ] Update docs/comments to explicitly state newline semantics - - [ ] Clarify "node after newline" definition (leading trivia OR previous sibling trailing trivia) - - [ ] Remove/correct any mention of "newline whitespace tokens" if newline is trivia-only - - [ ] Call out token-centric boundary ownership (containers do not "own" boundary newline flags) - - [ ] Ensure docs match actual implementation +- [x] Update docs/comments to explicitly state newline semantics + - [x] Clarify "node after newline" definition (leading trivia OR previous sibling trailing trivia) + - [x] Remove/correct any mention of "newline whitespace tokens" if newline is trivia-only + - [x] Call out token-centric boundary ownership (containers do not "own" boundary newline flags) + - [x] Ensure docs match actual implementation Suggested doc touchpoints: -- [ ] TinyTokenizer.wiki/Query-API.md -- [ ] TinyTokenizer.wiki/Trivia.md +- [x] TinyTokenizer.wiki/Query-API.md +- [x] TinyTokenizer.wiki/Trivia.md - [ ] TinyTokenizer.wiki/TreeWalker.md (only if it mentions newline semantics) Acceptance criteria: From 119671a8d917f9d485d5a6a06d233a39262de66a Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:08:32 -0800 Subject: [PATCH 19/30] chore: update task list --- newline-query-optimization.todo | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo index ec36872..829779e 100644 --- a/newline-query-optimization.todo +++ b/newline-query-optimization.todo @@ -164,14 +164,14 @@ Acceptance criteria: ## Phase 7 — Validation / Perf Smoke -- [ ] Run unit tests - - [ ] `dotnet test TinyTokenizer.Tests` - -- [ ] Run benchmarks (baseline + comparison) - - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks` - - [ ] Filter relevant suites: - - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxTreeBenchmarks*` - - [ ] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxEditorBenchmarks*` +- [x] Run unit tests + - [x] `dotnet test TinyTokenizer.Tests` + +- [x] Run benchmarks (baseline + comparison) + - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks` + - [x] Filter relevant suites: + - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxTreeBenchmarks*` (0 matches in current benchmark set) + - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxEditorBenchmarks*` (0 matches in current benchmark set) - [ ] (Optional) Add a targeted benchmark if needed - [x] Benchmark `SyntaxTree.Select(Query.Newline)` From 055242c66494092ff2cbbcea733a5c453bb4cb89 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:23:27 -0800 Subject: [PATCH 20/30] chore: unrestrict benchmarks --- TinyTokenizer.Benchmarks/Program.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/TinyTokenizer.Benchmarks/Program.cs b/TinyTokenizer.Benchmarks/Program.cs index aff6351..5996970 100644 --- a/TinyTokenizer.Benchmarks/Program.cs +++ b/TinyTokenizer.Benchmarks/Program.cs @@ -1,6 +1,4 @@ using BenchmarkDotNet.Running; using TinyTokenizer.Benchmarks; -BenchmarkSwitcher.FromTypes([ - typeof(NewlineQueryBenchmarks) -]).Run(args); +BenchmarkSwitcher.FromAssembly(typeof(LexerBenchmarks).Assembly).Run(args); From 8c1841a2e814a9dd92e270bd17778137d2321232 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:33:41 -0800 Subject: [PATCH 21/30] chore: remove completed task list --- newline-query-optimization.todo | 181 -------------------------------- 1 file changed, 181 deletions(-) delete mode 100644 newline-query-optimization.todo diff --git a/newline-query-optimization.todo b/newline-query-optimization.todo deleted file mode 100644 index 829779e..0000000 --- a/newline-query-optimization.todo +++ /dev/null @@ -1,181 +0,0 @@ -# Newline Query + Query Engine Optimization Plan - -Goal: make `Query.Newline` extremely optimized and semantically correct for "node occurs AFTER a newline", -while reducing allocations/overhead in the hottest Query paths (small scope). - -Confirmed semantics: -- "After a newline" means either: - - The node’s leading trivia contains a newline, OR - - The previous sibling’s trailing trivia contains a newline. - -Implementation note (post node-flags work): -- These semantics are implemented via green-node *boundary* flags (token-centric): - - `HasLeadingNewline` on the current node, OR - - `HasTrailingNewline` on the previous sibling. - -Constraints: -- Keep change surface minimal: rewrite only the hottest LINQ paths. -- Preserve document order and all existing query semantics. -- Keep newline semantics identical in red matching and green matching. - ------------------------------------------------------------------------- - -## Phase 0 — Baseline + Safety Nets - -- [x] Identify current implementation + hotspots - - [x] Locate `NewlineNodeQuery` and its `TryMatchGreen(...)` implementation - - [x] Find the selection-mode code path (`SelectModes.Last` / `ApplyMode`) - - [x] Identify the hottest `Select(...)` implementations/combinators currently using LINQ - -- [x] Add/confirm semantic tests for newline behavior - - [x] Leading trivia newline qualifies as `Query.Newline` - - [x] Previous sibling trailing trivia newline qualifies as `Query.Newline` - - [x] Both cases work at top-level and inside blocks - - [x] `Query.NotNewline` is the exact negation of `Query.Newline` for the same contexts - - [x] Edge cases: first sibling (no previous sibling), empty blocks/regions - -Acceptance criteria: -- Existing behavior is captured in tests so perf refactors don’t change semantics. - ------------------------------------------------------------------------- - -## Phase 1 — Align Newline Semantics (Green Matching) - -- [x] Update green matching for newline - - [x] Implement both checks in `NewlineNodeQuery.TryMatchGreen(parent, childIndex, out consumedCount)`: - - [x] Current node `HasLeadingNewline` - - [x] Previous sibling `HasTrailingNewline` (when `childIndex > 0`) - - [x] Use only `(parent, childIndex)` context; do not allocate red siblings - - [x] Keep `consumedCount` behavior identical (should remain 1) - -- [x] Validate parity with red matching - - [x] Ensure red matching logic uses the same semantics (leading OR previous sibling trailing) - -Acceptance criteria: -- `Query.Newline` matches exactly the confirmed semantics in all test cases. -- Green matching uses boundary flags (`HasLeadingNewline`/`HasTrailingNewline`) and does not scan trivia. - ------------------------------------------------------------------------- - -## Phase 1b — SyntaxEditor Flag Mutation Tests (Green Flags) - -Goal: ensure green-node flags remain correct after `SyntaxEditor` mutations and undo/redo. - -- [x] Add SyntaxEditor tests that assert *green* flag values after mutations - - [x] Assert boundary flags (token-centric ownership) - - [x] `Replace(...)` preserves `HasLeadingNewline` on the replaced token when leading newline trivia is preserved - - [x] `InsertAfter(...)` with a trailing newline causes the next sibling to match newline semantics via previous sibling `HasTrailingNewline` - - [x] `Remove(...)` of a token that owns trailing newline removes the `HasTrailingNewline` boundary from the tree - - [x] Assert subtree "contains" flags remain correct - - [x] `ContainsNewline` propagates correctly through blocks/lists after insert/replace/remove - - [x] Undo/Redo restores flag state - - [x] After `Commit()`, `Undo()` restores the original green flags - - [x] After `Undo()`, `Redo()` restores the mutated green flags - - [x] Prefer stable selection + direct assertions - - [x] Select the target node via `Query`/positions, then assert `node.Green` flags (`GreenNodeFlags`) - - [ ] (Optional) also assert `Query.Newline` results as a behavioral cross-check - -Acceptance criteria: -- Tests directly assert green flag bits (not just query behavior) for the above scenarios. - ------------------------------------------------------------------------- - -## Phase 2 — Remove LINQ from Selection Modes (Start with `SelectModes.Last`) - -- [x] Rewrite selection-mode handling without LINQ - - [x] Replace `LastOrDefault()` usage with a simple scan that tracks the last match - - [x] Ensure ordering/behavior matches existing semantics - - [x] Avoid buffering unless semantics strictly require it - -- [x] Expand to other modes only if they are on the hot path - - [x] Audit remaining modes for LINQ/buffering - - [x] Rewrite only those that show up in profiles/benchmarks - -Acceptance criteria: -- `SelectModes.Last` returns the same result as before, with fewer allocations. - ------------------------------------------------------------------------- - -## Phase 3 — Replace LINQ in the Hottest `Select(...)` Implementations (Small Scope) - -- [x] Identify top `Select` hot paths - - [x] Common kind queries (e.g., `KindNodeQuery.Select(...)`) - - [x] Block queries and leaf queries used heavily by editor/regions - -- [x] Replace common LINQ patterns with tight loops - - [x] Replace `.Where(...)`, `.SelectMany(...)`, `.LastOrDefault()` in hot paths - - [x] Preserve document order and short-circuiting behavior - - [x] Avoid iterator/closure allocations where possible - -Acceptance criteria: -- Query results are byte-for-byte identical in ordering and content; allocations reduced in benchmarks. - ------------------------------------------------------------------------- - -## Phase 4 — Replace LINQ in the Hottest Combinators (Small Scope) - -- [x] Remove LINQ where it forces buffering/extra iterators - - [x] OR / AnyOf-style combinators - - [x] AND / sequence-style combinators - - [x] Any other combinator used by newline queries or editor/region resolution - -- [x] Preserve semantics and ordering - - [x] Document order remains stable - - [x] No duplicate matches unless already part of the semantics - -Acceptance criteria: -- Combinator behavior remains unchanged; perf improves in existing suites. - ------------------------------------------------------------------------- - -## Phase 5 — Optimize Region Resolution (Materialize-on-Match) - -- [x] Refactor region traversal to avoid per-visited-node allocations - - [x] Maintain an incremental slot-index stack while walking the tree - - [x] Construct `NodePath` only when a match is found (snapshot stack) - - [x] Avoid `NodePath.FromNode(...)` and repeated `.ToArray()` allocations in scans - -- [x] Validate correctness - - [x] Query regions (`IRegionQuery` + match-based fallback) return identical regions - - [x] Editor operations that depend on regions remain stable - -Acceptance criteria: -- Region-heavy operations allocate less and remain semantically identical. - ------------------------------------------------------------------------- - -## Phase 6 — Documentation Sanity - -- [x] Update docs/comments to explicitly state newline semantics - - [x] Clarify "node after newline" definition (leading trivia OR previous sibling trailing trivia) - - [x] Remove/correct any mention of "newline whitespace tokens" if newline is trivia-only - - [x] Call out token-centric boundary ownership (containers do not "own" boundary newline flags) - - [x] Ensure docs match actual implementation - -Suggested doc touchpoints: -- [x] TinyTokenizer.wiki/Query-API.md -- [x] TinyTokenizer.wiki/Trivia.md -- [ ] TinyTokenizer.wiki/TreeWalker.md (only if it mentions newline semantics) - -Acceptance criteria: -- Public docs and XML comments match runtime behavior. - ------------------------------------------------------------------------- - -## Phase 7 — Validation / Perf Smoke - -- [x] Run unit tests - - [x] `dotnet test TinyTokenizer.Tests` - -- [x] Run benchmarks (baseline + comparison) - - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks` - - [x] Filter relevant suites: - - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxTreeBenchmarks*` (0 matches in current benchmark set) - - [x] `dotnet run -c Release --project TinyTokenizer.Benchmarks -- --filter *SyntaxEditorBenchmarks*` (0 matches in current benchmark set) - -- [ ] (Optional) Add a targeted benchmark if needed - - [x] Benchmark `SyntaxTree.Select(Query.Newline)` - - [ ] Benchmark `SelectModes.Last` on a newline-heavy query - -Acceptance criteria: -- Tests pass; perf smoke shows reduced allocations or faster execution in the affected scenarios. From 043925d8ac5a6d2525c1b1b028f3a39e95ce60c2 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:33:50 -0800 Subject: [PATCH 22/30] chore: add new task list for test coverage --- syntaxeditor-flag-mutation-test-gaps.todo | 159 ++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 syntaxeditor-flag-mutation-test-gaps.todo diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo new file mode 100644 index 0000000..a80d360 --- /dev/null +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -0,0 +1,159 @@ +```todo +# SyntaxEditor Green-Flag Mutation Test Gaps + +Goal: expand `SyntaxEditor` test coverage to ensure `GreenNodeFlags` stay correct after mutations, +with emphasis on boundary-vs-contains semantics and undo/redo correctness. + +Context: +- Boundary flags are token-owned (`HasLeading*` / `HasTrailing*`), and containers must not inherit them. +- Contains flags (`Contains*`) must accurately reflect subtree presence. +- We already have a small set of green-flag mutation tests in `TinyTokenizer.Tests/SyntaxEditorTests.cs`. + +------------------------------------------------------------------------------ + +## Phase 0 — Inventory + Harness + +- [ ] Decide location for new tests + - [ ] Keep under `#region Green Flag Mutation Tests` in `TinyTokenizer.Tests/SyntaxEditorTests.cs`, OR + - [ ] Create a dedicated test class file (e.g., `TinyTokenizer.Tests/SyntaxEditorGreenFlagsTests.cs`) for clarity + +- [ ] Add a small helper to find leaves consistently after edits + - [ ] Helper: `FindToken(tree, kind, text)` (or `Query.Ident("...")` + `.OfType()`) + - [ ] Avoid relying on indexes when trivia can shift; prefer `Query` + text predicates + +Acceptance criteria: +- Tests are easy to read and robust to trivia shape. + +------------------------------------------------------------------------------ + +## Phase 1 — Boundary Flags After Replace + +- [ ] Replace preserves *trailing newline* boundary ownership + - [ ] Source: `"a\nb"` + - [ ] Replace `a` -> `X` + - [ ] Assert `X.Green.Flags` includes `HasTrailingNewlineTrivia` + - [ ] Assert `b.Green.Flags` does NOT include `HasLeadingNewlineTrivia` + - [ ] Assert `tree.GreenRoot.Flags` includes `ContainsNewlineTrivia` + +- [ ] Replace preserves *same-line comment* trailing boundary flags + - [ ] Use `TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine)` + - [ ] Source: `"a // c\nb"` + - [ ] Replace `a` -> `X` + - [ ] Assert `X.Green.Flags` includes `HasTrailingCommentTrivia` AND `HasTrailingNewlineTrivia` + - [ ] Assert `b.Green.Flags` does NOT include `HasLeadingCommentTrivia` or `HasLeadingNewlineTrivia` + +- [ ] Replace preserves/tranfers *trailing whitespace* boundary (non-newline) + - [ ] Source that produces trailing whitespace trivia on the replaced token (e.g., `"a \nb"` or similar) + - [ ] Replace target token + - [ ] Assert `HasTrailingWhitespaceTrivia` is preserved on replacement token + +Acceptance criteria: +- Boundary flags remain token-centric and are preserved on replacement where trivia is preserved. + +------------------------------------------------------------------------------ + +## Phase 2 — Boundary Flags After InsertBefore / InsertAfter + +- [ ] InsertBefore does NOT steal leading whitespace from the following node + - [ ] Source: `"a b"` (two spaces before `b`) + - [ ] InsertBefore(`b`, `"X"`) + - [ ] Assert `b.Green.Flags` includes `HasLeadingWhitespaceTrivia` + - [ ] Assert `X.Green.Flags` does NOT unexpectedly include `HasLeadingWhitespaceTrivia` + - [ ] Assert `tree.GreenRoot.Flags` includes `ContainsWhitespaceTrivia` + +- [ ] InsertAfter does not accidentally manufacture leading newline on following token + - [ ] Already partially covered for inserted `X\n`; add coverage for variations: + - [ ] InsertAfter with `"X\r\n"` (CRLF) + - [ ] InsertAfter with `" X\n"` (inserted node has leading whitespace + trailing newline) + - [ ] Assert following token does NOT gain `HasLeadingNewlineTrivia` + +Acceptance criteria: +- Insertion doesn’t transfer boundary ownership incorrectly and doesn’t corrupt following-token boundary flags. + +------------------------------------------------------------------------------ + +## Phase 3 — Contains Flags Correctness Under Partial Removal + +- [ ] Removing one newline owner does not clear `ContainsNewlineTrivia` when others remain + - [ ] Source: `"a\nb\nc"` + - [ ] Remove `a` + - [ ] Assert `tree.GreenRoot.Flags` still includes `ContainsNewlineTrivia` + +- [ ] Removing one comment owner does not clear `ContainsCommentTrivia` when others remain + - [ ] Use `CommentStyle.CStyleSingleLine` + - [ ] Source with multiple comments + - [ ] Remove one token owning a comment + - [ ] Assert `ContainsCommentTrivia` remains set + +- [ ] Removing one whitespace owner does not clear `ContainsWhitespaceTrivia` when others remain + - [ ] Source with multiple leading/trailing whitespace trivia across tokens + - [ ] Remove one token + - [ ] Assert `ContainsWhitespaceTrivia` remains set + +Acceptance criteria: +- Contains flags behave like true subtree aggregations (don’t clear prematurely). + +------------------------------------------------------------------------------ + +## Phase 4 — Multi-node Replacement Trivia Transfer (First/Last) + +- [ ] Replace a token with *multiple nodes* transfers leading boundary to first and trailing boundary to last + - [ ] Source where target has leading boundary trivia and trailing boundary trivia + - [ ] Replace target with `"X Y"` + - [ ] Assert `X` receives the leading boundary flags expected + - [ ] Assert `Y` receives the trailing boundary flags expected + - [ ] Assert middle nodes (if any) do not get boundary flags unless their own trivia demands it + +- [ ] Replace with nodes that begin/end with containers (edge-case semantics) + - [ ] Replace a token with something like `"{x}"` or `"(x)"` + - [ ] Decide expected behavior: boundary trivia should attach only if first/last replacement nodes are leaves + - [ ] Add assertions to lock in intended behavior (or adjust `TransferTrivia` if desired) + +Acceptance criteria: +- Trivia transfer semantics are explicitly tested and stable. + +------------------------------------------------------------------------------ + +## Phase 5 — Undo/Redo Leaf-Level Flag Restoration + +- [ ] Undo/Redo restores *leaf* boundary flags, not just root flags + - [ ] Start with a tree where a target leaf’s flags are known + - [ ] Commit an edit that changes boundary/contains flags (e.g., insert `X\n`) + - [ ] Capture the leaf flags before/after + - [ ] Undo and assert leaf flags are restored + - [ ] Redo and assert leaf flags match the mutated state + +Acceptance criteria: +- Undo/Redo correctness is verified at the leaf level. + +------------------------------------------------------------------------------ + +## Phase 6 — Schema/Rebind Interaction (Syntax Binding) + +- [ ] Ensure edits with schema + syntax definitions do not violate token-centric boundary semantics + - [ ] Parse with a schema that includes syntax definitions (so `Commit()` triggers `RebindAt`) + - [ ] Perform insert/replace/remove inside or around bound syntax nodes + - [ ] Assert: + - [ ] leaf boundary flags are correct + - [ ] syntax containers do NOT have `GreenNodeFlagMasks.Boundary` + - [ ] contains flags reflect subtree + +Acceptance criteria: +- Rebinding does not break boundary/contains invariants. + +------------------------------------------------------------------------------ + +## Phase 7 — Oracle-Style Regression Test (Optional “Big Hammer”) + +- [ ] Add a “reparse oracle” test for flags + - [ ] After an edit, compute `editedText = tree.ToText()` + - [ ] Parse a fresh `oracle = SyntaxTree.Parse(editedText, sameOptionsOrSchema)` + - [ ] Compare: + - [ ] `tree.GreenRoot.Flags == oracle.GreenRoot.Flags` + - [ ] Per-leaf flags match for corresponding leaves (by `(Kind, Text, occurrence index)` or by positions) + - [ ] Use this for 1–2 representative complex edits (nested blocks + comments) + +Acceptance criteria: +- A broad regression net exists that catches stale/miscomputed flags without enumerating every case. + +``` From b874c34249050dc941495def3eda2f34f3861e29 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:39:38 -0800 Subject: [PATCH 23/30] [phase 0]: Inventory + Harness --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 19 +++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 10 +++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 182a107..c3f7a86 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1626,6 +1626,25 @@ private static void AssertNotHasFlags(GreenNodeFlags actual, GreenNodeFlags unex Assert.True((actual & unexpected) == 0, $"Expected flags to NOT include {unexpected} but was {actual}"); } + private static SyntaxToken FindToken(SyntaxTree tree, NodeKind kind, string text, int occurrence = 0) + { + ArgumentNullException.ThrowIfNull(tree); + ArgumentNullException.ThrowIfNull(text); + + var matches = tree + .Select(Query.Kind(kind).WithText(text)) + .OfType() + .ToList(); + + Assert.True(matches.Count > 0, $"Expected to find at least 1 token of kind '{kind}' with text '{text}', but found none."); + Assert.True( + occurrence >= 0 && occurrence < matches.Count, + $"Expected occurrence {occurrence} for token kind '{kind}' text '{text}', but only found {matches.Count} match(es)." + ); + + return matches[occurrence]; + } + [Fact] public void Replace_OwnLineCommentLeadingTrivia_PreservesGreenBoundaryFlags_OnReplacement() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index a80d360..7128b1f 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -13,13 +13,13 @@ Context: ## Phase 0 — Inventory + Harness -- [ ] Decide location for new tests - - [ ] Keep under `#region Green Flag Mutation Tests` in `TinyTokenizer.Tests/SyntaxEditorTests.cs`, OR +- [x] Decide location for new tests + - [x] Keep under `#region Green Flag Mutation Tests` in `TinyTokenizer.Tests/SyntaxEditorTests.cs`, OR - [ ] Create a dedicated test class file (e.g., `TinyTokenizer.Tests/SyntaxEditorGreenFlagsTests.cs`) for clarity -- [ ] Add a small helper to find leaves consistently after edits - - [ ] Helper: `FindToken(tree, kind, text)` (or `Query.Ident("...")` + `.OfType()`) - - [ ] Avoid relying on indexes when trivia can shift; prefer `Query` + text predicates +- [x] Add a small helper to find leaves consistently after edits + - [x] Helper: `FindToken(tree, kind, text)` (or `Query.Ident("...")` + `.OfType()`) + - [x] Avoid relying on indexes when trivia can shift; prefer `Query` + text predicates Acceptance criteria: - Tests are easy to read and robust to trivia shape. From aba5043fb2bd9e1e86272515d38bb72510186074 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 02:52:23 -0800 Subject: [PATCH 24/30] [phase 1]: Boundary Flags After Replace --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 61 +++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 36 ++++++------- 2 files changed, 79 insertions(+), 18 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index c3f7a86..99812b4 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1663,6 +1663,67 @@ public void Replace_OwnLineCommentLeadingTrivia_PreservesGreenBoundaryFlags_OnRe AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.ContainsNewlineTrivia | GreenNodeFlags.ContainsCommentTrivia); } + [Fact] + public void Replace_TokenOwningTrailingNewline_PreservesGreenBoundaryFlags_OnReplacement() + { + var tree = SyntaxTree.Parse("a\nb"); + + var aBefore = FindToken(tree, NodeKind.Ident, "a"); + AssertHasFlags(aBefore.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("a"), "X") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void Replace_SameLineCommentTrailingTrivia_PreservesGreenBoundaryFlags_OnReplacement() + { + var options = TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine); + var tree = SyntaxTree.Parse("a // c\nb", options); + + var aBefore = FindToken(tree, NodeKind.Ident, "a"); + AssertHasFlags(aBefore.Green.Flags, GreenNodeFlags.HasTrailingCommentTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("a"), "X") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingCommentTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingCommentTrivia | GreenNodeFlags.HasLeadingNewlineTrivia); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsCommentTrivia | GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void Replace_TokenOwningTrailingWhitespace_PreservesGreenBoundaryFlags_OnReplacement() + { + var tree = SyntaxTree.Parse("a b"); + + var aBefore = FindToken(tree, NodeKind.Ident, "a"); + AssertHasFlags(aBefore.Green.Flags, GreenNodeFlags.HasTrailingWhitespaceTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("a"), "X") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingWhitespaceTrivia); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); + } + [Fact] public void InsertAfter_InsertedTextWithTrailingNewline_SetsGreenFlags_OnInsertedAndFollowingTokens() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index 7128b1f..f77d04f 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -28,24 +28,24 @@ Acceptance criteria: ## Phase 1 — Boundary Flags After Replace -- [ ] Replace preserves *trailing newline* boundary ownership - - [ ] Source: `"a\nb"` - - [ ] Replace `a` -> `X` - - [ ] Assert `X.Green.Flags` includes `HasTrailingNewlineTrivia` - - [ ] Assert `b.Green.Flags` does NOT include `HasLeadingNewlineTrivia` - - [ ] Assert `tree.GreenRoot.Flags` includes `ContainsNewlineTrivia` - -- [ ] Replace preserves *same-line comment* trailing boundary flags - - [ ] Use `TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine)` - - [ ] Source: `"a // c\nb"` - - [ ] Replace `a` -> `X` - - [ ] Assert `X.Green.Flags` includes `HasTrailingCommentTrivia` AND `HasTrailingNewlineTrivia` - - [ ] Assert `b.Green.Flags` does NOT include `HasLeadingCommentTrivia` or `HasLeadingNewlineTrivia` - -- [ ] Replace preserves/tranfers *trailing whitespace* boundary (non-newline) - - [ ] Source that produces trailing whitespace trivia on the replaced token (e.g., `"a \nb"` or similar) - - [ ] Replace target token - - [ ] Assert `HasTrailingWhitespaceTrivia` is preserved on replacement token +- [x] Replace preserves *trailing newline* boundary ownership + - [x] Source: `"a\nb"` + - [x] Replace `a` -> `X` + - [x] Assert `X.Green.Flags` includes `HasTrailingNewlineTrivia` + - [x] Assert `b.Green.Flags` does NOT include `HasLeadingNewlineTrivia` + - [x] Assert `tree.GreenRoot.Flags` includes `ContainsNewlineTrivia` + +- [x] Replace preserves *same-line comment* trailing boundary flags + - [x] Use `TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine)` + - [x] Source: `"a // c\nb"` + - [x] Replace `a` -> `X` + - [x] Assert `X.Green.Flags` includes `HasTrailingCommentTrivia` AND `HasTrailingNewlineTrivia` + - [x] Assert `b.Green.Flags` does NOT include `HasLeadingCommentTrivia` or `HasLeadingNewlineTrivia` + +- [x] Replace preserves/tranfers *trailing whitespace* boundary (non-newline) + - [x] Source that produces trailing whitespace trivia on the replaced token (e.g., `"a \nb"` or similar) + - [x] Replace target token + - [x] Assert `HasTrailingWhitespaceTrivia` is preserved on replacement token Acceptance criteria: - Boundary flags remain token-centric and are preserved on replacement where trivia is preserved. From 968685ea21ebc7014f430fedf203e48428c4f349 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 03:01:02 -0800 Subject: [PATCH 25/30] [phase 2]: Boundary Flags After InsertBefore / InsertAfter --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 44 +++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 24 ++++++------- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 99812b4..1120b84 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1745,6 +1745,50 @@ public void InsertAfter_InsertedTextWithTrailingNewline_SetsGreenFlags_OnInserte AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); } + [Fact] + public void InsertBefore_DoesNotStealLeadingWhitespace_FromFollowingToken_GreenFlags() + { + // In the token-centric trivia model, indentation after a newline is leading trivia on the following token. + var tree = SyntaxTree.Parse("a\n b"); + + var bBefore = FindToken(tree, NodeKind.Ident, "b"); + AssertHasFlags(bBefore.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + + tree.CreateEditor() + .InsertBefore(bBefore, "X") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + // Insertion must not transfer whitespace ownership from b to X. + AssertHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + AssertNotHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + + // Root should reflect subtree contains. + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); + } + + [Fact] + public void InsertAfter_InsertedTextWithTrailingCRLF_SetsGreenFlags_OnInsertedAndFollowingTokens() + { + var tree = SyntaxTree.Parse("a b"); + var aNode = FindToken(tree, NodeKind.Ident, "a"); + + tree.CreateEditor() + .InsertAfter(aNode, " X\r\n") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + // Inserted token owns the trailing newline; following token should NOT gain leading newline ownership. + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia | GreenNodeFlags.ContainsNewlineTrivia); + AssertNotHasFlags(bAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia); + + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + [Fact] public void Remove_TokenOwningTrailingNewline_RemovesNewlineBoundaryAndContainsFlags() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index f77d04f..7b4e007 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -54,18 +54,18 @@ Acceptance criteria: ## Phase 2 — Boundary Flags After InsertBefore / InsertAfter -- [ ] InsertBefore does NOT steal leading whitespace from the following node - - [ ] Source: `"a b"` (two spaces before `b`) - - [ ] InsertBefore(`b`, `"X"`) - - [ ] Assert `b.Green.Flags` includes `HasLeadingWhitespaceTrivia` - - [ ] Assert `X.Green.Flags` does NOT unexpectedly include `HasLeadingWhitespaceTrivia` - - [ ] Assert `tree.GreenRoot.Flags` includes `ContainsWhitespaceTrivia` - -- [ ] InsertAfter does not accidentally manufacture leading newline on following token - - [ ] Already partially covered for inserted `X\n`; add coverage for variations: - - [ ] InsertAfter with `"X\r\n"` (CRLF) - - [ ] InsertAfter with `" X\n"` (inserted node has leading whitespace + trailing newline) - - [ ] Assert following token does NOT gain `HasLeadingNewlineTrivia` +- [x] InsertBefore does NOT steal leading whitespace from the following node + - [x] Source: `"a\n b"` (indentation is leading whitespace on `b`) + - [x] InsertBefore(`b`, `"X"`) + - [x] Assert `b.Green.Flags` includes `HasLeadingWhitespaceTrivia` + - [x] Assert `X.Green.Flags` does NOT unexpectedly include `HasLeadingWhitespaceTrivia` + - [x] Assert `tree.GreenRoot.Flags` includes `ContainsWhitespaceTrivia` + +- [x] InsertAfter does not accidentally manufacture leading newline on following token + - [x] Already partially covered for inserted `X\n`; add coverage for variations: + - [x] InsertAfter with `"X\r\n"` (CRLF) + - [x] InsertAfter with `" X\n"` (inserted node has leading whitespace + trailing newline) + - [x] Assert following token does NOT gain `HasLeadingNewlineTrivia` Acceptance criteria: - Insertion doesn’t transfer boundary ownership incorrectly and doesn’t corrupt following-token boundary flags. From 56c12e7bb147ba2035ef51059cedd891ac0fff69 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 03:14:27 -0800 Subject: [PATCH 26/30] [phase 3]: Contains Flags Correctness Under Partial Removal --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 44 +++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 30 ++++++++-------- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 1120b84..5efdc40 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1809,6 +1809,50 @@ public void Remove_TokenOwningTrailingNewline_RemovesNewlineBoundaryAndContainsF AssertNotHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); } + [Fact] + public void Remove_OneNewlineOwner_DoesNotClearContainsNewlineTrivia_WhenOthersRemain() + { + var tree = SyntaxTree.Parse("a\nb\nc"); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + + tree.CreateEditor() + .Remove(Q.Ident("a")) + .Commit(); + + // The newline between b and c should still exist. + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void Remove_OneCommentOwner_DoesNotClearContainsCommentTrivia_WhenOthersRemain() + { + var options = TokenizerOptions.Default.WithCommentStyles(CommentStyle.CStyleSingleLine); + var tree = SyntaxTree.Parse("a // c1\nb // c2\nc", options); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsCommentTrivia); + + tree.CreateEditor() + .Remove(Q.Ident("a")) + .Commit(); + + // The comment after b should still exist. + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsCommentTrivia); + } + + [Fact] + public void Remove_OneWhitespaceOwner_DoesNotClearContainsWhitespaceTrivia_WhenOthersRemain() + { + // Two separate whitespace regions: after 'a' and after 'b'. + var tree = SyntaxTree.Parse("a b c"); + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); + + tree.CreateEditor() + .Remove(Q.Ident("a")) + .Commit(); + + // The remaining space between b and c should still exist. + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); + } + [Fact] public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index 7b4e007..debcf7d 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -74,21 +74,21 @@ Acceptance criteria: ## Phase 3 — Contains Flags Correctness Under Partial Removal -- [ ] Removing one newline owner does not clear `ContainsNewlineTrivia` when others remain - - [ ] Source: `"a\nb\nc"` - - [ ] Remove `a` - - [ ] Assert `tree.GreenRoot.Flags` still includes `ContainsNewlineTrivia` - -- [ ] Removing one comment owner does not clear `ContainsCommentTrivia` when others remain - - [ ] Use `CommentStyle.CStyleSingleLine` - - [ ] Source with multiple comments - - [ ] Remove one token owning a comment - - [ ] Assert `ContainsCommentTrivia` remains set - -- [ ] Removing one whitespace owner does not clear `ContainsWhitespaceTrivia` when others remain - - [ ] Source with multiple leading/trailing whitespace trivia across tokens - - [ ] Remove one token - - [ ] Assert `ContainsWhitespaceTrivia` remains set +- [x] Removing one newline owner does not clear `ContainsNewlineTrivia` when others remain + - [x] Source: `"a\nb\nc"` + - [x] Remove `a` + - [x] Assert `tree.GreenRoot.Flags` still includes `ContainsNewlineTrivia` + +- [x] Removing one comment owner does not clear `ContainsCommentTrivia` when others remain + - [x] Use `CommentStyle.CStyleSingleLine` + - [x] Source with multiple comments + - [x] Remove one token owning a comment + - [x] Assert `ContainsCommentTrivia` remains set + +- [x] Removing one whitespace owner does not clear `ContainsWhitespaceTrivia` when others remain + - [x] Source with multiple leading/trailing whitespace trivia across tokens + - [x] Remove one token + - [x] Assert `ContainsWhitespaceTrivia` remains set Acceptance criteria: - Contains flags behave like true subtree aggregations (don’t clear prematurely). From 007154a843cd762a6c9dc7a8947d205aaeb7bde7 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 03:30:02 -0800 Subject: [PATCH 27/30] [phase 4]: Multi-node Replacement Trivia Transfer --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 54 +++++++ TinyTokenizer/Ast/SyntaxEditor.cs | 168 ++++++++++++++++------ syntaxeditor-flag-mutation-test-gaps.todo | 22 +-- 3 files changed, 193 insertions(+), 51 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 5efdc40..8301121 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1853,6 +1853,60 @@ public void Remove_OneWhitespaceOwner_DoesNotClearContainsWhitespaceTrivia_WhenO AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia); } + [Fact] + public void Replace_MultipleNodes_TransfersLeadingToFirst_AndTrailingToLast_GreenBoundaryFlags() + { + // b owns leading whitespace (indentation) and trailing newline. + var tree = SyntaxTree.Parse("a\n b\nc"); + + var bBefore = FindToken(tree, NodeKind.Ident, "b"); + AssertHasFlags(bBefore.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("b"), "X Y") + .Commit(); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var yAfter = FindToken(tree, NodeKind.Ident, "Y"); + var cAfter = FindToken(tree, NodeKind.Ident, "c"); + + // Leading boundary transfers to first replacement node. + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + AssertNotHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + + // Trailing boundary transfers to last replacement node. + AssertHasFlags(yAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(yAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + + // Following token should not incorrectly gain leading newline ownership. + AssertNotHasFlags(cAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia); + + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void Replace_WithBlockAtEdges_TransfersTriviaToBlockBoundaries_GreenFlags() + { + // Replacement begins/ends with a container (block). Trivia should attach to opener/closer boundaries. + var tree = SyntaxTree.Parse("a\n b\nc"); + + var bBefore = FindToken(tree, NodeKind.Ident, "b"); + AssertHasFlags(bBefore.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + + tree.CreateEditor() + .Replace(Q.Ident("b"), "{x}") + .Commit(); + + var block = Assert.Single(tree.Select(Q.BraceBlock).OfType()); + + // Boundary trivia should be preserved on the block boundaries. + AssertHasFlags(block.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + + // Containers must not accidentally carry child boundary flags beyond their own semantics. + // (Block boundary flags are only opener-leading and closer-trailing.) + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsNewlineTrivia); + } + [Fact] public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() { diff --git a/TinyTokenizer/Ast/SyntaxEditor.cs b/TinyTokenizer/Ast/SyntaxEditor.cs index 47efd84..77e3148 100644 --- a/TinyTokenizer/Ast/SyntaxEditor.cs +++ b/TinyTokenizer/Ast/SyntaxEditor.cs @@ -1066,21 +1066,43 @@ private static ImmutableArray TransferTrivia( var result = nodes.ToBuilder(); // Add leading trivia to first node - if (!leading.IsEmpty && result[0] is GreenLeaf firstLeaf) + if (!leading.IsEmpty) { - var newLeading = firstLeaf.LeadingTrivia.IsEmpty - ? leading - : leading.AddRange(firstLeaf.LeadingTrivia); - result[0] = firstLeaf.WithLeadingTrivia(newLeading); + if (result[0] is GreenLeaf firstLeaf) + { + var newLeading = firstLeaf.LeadingTrivia.IsEmpty + ? leading + : leading.AddRange(firstLeaf.LeadingTrivia); + result[0] = firstLeaf.WithLeadingTrivia(newLeading); + } + else if (result[0] is GreenBlock firstBlock) + { + var existingLeading = firstBlock.OpenerNode.LeadingTrivia; + var newLeading = existingLeading.IsEmpty + ? leading + : leading.AddRange(existingLeading); + result[0] = firstBlock.WithLeadingTrivia(newLeading); + } } // Add trailing trivia to last node - if (!trailing.IsEmpty && result[^1] is GreenLeaf lastLeaf) + if (!trailing.IsEmpty) { - var newTrailing = lastLeaf.TrailingTrivia.IsEmpty - ? trailing - : lastLeaf.TrailingTrivia.AddRange(trailing); - result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + if (result[^1] is GreenLeaf lastLeaf) + { + var newTrailing = lastLeaf.TrailingTrivia.IsEmpty + ? trailing + : lastLeaf.TrailingTrivia.AddRange(trailing); + result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + } + else if (result[^1] is GreenBlock lastBlock) + { + var existingTrailing = lastBlock.CloserNode.TrailingTrivia; + var newTrailing = existingTrailing.IsEmpty + ? trailing + : existingTrailing.AddRange(trailing); + result[^1] = lastBlock.WithTrailingTrivia(newTrailing); + } } return result.ToImmutable(); @@ -1143,21 +1165,43 @@ private static ImmutableArray TransferTrivia( var result = nodes.ToBuilder(); // Add leading trivia to first node - if (!leading.IsEmpty && result[0] is GreenLeaf firstLeaf) + if (!leading.IsEmpty) { - var newLeading = firstLeaf.LeadingTrivia.IsEmpty - ? leading - : leading.AddRange(firstLeaf.LeadingTrivia); - result[0] = firstLeaf.WithLeadingTrivia(newLeading); + if (result[0] is GreenLeaf firstLeaf) + { + var newLeading = firstLeaf.LeadingTrivia.IsEmpty + ? leading + : leading.AddRange(firstLeaf.LeadingTrivia); + result[0] = firstLeaf.WithLeadingTrivia(newLeading); + } + else if (result[0] is GreenBlock firstBlock) + { + var existingLeading = firstBlock.OpenerNode.LeadingTrivia; + var newLeading = existingLeading.IsEmpty + ? leading + : leading.AddRange(existingLeading); + result[0] = firstBlock.WithLeadingTrivia(newLeading); + } } // Add trailing trivia to last node - if (!trailing.IsEmpty && result[^1] is GreenLeaf lastLeaf) + if (!trailing.IsEmpty) { - var newTrailing = lastLeaf.TrailingTrivia.IsEmpty - ? trailing - : lastLeaf.TrailingTrivia.AddRange(trailing); - result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + if (result[^1] is GreenLeaf lastLeaf) + { + var newTrailing = lastLeaf.TrailingTrivia.IsEmpty + ? trailing + : lastLeaf.TrailingTrivia.AddRange(trailing); + result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + } + else if (result[^1] is GreenBlock lastBlock) + { + var existingTrailing = lastBlock.CloserNode.TrailingTrivia; + var newTrailing = existingTrailing.IsEmpty + ? trailing + : existingTrailing.AddRange(trailing); + result[^1] = lastBlock.WithTrailingTrivia(newTrailing); + } } return result.ToImmutable(); @@ -1241,20 +1285,42 @@ private static ImmutableArray TransferTrivia( var result = nodes.ToBuilder(); - if (!leading.IsEmpty && result[0] is GreenLeaf firstLeaf) + if (!leading.IsEmpty) { - var newLeading = firstLeaf.LeadingTrivia.IsEmpty - ? leading - : leading.AddRange(firstLeaf.LeadingTrivia); - result[0] = firstLeaf.WithLeadingTrivia(newLeading); + if (result[0] is GreenLeaf firstLeaf) + { + var newLeading = firstLeaf.LeadingTrivia.IsEmpty + ? leading + : leading.AddRange(firstLeaf.LeadingTrivia); + result[0] = firstLeaf.WithLeadingTrivia(newLeading); + } + else if (result[0] is GreenBlock firstBlock) + { + var existingLeading = firstBlock.OpenerNode.LeadingTrivia; + var newLeading = existingLeading.IsEmpty + ? leading + : leading.AddRange(existingLeading); + result[0] = firstBlock.WithLeadingTrivia(newLeading); + } } - if (!trailing.IsEmpty && result[^1] is GreenLeaf lastLeaf) + if (!trailing.IsEmpty) { - var newTrailing = lastLeaf.TrailingTrivia.IsEmpty - ? trailing - : lastLeaf.TrailingTrivia.AddRange(trailing); - result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + if (result[^1] is GreenLeaf lastLeaf) + { + var newTrailing = lastLeaf.TrailingTrivia.IsEmpty + ? trailing + : lastLeaf.TrailingTrivia.AddRange(trailing); + result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + } + else if (result[^1] is GreenBlock lastBlock) + { + var existingTrailing = lastBlock.CloserNode.TrailingTrivia; + var newTrailing = existingTrailing.IsEmpty + ? trailing + : existingTrailing.AddRange(trailing); + result[^1] = lastBlock.WithTrailingTrivia(newTrailing); + } } return result.ToImmutable(); @@ -1309,20 +1375,42 @@ private static ImmutableArray TransferTrivia( var result = nodes.ToBuilder(); - if (!leading.IsEmpty && result[0] is GreenLeaf firstLeaf) + if (!leading.IsEmpty) { - var newLeading = firstLeaf.LeadingTrivia.IsEmpty - ? leading - : leading.AddRange(firstLeaf.LeadingTrivia); - result[0] = firstLeaf.WithLeadingTrivia(newLeading); + if (result[0] is GreenLeaf firstLeaf) + { + var newLeading = firstLeaf.LeadingTrivia.IsEmpty + ? leading + : leading.AddRange(firstLeaf.LeadingTrivia); + result[0] = firstLeaf.WithLeadingTrivia(newLeading); + } + else if (result[0] is GreenBlock firstBlock) + { + var existingLeading = firstBlock.OpenerNode.LeadingTrivia; + var newLeading = existingLeading.IsEmpty + ? leading + : leading.AddRange(existingLeading); + result[0] = firstBlock.WithLeadingTrivia(newLeading); + } } - if (!trailing.IsEmpty && result[^1] is GreenLeaf lastLeaf) + if (!trailing.IsEmpty) { - var newTrailing = lastLeaf.TrailingTrivia.IsEmpty - ? trailing - : lastLeaf.TrailingTrivia.AddRange(trailing); - result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + if (result[^1] is GreenLeaf lastLeaf) + { + var newTrailing = lastLeaf.TrailingTrivia.IsEmpty + ? trailing + : lastLeaf.TrailingTrivia.AddRange(trailing); + result[^1] = lastLeaf.WithTrailingTrivia(newTrailing); + } + else if (result[^1] is GreenBlock lastBlock) + { + var existingTrailing = lastBlock.CloserNode.TrailingTrivia; + var newTrailing = existingTrailing.IsEmpty + ? trailing + : existingTrailing.AddRange(trailing); + result[^1] = lastBlock.WithTrailingTrivia(newTrailing); + } } return result.ToImmutable(); diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index debcf7d..6dd7147 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -97,17 +97,17 @@ Acceptance criteria: ## Phase 4 — Multi-node Replacement Trivia Transfer (First/Last) -- [ ] Replace a token with *multiple nodes* transfers leading boundary to first and trailing boundary to last - - [ ] Source where target has leading boundary trivia and trailing boundary trivia - - [ ] Replace target with `"X Y"` - - [ ] Assert `X` receives the leading boundary flags expected - - [ ] Assert `Y` receives the trailing boundary flags expected - - [ ] Assert middle nodes (if any) do not get boundary flags unless their own trivia demands it - -- [ ] Replace with nodes that begin/end with containers (edge-case semantics) - - [ ] Replace a token with something like `"{x}"` or `"(x)"` - - [ ] Decide expected behavior: boundary trivia should attach only if first/last replacement nodes are leaves - - [ ] Add assertions to lock in intended behavior (or adjust `TransferTrivia` if desired) +- [x] Replace a token with *multiple nodes* transfers leading boundary to first and trailing boundary to last + - [x] Source where target has leading boundary trivia and trailing boundary trivia + - [x] Replace target with `"X Y"` + - [x] Assert `X` receives the leading boundary flags expected + - [x] Assert `Y` receives the trailing boundary flags expected + - [x] Assert middle nodes (if any) do not get boundary flags unless their own trivia demands it + +- [x] Replace with nodes that begin/end with containers (edge-case semantics) + - [x] Replace a token with something like `"{x}"` or `"(x)"` + - [x] Decide expected behavior: preserve boundary trivia on block opener/closer when replacement is a block + - [x] Add assertions to lock in intended behavior Acceptance criteria: - Trivia transfer semantics are explicitly tested and stable. From db19b25ca1d85aacbe7c55b39a8de4502275768e Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 03:31:38 -0800 Subject: [PATCH 28/30] [phase 5]: Undo/Redo Leaf-Level Flag Restoration --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 47 +++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 12 +++--- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 8301121..2864eaa 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1907,6 +1907,53 @@ public void Replace_WithBlockAtEdges_TransfersTriviaToBlockBoundaries_GreenFlags AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsNewlineTrivia); } + [Fact] + public void UndoRedo_RestoresLeafBoundaryFlags_NotJustRootFlags() + { + var tree = SyntaxTree.Parse("a b"); + var aBefore = FindToken(tree, NodeKind.Ident, "a"); + var bBefore = FindToken(tree, NodeKind.Ident, "b"); + + var aFlagsBefore = aBefore.Green.Flags; + var bFlagsBefore = bBefore.Green.Flags; + var rootFlagsBefore = tree.GreenRoot.Flags; + + tree.CreateEditor() + .InsertAfter(aBefore, " X\n") + .Commit(); + + var aAfter = FindToken(tree, NodeKind.Ident, "a"); + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var bAfter = FindToken(tree, NodeKind.Ident, "b"); + + var aFlagsAfter = aAfter.Green.Flags; + var xFlagsAfter = xAfter.Green.Flags; + var bFlagsAfter = bAfter.Green.Flags; + var rootFlagsAfter = tree.GreenRoot.Flags; + + // Sanity: mutation should introduce a newline owner. + AssertHasFlags(xFlagsAfter, GreenNodeFlags.HasTrailingNewlineTrivia); + AssertHasFlags(rootFlagsAfter, GreenNodeFlags.ContainsNewlineTrivia); + + Assert.True(tree.Undo()); + + var aUndo = FindToken(tree, NodeKind.Ident, "a"); + var bUndo = FindToken(tree, NodeKind.Ident, "b"); + Assert.Equal(aFlagsBefore, aUndo.Green.Flags); + Assert.Equal(bFlagsBefore, bUndo.Green.Flags); + Assert.Equal(rootFlagsBefore, tree.GreenRoot.Flags); + + Assert.True(tree.Redo()); + + var aRedo = FindToken(tree, NodeKind.Ident, "a"); + var xRedo = FindToken(tree, NodeKind.Ident, "X"); + var bRedo = FindToken(tree, NodeKind.Ident, "b"); + Assert.Equal(aFlagsAfter, aRedo.Green.Flags); + Assert.Equal(xFlagsAfter, xRedo.Green.Flags); + Assert.Equal(bFlagsAfter, bRedo.Green.Flags); + Assert.Equal(rootFlagsAfter, tree.GreenRoot.Flags); + } + [Fact] public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index 6dd7147..0935bf1 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -116,12 +116,12 @@ Acceptance criteria: ## Phase 5 — Undo/Redo Leaf-Level Flag Restoration -- [ ] Undo/Redo restores *leaf* boundary flags, not just root flags - - [ ] Start with a tree where a target leaf’s flags are known - - [ ] Commit an edit that changes boundary/contains flags (e.g., insert `X\n`) - - [ ] Capture the leaf flags before/after - - [ ] Undo and assert leaf flags are restored - - [ ] Redo and assert leaf flags match the mutated state +- [x] Undo/Redo restores *leaf* boundary flags, not just root flags + - [x] Start with a tree where a target leaf’s flags are known + - [x] Commit an edit that changes boundary/contains flags (e.g., insert `X\n`) + - [x] Capture the leaf flags before/after + - [x] Undo and assert leaf flags are restored + - [x] Redo and assert leaf flags match the mutated state Acceptance criteria: - Undo/Redo correctness is verified at the leaf level. From 99378253e61c07eb34bf49183058d534fe8b2580 Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 03:37:37 -0800 Subject: [PATCH 29/30] [phase 6]: Schema/Rebind Interaction --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 98 +++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 14 ++-- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 2864eaa..1ad9e0e 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1954,6 +1954,104 @@ public void UndoRedo_RestoresLeafBoundaryFlags_NotJustRootFlags() Assert.Equal(rootFlagsAfter, tree.GreenRoot.Flags); } + [Fact] + public void SchemaRebind_ReplaceInsideSyntaxNode_PreservesLeafBoundaryFlags_AndKeepsSyntaxContainersBoundaryFree() + { + const GreenNodeFlags boundaryMask = + GreenNodeFlags.HasLeadingNewlineTrivia | + GreenNodeFlags.HasTrailingNewlineTrivia | + GreenNodeFlags.HasLeadingWhitespaceTrivia | + GreenNodeFlags.HasTrailingWhitespaceTrivia | + GreenNodeFlags.HasLeadingCommentTrivia | + GreenNodeFlags.HasTrailingCommentTrivia; + + var schema = Schema.Create() + .WithCommentStyles(CommentStyle.CStyleSingleLine) + .WithTagPrefixes('@') + .DefineSyntax(Syntax.Define("testTagged") + .Match(Q.AnyTaggedIdent, Q.AnyString) + .Build()) + .Build(); + + var tree = SyntaxTree.Parse("before\n @tag \"value\" // c\nnext", schema); + + var syntaxBefore = Assert.Single(tree.Select(Q.Syntax()).OfType()); + AssertNotHasFlags(syntaxBefore.Green.Flags, boundaryMask); + + var tagBefore = FindToken(tree, NodeKind.TaggedIdent, "@tag"); + var valueBefore = FindToken(tree, NodeKind.String, "\"value\""); + var nextBefore = FindToken(tree, NodeKind.Ident, "next"); + + // Indentation is leading whitespace on the first token inside the syntax node. + AssertHasFlags(tagBefore.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + + // The string token owns same-line comment + newline. + AssertHasFlags(valueBefore.Green.Flags, GreenNodeFlags.HasTrailingCommentTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(nextBefore.Green.Flags, GreenNodeFlags.HasLeadingCommentTrivia | GreenNodeFlags.HasLeadingNewlineTrivia); + + tree.CreateEditor() + .Replace(Q.String("\"value\""), "\"X\"") + .Commit(); + + var syntaxAfter = Assert.Single(tree.Select(Q.Syntax()).OfType()); + AssertNotHasFlags(syntaxAfter.Green.Flags, boundaryMask); + + var tagAfter = FindToken(tree, NodeKind.TaggedIdent, "@tag"); + var xAfter = FindToken(tree, NodeKind.String, "\"X\""); + var nextAfter = FindToken(tree, NodeKind.Ident, "next"); + + AssertHasFlags(tagAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingCommentTrivia | GreenNodeFlags.HasTrailingNewlineTrivia); + AssertNotHasFlags(nextAfter.Green.Flags, GreenNodeFlags.HasLeadingCommentTrivia | GreenNodeFlags.HasLeadingNewlineTrivia); + + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsCommentTrivia | GreenNodeFlags.ContainsNewlineTrivia); + } + + [Fact] + public void SchemaRebind_InsertBeforeSyntaxNode_DoesNotStealLeadingWhitespace_AndKeepsSyntaxContainersBoundaryFree() + { + const GreenNodeFlags boundaryMask = + GreenNodeFlags.HasLeadingNewlineTrivia | + GreenNodeFlags.HasTrailingNewlineTrivia | + GreenNodeFlags.HasLeadingWhitespaceTrivia | + GreenNodeFlags.HasTrailingWhitespaceTrivia | + GreenNodeFlags.HasLeadingCommentTrivia | + GreenNodeFlags.HasTrailingCommentTrivia; + + var schema = Schema.Create() + .WithTagPrefixes('@') + .DefineSyntax(Syntax.Define("testTagged") + .Match(Q.AnyTaggedIdent, Q.AnyString) + .Build()) + .Build(); + + var tree = SyntaxTree.Parse("before\n @tag \"value\"\nafter", schema); + + var syntaxBefore = Assert.Single(tree.Select(Q.Syntax()).OfType()); + AssertNotHasFlags(syntaxBefore.Green.Flags, boundaryMask); + + var tagBefore = FindToken(tree, NodeKind.TaggedIdent, "@tag"); + AssertHasFlags(tagBefore.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + + tree.CreateEditor() + .InsertBefore(Q.Syntax(), "X\n") + .Commit(); + + var syntaxAfter = Assert.Single(tree.Select(Q.Syntax()).OfType()); + AssertNotHasFlags(syntaxAfter.Green.Flags, boundaryMask); + + var xAfter = FindToken(tree, NodeKind.Ident, "X"); + var tagAfter = FindToken(tree, NodeKind.TaggedIdent, "@tag"); + var afterAfter = FindToken(tree, NodeKind.Ident, "after"); + + // Inserted token owns its trailing newline; syntax node's first token keeps its indentation. + AssertHasFlags(xAfter.Green.Flags, GreenNodeFlags.HasTrailingNewlineTrivia); + AssertHasFlags(tagAfter.Green.Flags, GreenNodeFlags.HasLeadingWhitespaceTrivia); + AssertNotHasFlags(afterAfter.Green.Flags, GreenNodeFlags.HasLeadingNewlineTrivia); + + AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsNewlineTrivia); + } + [Fact] public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index 0935bf1..d7063f8 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -130,13 +130,13 @@ Acceptance criteria: ## Phase 6 — Schema/Rebind Interaction (Syntax Binding) -- [ ] Ensure edits with schema + syntax definitions do not violate token-centric boundary semantics - - [ ] Parse with a schema that includes syntax definitions (so `Commit()` triggers `RebindAt`) - - [ ] Perform insert/replace/remove inside or around bound syntax nodes - - [ ] Assert: - - [ ] leaf boundary flags are correct - - [ ] syntax containers do NOT have `GreenNodeFlagMasks.Boundary` - - [ ] contains flags reflect subtree +- [x] Ensure edits with schema + syntax definitions do not violate token-centric boundary semantics + - [x] Parse with a schema that includes syntax definitions (so `Commit()` triggers `RebindAt`) + - [x] Perform insert/replace/remove inside or around bound syntax nodes + - [x] Assert: + - [x] leaf boundary flags are correct + - [x] syntax containers do NOT have `GreenNodeFlagMasks.Boundary` + - [x] contains flags reflect subtree Acceptance criteria: - Rebinding does not break boundary/contains invariants. From 17451c8f9ca636868184686befb67e92cbadb32e Mon Sep 17 00:00:00 2001 From: David Sisco Date: Sun, 11 Jan 2026 04:09:12 -0800 Subject: [PATCH 30/30] [phase 7]: Oracle-Style Regression Test --- TinyTokenizer.Tests/SyntaxEditorTests.cs | 151 ++++++++++++++++++++++ syntaxeditor-flag-mutation-test-gaps.todo | 14 +- 2 files changed, 158 insertions(+), 7 deletions(-) diff --git a/TinyTokenizer.Tests/SyntaxEditorTests.cs b/TinyTokenizer.Tests/SyntaxEditorTests.cs index 1ad9e0e..eaa9a68 100644 --- a/TinyTokenizer.Tests/SyntaxEditorTests.cs +++ b/TinyTokenizer.Tests/SyntaxEditorTests.cs @@ -1645,6 +1645,31 @@ private static SyntaxToken FindToken(SyntaxTree tree, NodeKind kind, string text return matches[occurrence]; } + private static void AssertReparseOracleFlagsMatch(SyntaxTree tree, SyntaxTree oracle) + { + ArgumentNullException.ThrowIfNull(tree); + ArgumentNullException.ThrowIfNull(oracle); + + Assert.Equal(oracle.GreenRoot.Flags, tree.GreenRoot.Flags); + + var treeLeaves = tree.Leaves.ToList(); + var oracleLeaves = oracle.Leaves.ToList(); + Assert.Equal(oracleLeaves.Count, treeLeaves.Count); + + for (int i = 0; i < treeLeaves.Count; i++) + { + Assert.Equal(oracleLeaves[i].Kind, treeLeaves[i].Kind); + Assert.Equal(oracleLeaves[i].Text, treeLeaves[i].Text); + + var expected = oracleLeaves[i].Green.Flags; + var actual = treeLeaves[i].Green.Flags; + Assert.True( + expected == actual, + $"Leaf flags mismatch at index {i}: {treeLeaves[i].Kind} '{treeLeaves[i].Text}'. Expected={expected} Actual={actual}" + ); + } + } + [Fact] public void Replace_OwnLineCommentLeadingTrivia_PreservesGreenBoundaryFlags_OnReplacement() { @@ -2052,6 +2077,132 @@ public void SchemaRebind_InsertBeforeSyntaxNode_DoesNotStealLeadingWhitespace_An AssertHasFlags(tree.GreenRoot.Flags, GreenNodeFlags.ContainsWhitespaceTrivia | GreenNodeFlags.ContainsNewlineTrivia); } + [Fact] + public void ReparseOracle_AfterComplexEdit_MatchesRootAndLeafFlags_WithOptions() + { + var options = TokenizerOptions.Default + .WithCommentStyles(CommentStyle.CStyleSingleLine, CommentStyle.CStyleMultiLine); + + const string source = "a{\n b // c1\n d(e) /* c2 */\n}\n"; + var tree = SyntaxTree.Parse(source, options); + + tree.CreateEditor(options) + .Replace(Q.Ident("b"), "{x}") + .InsertAfter(Q.Ident("d"), ".Y\n") + .Commit(); + + var editedText = tree.ToText(); + var oracle = SyntaxTree.Parse(editedText, options); + + AssertReparseOracleFlagsMatch(tree, oracle); + } + + [Fact] + public void ReparseOracle_AfterComplexEdit_MatchesRootAndLeafFlags_WithSchemaAndBinding() + { + var schema = Schema.Create() + .WithCommentStyles(CommentStyle.CStyleSingleLine, CommentStyle.CStyleMultiLine) + .WithTagPrefixes('@') + .DefineSyntax(Syntax.Define("testTagged") + .Match(Q.AnyTaggedIdent, Q.AnyString) + .Build()) + .Build(); + + var tree = SyntaxTree.Parse("before\n @tag \"value\" // c\nafter", schema); + + tree.CreateEditor() + .InsertBefore(Q.Syntax(), "X\n") + .Replace(Q.String("\"value\""), "\"Z\"") + .Commit(); + + var editedText = tree.ToText(); + var oracle = SyntaxTree.Parse(editedText, schema); + + AssertReparseOracleFlagsMatch(tree, oracle); + } + + [Fact] + public void ReparseOracle_AfterRemoveReplaceAndCRLFInsert_MatchesRootAndLeafFlags_WithOptions() + { + var options = TokenizerOptions.Default + .WithCommentStyles(CommentStyle.CStyleSingleLine, CommentStyle.CStyleMultiLine); + + const string source = "a // c1\r\n b /* c2 */\r\nc d"; + var tree = SyntaxTree.Parse(source, options); + + tree.CreateEditor(options) + // Remove token that owns trailing comment+newline + .Remove(Q.Ident("a")) + // Replace a token that owns leading indentation + .Replace(Q.Ident("b"), "{x}") + // Insert a token that owns CRLF; avoid leading trivia by starting with a symbol + .InsertAfter(Q.Ident("c"), ".Y\r\n") + .Commit(); + + var editedText = tree.ToText(); + var oracle = SyntaxTree.Parse(editedText, options); + + AssertReparseOracleFlagsMatch(tree, oracle); + } + + [Fact] + public void ReparseOracle_AfterEditsOnMultipleSyntaxNodes_MatchesRootAndLeafFlags_WithSchemaAndBinding() + { + var schema = Schema.Create() + .WithCommentStyles(CommentStyle.CStyleSingleLine) + .WithTagPrefixes('@') + .DefineSyntax(Syntax.Define("testTagged") + .Match(Q.AnyTaggedIdent, Q.AnyString) + .Build()) + .DefineSyntax(Syntax.Define("funcCall") + .Match(Q.AnyIdent, Q.ParenBlock) + .Build()) + .Build(); + + var tree = SyntaxTree.Parse("@tag \"value\"\nfoo(a, b)\n", schema); + + tree.CreateEditor() + .Replace(Q.String("\"value\""), "\"Z\"") + // Insert inside the function call argument list. Start with a symbol to avoid leading trivia. + .InsertAfter(Q.Ident("a"), ",c") + .Commit(); + + var editedText = tree.ToText(); + var oracle = SyntaxTree.Parse(editedText, schema); + + AssertReparseOracleFlagsMatch(tree, oracle); + } + + [Fact] + public void ReparseOracle_AfterDeeplyNestedBlockEdits_MatchesRootAndLeafFlags_WithOptions() + { + var options = TokenizerOptions.Default + .WithCommentStyles(CommentStyle.CStyleSingleLine, CommentStyle.CStyleMultiLine); + + const string source = + "root {\n" + + " a(b[c{d(e)}]) // c1\n" + + " { x /* c2 */ }\n" + + "}\n"; + + var tree = SyntaxTree.Parse(source, options); + + // Perform multiple edits inside deeply nested structures. + tree.CreateEditor(options) + // Replace an inner identifier. + .Replace(Q.Ident("d"), "D") + // Insert inside the deepest paren. Start with a symbol to avoid leading-trivia ambiguity. + .InsertAfter(Q.Ident("e"), ".Y\n") + // Replace an identifier with a block to exercise block-edge trivia semantics. + .Replace(Q.Ident("x"), "{z}") + .Commit(); + + var editedText = tree.ToText(); + var oracle = SyntaxTree.Parse(editedText, options); + + AssertReparseOracleFlagsMatch(tree, oracle); + } + [Fact] public void InsertAfter_BlockContainsNewlineFlag_UpdatesAfterMutation() { diff --git a/syntaxeditor-flag-mutation-test-gaps.todo b/syntaxeditor-flag-mutation-test-gaps.todo index d7063f8..572680d 100644 --- a/syntaxeditor-flag-mutation-test-gaps.todo +++ b/syntaxeditor-flag-mutation-test-gaps.todo @@ -145,13 +145,13 @@ Acceptance criteria: ## Phase 7 — Oracle-Style Regression Test (Optional “Big Hammer”) -- [ ] Add a “reparse oracle” test for flags - - [ ] After an edit, compute `editedText = tree.ToText()` - - [ ] Parse a fresh `oracle = SyntaxTree.Parse(editedText, sameOptionsOrSchema)` - - [ ] Compare: - - [ ] `tree.GreenRoot.Flags == oracle.GreenRoot.Flags` - - [ ] Per-leaf flags match for corresponding leaves (by `(Kind, Text, occurrence index)` or by positions) - - [ ] Use this for 1–2 representative complex edits (nested blocks + comments) +- [x] Add a “reparse oracle” test for flags + - [x] After an edit, compute `editedText = tree.ToText()` + - [x] Parse a fresh `oracle = SyntaxTree.Parse(editedText, sameOptionsOrSchema)` + - [x] Compare: + - [x] `tree.GreenRoot.Flags == oracle.GreenRoot.Flags` + - [x] Per-leaf flags match for corresponding leaves (by `(Kind, Text, occurrence index)` or by positions) + - [x] Use this for 1–2 representative complex edits (nested blocks + comments) Acceptance criteria: - A broad regression net exists that catches stale/miscomputed flags without enumerating every case.