Fix duplicate and crashing "find all references" results - #1791
Open
TwitchBronBron wants to merge 1 commit into
Open
Fix duplicate and crashing "find all references" results#1791TwitchBronBron wants to merge 1 commit into
TwitchBronBron wants to merge 1 commit into
Conversation
`ReferencesProvider` declared its `processedFiles` set inside the scope loop, so a file reachable through multiple scopes was walked once per scope. Since the search is a case-insensitive text match against the file's AST, each of those walks produced identical `Location`s, and nothing between the provider and the LSP client dedupes them — a file included by 3 components showed every reference 3 times in the references panel. Hoisting the set keeps each file walked once across the whole traversal; references only reachable through a single scope are still found, because the set keys on file rather than scope. `getTokenAt` returns undefined when there is no token at the position (e.g. the cursor is past the end of a line), which made the following `.text` access throw. The plugin runner swallowed it, so the user silently got no references. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two bugs in
ReferencesProvider, both user-visible in the VSCode references panel today.Duplicate results.
processedFileswas declared inside the scope loop, so a file reachable through multiple scopes got re-walked once per scope. The search is a case-insensitive text match over the file AST, so every re-walk emitted identicalLocations, and no layer between the provider and the LSP client dedupes them — asource/file included by 3 components showed each reference 3× in the panel. Hoisting the set walks each file once across the whole traversal.Worth noting since it looks like it might over-dedupe: references reachable through only one scope are still found, because the set keys on file, not scope. A test covers exactly that case (
alphareferenced from a file unique to scope A and another unique to scope B — both still returned).Crash on positions with no token.
getTokenAtreturns undefined when the cursor is past the end of a line, socallSiteToken.textthrew. The plugin runner swallowed the error and the user silently got no references.Tests: 11 new cases in
ReferencesProvider.spec.ts(both fixes, scope-specific references, case-insensitivity, params, XML, and the threeprovideReferencesplugin events, which had no coverage) plus one at theLanguageServer.onReferenceslevel asserting no duplicateLocations reach the client. Verified both fixes fail their tests when reverted.npm run test:nocover3048 → 3059 passing, lint andtsc --noEmitclean.🤖 Generated with Claude Code