fix(document-symbols): unbounded MAP/MODULE scan corrupting the outline - #382
Open
geircodes wants to merge 2 commits into
Open
fix(document-symbols): unbounded MAP/MODULE scan corrupting the outline#382geircodes wants to merge 2 commits into
geircodes wants to merge 2 commits into
Conversation
…tions handleStructureToken() computed endsOnSameLine for structures that open and close on one line (e.g. `fq QUEUE(FILE:Queue) END`) and returned early — before reaching the code further down that reads the preceding same-line token and assigns token.label. Multi-line structures worked fine; any single-line QUEUE/GROUP/etc. never got its declared name attached, so the outline showed a bare "QUEUE" node instead of "QUEUE (fq)". Hoisted the label-assignment block (including the msarson#65 LOOP/ACCEPT special case) to run before the endsOnSameLine check, and removed the now-dead duplicate copy that used to sit after the early return.
…dant "in X" detail text
Two independent outline bugs found while auditing a WINDOW procedure's
document symbols:
1. The MAP/MODULE shorthand-procedure lookahead accepted an EndStatement as
the structure's true end only if no Structure token had been seen since —
but a MODULE nested inside a MAP is itself a Structure token, so that
condition could never become true again once one appeared, and the scan
ran unbounded through the rest of the file. Any later identifier
immediately followed by "(" (e.g. WINDOW attributes like FONT(...),
VALUE(...), FROM(...)) got misclassified as a MAP/MODULE procedure and
surfaced as a stray top-level outline entry.
Fix: bound the lookahead with the structure's own finishesAt, which the
tokenizer already computes with correct nesting depth (and which the rest
of the codebase, e.g. MapProcedureResolver, already relies on) — instead
of re-deriving the end with the broken heuristic.
2. Every variable/method symbol got a `detail` of "in <ParentName>", which
duplicates what's already shown by the tree hierarchy (both in this
provider's own Monaco-based outline and VS Code's native Outline view),
and is redundant even for its originally-intended breadcrumb-dropdown use
case (a dropdown only ever lists siblings under one already-visible
parent). WorkspaceSymbolProvider's flattened cross-file search — the one
place where parent context actually matters — already derives its own
containerName from the symbol tree and never reads `detail`.
Disabled (commented out, not deleted, for a cheap revert) the three call
sites that set this text.
Test plan:
- npx tsc -b — clean compile after each commit
- Full test suite: 2340 passing, 0 failing, 4 pending (pre-existing,
unrelated) — re-verified after every change in this PR
- Verified directly against a real production .clw file: no stray
FONT/VALUE/FROM entries, no bare QUEUE nodes, no "in X" detail text
anywhere in the resulting symbol tree
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three document-symbol (outline) bugs found while auditing a real production
.clwfile'sDocument Structure (outline)panel for aWINDOWprocedure — one of them significant beyond just that outline, since it silently corrupted symbol classification for the rest of the file once triggered.ClarionDocumentSymbolProvider.ts— unbounded MAP/MODULE lookahead. When the parser hits aMAP/MODULEstructure, it scans forward to find shorthand procedure declarations, accepting anEndStatementas the structure's true end only if noStructuretoken had been seen since. AMODULE(...)nested inside aMAPis itself aStructuretoken, so once one appeared, that condition could never become true again — the scan ran unbounded through the rest of the file. Any later identifier immediately followed by((e.g.WINDOWattributes likeFONT(...),VALUE(...),FROM(...)) got misclassified as a MAP/MODULE procedure and surfaced as a stray top-level outline entry, for every symbol after that point in the file — not just inside the offendingWINDOW.Fix: bound the lookahead with the structure's own
finishesAt, which the tokenizer already computes with correct nesting depth and which the rest of the codebase (e.g.MapProcedureResolver) already relies on — instead of re-deriving the end with the broken heuristic.DocumentStructure.ts— single-line structure declarations never got a label.handleStructureToken()computesendsOnSameLinefor structures that open and close on one line (e.g.fq QUEUE(FILE:Queue) END) and returns early — before reaching the code further down that reads the preceding same-line token and assignstoken.label. Multi-line structures worked fine; any single-lineQUEUE/GROUP/etc. never got its declared name attached, so the outline showed a bare"QUEUE"node instead of"QUEUE (fq)".Fix: hoisted the label-assignment block (including the Language support: labeled LOOP and BREAK/CYCLE with label target #65 LOOP/ACCEPT special case) to run before the
endsOnSameLinecheck, and removed the now-dead duplicate copy that used to sit after the early return.ClarionDocumentSymbolProvider.ts— redundant"in <Parent>"detail text. Every variable/method symbol got adetailof"in <ParentName>". This duplicates what's already shown by the tree hierarchy (both this provider's own outline consumers and VS Code's native Outline view), and is redundant even for its originally-intended breadcrumb-dropdown use case — a breadcrumb dropdown only ever lists siblings under one already-visible parent, so repeating that parent's name on every row adds nothing.WorkspaceSymbolProvider's flattened cross-file search — the one place parent context actually matters for disambiguation — already derives its owncontainerNamefrom the symbol tree and never readsdetail.Disabled (commented out, not deleted, for a cheap revert if some consumer turns out to need it) the three call sites that set this text.
Checked git history before writing (1) and (2): both bugs were introduced together, from scratch, in the single commit that created
DocumentStructure.ts(#280) — not a regression of previously-correct behavior, and no existing helper elsewhere in the codebase does the same job that could have been reused instead.Test plan
npx tsc -b— clean compile after each commit.clwfile (tokenizer + provider run outside the test suite): no strayFONT/VALUE/FROMentries, no bareQUEUEnodes, no"in X"detail text anywhere in the resulting symbol tree (1488 symbols checked)🤖 Generated with Claude Code