Skip to content

Fix inverted condition for module references in findallreferences #1780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Fix inverted condition for module references in findallreferences #1780

wants to merge 6 commits into from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 1, 2025 •
edited
Loading

  • Understand the issue: panic when highlighting import paths with module symbols
  • Trace through the code to find the root cause
  • Identify the bug: inverted condition in getReferencedSymbolsForNode (line 676)
  • Fix the inverted condition to match TypeScript source
  • Implement minimal getReferencedSymbolsForModule to make the fix testable
  • Create test that reproduces the issue (hangs with wrong condition)
  • Verify the fix prevents the crash and makes highlights work
  • Run all tests - they pass
  • Format and lint the code
Original prompt

This section details on the original issue you should resolve

<issue_title>Request textDocument/documentHighlight failed</issue_title>
<issue_description>## Stack trace

[Error - 17:43:57] Request textDocument/documentHighlight failed.
  Message: InternalError: panic handling request textDocument/documentHighlight: Unexpected symbol at KindStringLiteral: "/.../node_modules/@.../dist/index"
  Code: -32603 
panic handling request textDocument/documentHighlight Unexpected symbol at KindStringLiteral: "/.../node_modules/.../dist/index" goroutine 17247 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:26 +0x64
github.com/microsoft/typescript-go/internal/lsp.(*Server).recover(0x14000000f00, 0x14004b52b70)
	github.com/microsoft/typescript-go/internal/lsp/server.go:541 +0x44
panic({0x102e58820?, 0x14005ce9280?})
	runtime/panic.go:783 +0x120
github.com/microsoft/typescript-go/internal/ls.skipPastExportOrImportSpecifierOrUnion.func1(...)
	github.com/microsoft/typescript-go/internal/ls/findallreferences.go:327
github.com/microsoft/typescript-go/internal/core.FirstNonNil[...](...)
	github.com/microsoft/typescript-go/internal/core/core.go:260
github.com/microsoft/typescript-go/internal/ls.skipPastExportOrImportSpecifierOrUnion(0x14017d28b60, 0x14000651570, 0x14019745308, 0x70?)
	github.com/microsoft/typescript-go/internal/ls/findallreferences.go:320 +0x1d0
github.com/microsoft/typescript-go/internal/ls.getReferencedSymbolsForSymbol(0x14017d28b60, 0x14000651570, {0x14019d52d00, 0x1, 0x1}, 0x14019d52d48, 0x14019745308, {0x0, 0x0, 0x2, ...})
	github.com/microsoft/typescript-go/internal/ls/findallreferences.go:1036 +0x84
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getReferencedSymbolsForNode(0x140148c6930, {0x10306e050, 0x14015f45360}, 0x371, 0x14000651570, 0x1400075cf00, {0x14019d52d00, 0x1, 0x1}, {0x0, ...}, ...)
	github.com/microsoft/typescript-go/internal/ls/findallreferences.go:683 +0x59c
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getSemanticDocumentHighlights(0x140148c6930, {0x10306e050, 0x14015f45360}, 0x371, 0x14000651570, 0x1400075cf00, 0x14001124908)
	github.com/microsoft/typescript-go/internal/ls/documenthighlights.go:55 +0xc4
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).ProvideDocumentHighlights(0x140148c6930, {0x10306e050, 0x14015f45360}, {0x14013f6ad20?, 0x1401582c6c0?}, {0x151d7208?, 0x140?})
	github.com/microsoft/typescript-go/internal/ls/documenthighlights.go:45 +0x34c
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleDocumentHighlight(0x140001662a0?, {0x10306e050?, 0x14015f45360?}, 0x14013f6ad20?, 0x51?)
	github.com/microsoft/typescript-go/internal/lsp/server.go:844 +0x38
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerLanguageServiceDocumentRequestHandler[...].24({0x10306e050, 0x14015f45360}, 0x14004b52b70)
	github.com/microsoft/typescript-go/internal/lsp/server.go:527 +0xe0
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification(0x14000000f00, {0x10306e050, 0x14015f45360}, 0x14004b52b70)
	github.com/microsoft/typescript-go/internal/lsp/server.go:424 +0xf4
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
	github.com/microsoft/typescript-go/internal/lsp/server.go:329 +0x34
