Fix/diagnostics scanprocedures class member leak - #392
Open
geircodes wants to merge 2 commits into
Open
Conversation
…mber prototypes from the bare-name procedure index A column-0 `Name PROCEDURE/FUNCTION` line inside a CLASS/INTERFACE body (a common, legal Clarion style — member labels written unindented, same shape as a real top-level declaration) indexed identically to a global procedure. SymbolFinderService.findProcedureViaIndex trusts that index unconditionally, so a bare-word lookup for the member's name could resolve to it from anywhere in the solution — including the undeclared-variable diagnostic's cross-file augmentation silently treating a genuinely undeclared name as declared. Same defect class fixed for hover in msarson#391 (MemberLocatorService .isVariableLookupCandidate), but that fix never touched this separate, tokenizer-free scanner. Tracks CLASS/INTERFACE/QUEUE/GROUP/RECORD/FILE/VIEW nesting as a stack, mirroring the existing mapDepth tracking for MAP/MODULE, and skips indexing a column-0 PROCEDURE/FUNCTION line while a CLASS or INTERFACE is open. Other structure kinds are tracked too, purely so an inline nested structure's own END (e.g. a GROUP data member declared inside a class) pops correctly instead of prematurely closing the enclosing CLASS. Bumps DISK_CACHE_VERSION 3->4 — the on-disk SDI cache is keyed on file mtime, not scanner semantics, so a code-only fix would leave stale bad entries served indefinitely post-deploy.
…e bare-name procedure index Pins the shape that triggers the bug (an unindented CLASS body with a column-0 DoWork PROCEDURE(...) member, plus an INTERFACE variant), and a nested-GROUP-inside-CLASS case confirming the new structureStack pop is innermost-first so an inline nested structure's own END doesn't prematurely close the enclosing CLASS. Both assert a real global procedure declared after the CLASS/INTERFACE closes is still indexed correctly.
geircodes
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(diagnostics): scanSourceForProcedures excludes CLASS/INTERFACE members from the bare-name procedure index
What happened
The undeclared-variable diagnostic can silently fail to fire on a genuinely undeclared bare word, when an unrelated CLASS or INTERFACE elsewhere in the solution happens to declare a member with the same name.
Repro (see the new tests in
ScanSourceForProcedures362.test.ts): a.incfile declaresSomeClass CLASS,TYPE ... DoWork PROCEDURE(*?) ... END, written unindented (member labels at column 0 — a common, legal Clarion style). A bare, genuinely undeclaredDoWorkanywhere else in the solution resolves viaSymbolFinderService.findProcedureViaIndexto this class member, so the undeclared-variable diagnostic's cross-file augmentation treats it as declared and never warns.Root cause
StructureDeclarationIndexer.scanSourceForProcedures— the lightweight regex scanner that builds the "which file declares procedure X" index consumed bySymbolFinderService.findProcedureViaIndex(and so by hover/F12's fast path too) — indexes any column-0Name PROCEDURE/FUNCTIONline as a global procedure, with no CLASS/INTERFACE containment awareness. It already tracks MAP/MODULE nesting (mapDepth) so shorthand prototypes inside those contexts aren't double-counted, but has no equivalent for CLASS/INTERFACE.Same defect class as #391 (
MemberLocatorService.isVariableLookupCandidate, fixed for hover) — a bare word resolving to an unrelated class member — but a separate, tokenizer-free scanning code path that fix never touched.Fix
Tracks currently-open TYPE_PATTERN structures (CLASS/INTERFACE/QUEUE/GROUP/RECORD/FILE/VIEW) as a stack, mirroring the existing
mapDepthpattern. Skips indexing a column-0 PROCEDURE/FUNCTION line while a CLASS or INTERFACE is anywhere on the stack. Non-CLASS/INTERFACE kinds are tracked too, purely so an inline nested structure's ownEND(e.g. a GROUP data member declared inside a class) pops correctly instead of prematurely closing the enclosing CLASS.Bumps
DISK_CACHE_VERSION3→4 — the on-disk SDI cache is keyed on file mtime, not scanner semantics, so a code-only fix would leave stale bad entries served indefinitely after an upgrade.Testing
New tests in
ScanSourceForProcedures362.test.ts: the triggering shape (unindented CLASS + INTERFACE member prototypes, confirming they're excluded while a genuine global procedure declared after the closingENDis still indexed), and a nested-GROUP-inside-CLASS case confirming the stack pop is innermost-first.npx tsc -b: clean. Full suite (isolated worktree offorigin/version-1.0.1): 2342 passing, 4 pending (pre-existing), 0 failing.Scope
Two files changed, both in this PR:
server/src/utils/StructureDeclarationIndexer.ts,server/src/test/ScanSourceForProcedures362.test.ts. Plain scope-resolution bug in a shared cross-file index, unrelated to any in-flight feature work, so it goes out as its own PR.