created by github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop in goroutine 34
	github.com/microsoft/typescript-go/internal/lsp/server.go:349 +0x6f8

Steps to reproduce

  1. Focus on path of any import statement.</issue_description>

<agent_instructions>Find a crasher first. It should be something like a doc highlight test that highlights inside an import. See existing tests from #1699</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #1772

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits October 1, 2025 14:47
@Copilot @jakebailey
Initial analysis and test case for document highlight crash
51d5fe5 
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
@Copilot @jakebailey
Fix inverted condition for module references in document highlights
6e12a02 
The condition checking whether to return early for module references was inverted.
The TypeScript implementation returns early when the symbol is NOT transient,
but the Go code was returning early only when it IS transient.

This caused module symbols (whose declarations are SourceFiles with no parent)
to be passed to getReferencedSymbolsForSymbol, which then called
skipPastExportOrImportSpecifierOrUnion. That function would panic when
encountering a declaration without a parent for a non-transient symbol.

The fix aligns the Go implementation with the TypeScript source by checking
for `== 0` instead of `!= 0` on the transient flag.

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Request textDocument/documentHighlight failed Fix panic in textDocument/documentHighlight when highlighting import paths Oct 1, 2025
@Copilot Copilot AI requested a review from jakebailey October 1, 2025 14:57
Copilot finished work on behalf of jakebailey October 1, 2025 14:57
@Copilot @jakebailey
Move documentHighlightImportPath_test.go to parent directory
d0dfb33 
The test should be in internal/fourslash/tests/ instead of the gen/ subdirectory,
as the gen/ directory is only for generated tests.

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Copilot finished work on behalf of jakebailey October 1, 2025 15:13
@Copilot Copilot AI requested a review from jakebailey October 1, 2025 15:13
@Copilot @jakebailey
Remove test that doesn't reproduce the crash
96e16e3 
The test cannot reproduce the crash because getReferencedSymbolsForModule is not
yet implemented. The fix is correct (matches TypeScript source), but cannot be
verified with a test until that function is implemented.

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
@Copilot Copilot AI changed the title Fix panic in textDocument/documentHighlight when highlighting import paths Fix inverted condition for module references in findallreferences Oct 1, 2025
@Copilot Copilot AI requested a review from jakebailey October 1, 2025 17:37
Copilot finished work on behalf of jakebailey October 1, 2025 17:37
@Copilot @jakebailey
Implement minimal getReferencedSymbolsForModule to fix crash
e3d8249 
Implemented a minimal version of getReferencedSymbolsForModule that returns
module declarations as references. This, combined with the corrected condition
(checking for non-transient instead of transient), prevents the crash when
highlighting import paths.

The crash occurred because:
1. GetSymbolAtLocation on an import string returns a module symbol
2. getReferencedSymbolsForModuleIfDeclaredBySourceFile is called and now returns results
3. The old condition checked for transient symbols, causing non-transient module
   symbols to fall through to getReferencedSymbolsForSymbol
4. That function calls skipPastExportOrImportSpecifierOrUnion which panics on
   SourceFile declarations without parents

The fix ensures non-transient module symbols return early with the module references,
preventing the panic.

Added test TestDocumentHighlightImportPath that verifies document highlights work
on import paths and would hang/crash with the old condition.

Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Copilot finished work on behalf of jakebailey October 1, 2025 18:01
@jakebailey jakebailey mentioned this pull request Oct 2, 2025
Closed
@jakebailey jakebailey closed this Oct 3, 2025
@jakebailey jakebailey mentioned this pull request Oct 3, 2025
Merged
@jakebailey jakebailey deleted the copilot/fix-84b4b1d4-9690-4068-ac83-3adf725dd03b branch October 6, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakebailey jakebailey Awaiting requested review from jakebailey

Assignees

@jakebailey jakebailey

Copilot code review Copilot

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request textDocument/documentHighlight failed
2 participants
@Copilot @jakebailey