Implement more of doc highlights / find all refs #1796

jakebailey · 2025-10-03T02:06:42Z

Iterates on #1780

This is largely agent mode produced but it seems fine.

Co-authored-by: jakebailey <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

Copilot

Pull Request Overview

This PR implements more comprehensive support for document highlights and find all references functionality, particularly for module references. It adds support for tracking different types of module references (imports, references, implicit references) and enables several previously skipped tests.

Adds ModuleReference type and findModuleReferences function to track module imports, reference directives, and implicit references
Implements getReferencedSymbolsForModule to handle module symbol references comprehensively
Enables multiple previously skipped fourslash tests by removing t.Skip() calls

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/ls/importTracker.go	Adds ModuleReference types and findModuleReferences function for comprehensive module reference tracking
internal/ls/findallreferences.go	Implements getReferencedSymbolsForModule and fixes symbol flag condition
internal/ls/documenthighlights.go	Minor refactoring to use intermediate variable for response
internal/fourslash/tests/gen/*.go	Removes t.Skip() calls to enable previously disabled tests
internal/fourslash/tests/documentHighlightImportPath_test.go	Adds new test for document highlight on import paths
internal/fourslash/_scripts/failingTests.txt	Removes test names that are now passing
testdata/baselines/reference/fourslash/*.baseline.jsonc	Updated test baselines with expected outputs

internal/ls/findallreferences.go

jakebailey · 2025-10-03T02:58:36Z

This got sort of out of control as I got more ported, oops

gabritto · 2025-10-03T15:20:30Z

internal/fourslash/tests/documentHighlightImportPath_test.go

@@ -0,0 +1,21 @@
+package fourslash_test


Does this test repro the crash?

Wondering if this still applies:

#1796 does fix it, though I have yet to make a test which actually shows the crash and not just an empty response.

Sort of. It returns a nil location which fails the test runner. I still could not get a good test...

gabritto · 2025-10-03T15:42:43Z

...nce/fourslash/findAllReferences/jsxFindAllReferencesOnRuntimeImportWithPaths1.baseline.jsonc

@@ -0,0 +1,3 @@
+// === findAllReferences ===


Not sure if this is relevant for this PR, but this baseline (and the tslibFindAllReferences... below) still looks like it's missing other references.

I have more code I could push which scaffolds more of this out; let me see if that (questionable) code works for this

My other changes don't affect this file, unfortunately...

internal/ls/documenthighlights.go

* Port "Infer from annotated return type nodes before assigning contextual parameter types" (microsoft#1804) * Port "make exported destructured discriminated union narrowing work" (microsoft#1806) Co-authored-by: =?UTF-8?q?Michal=20Mar=C5=A1=C3=A1lek?= <[email protected]> * Implement more of doc highlights / find all refs (microsoft#1796) Co-authored-by: copilot-swe-agent[bot] <[email protected]> * Port "Provide string completions for `in` keyword checks" (microsoft#1803) * Port "Fix #61098" (microsoft#1810) Co-authored-by: Hans Brende <[email protected]> * Port "fix(61258): Renaming namespace with const enum doesn't update enum references" (microsoft#1811) Co-authored-by: "Oleksandr T." <[email protected]> * Update dependencies (microsoft#1785) * Mark libReplacement default as false (microsoft#1812) * Port "Deemphasize old JSX transform" (microsoft#1809) Co-authored-by: "Sebastian \"Sebbie\" Silbermann" <[email protected]> * Port "fix(checker): report error when using bigint as enum key" (microsoft#1814) Co-authored-by: magic-akari <[email protected]> * Port "explicitly disallow `using` in ambient contexts" (microsoft#1815) Co-authored-by: =?UTF-8?q?Ren=C3=A9?= <[email protected]> * Port 'Issue "'{0}' declarations can only be declared inside a block." for block-scoped variables in presence of parse errors' (microsoft#1816) * Port "Avoid resolving source prop type when the target is `unknown`/`any`" (microsoft#1817) * Port "Allow trailing commas after import attributes in `ImportType`" (microsoft#1818) * Port "Fix BigInt literal error in ambient contexts when targeting < ES2020" (microsoft#1819) * Port "Stop reassigning `.valueDeclaration` to avoid replacing earlier declarations with late ones" (microsoft#1813) * Port "Keep accessors as accessors in emitted anonymous class declarations" (microsoft#1822) * Port "Fixed crash in `hasVisibleDeclarations` related to binding elements" (microsoft#1820) * Port "Properly disallow `yield` in bodyless arrows" (microsoft#1825) * Port "Account for right operands & fix a weird error message for leftmost nullish literals in checkNullishCoalesceOperands" (microsoft#1805) Co-authored-by: Chiri Vulpes <[email protected]> * Remove JSDoc handling from the binder (microsoft#1827) * Port "`arguments` should not be allowed in class static block" (microsoft#1828) Co-authored-by: Zzzen <[email protected]> * Port "check usage before declaration for decorators" (microsoft#1829) Co-authored-by: Zzzen <[email protected]> * Fix nil pointer dereference in snapshotFS.GetFile for non-existent files (microsoft#1830) * Fix translation bug causing `unknown` infer-extends constraint to be emitted (microsoft#1831) * Port "Fixed nullish coalesce operator's precedence" (microsoft#1824) * Fixed an issue with type params being renamed incorrectly (microsoft#1833) * Port "Add support for `import defer` proposal" (microsoft#1826) Co-authored-by: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= <[email protected]> * Bump github/codeql-action from 3.30.5 to 3.30.6 in the github-actions group across 1 directory (microsoft#1832) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Port "Preserve type parameter constraint in emitted mapped types while preserving their distributivity" (microsoft#1834) * disable create-cache ci job * merge fixup --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Mateusz Burzyński <[email protected]> Co-authored-by: =?UTF-8?q?Michal=20Mar=C5=A1=C3=A1lek?= <[email protected]> Co-authored-by: Jake Bailey <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Hans Brende <[email protected]> Co-authored-by: "Oleksandr T." <[email protected]> Co-authored-by: "Sebastian \"Sebbie\" Silbermann" <[email protected]> Co-authored-by: magic-akari <[email protected]> Co-authored-by: =?UTF-8?q?Ren=C3=A9?= <[email protected]> Co-authored-by: Chiri Vulpes <[email protected]> Co-authored-by: Zzzen <[email protected]> Co-authored-by: John Xu <[email protected]> Co-authored-by: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Copilot AI and others added 11 commits October 1, 2025 14:35

Initial plan

c206f00

Initial analysis and test case for document highlight crash

51d5fe5

Co-authored-by: jakebailey <[email protected]>

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 <[email protected]>

Merge branch 'main' into HEAD

ae28c22

revert

35c7b97

more

4980bf4

more

3250b31

redo files

c808430

jakebailey requested review from gabritto, iisaduan, johnfav03 and Copilot October 3, 2025 02:06

Copilot AI reviewed Oct 3, 2025

View reviewed changes

internal/ls/findallreferences.go Show resolved Hide resolved

jakebailey added 4 commits October 2, 2025 19:15

more maybe

5c98631

more

f6f429b

more

2bf56b7

remove unused I guess

17f2964

gabritto approved these changes Oct 3, 2025

View reviewed changes

jakebailey commented Oct 3, 2025

View reviewed changes

internal/ls/documenthighlights.go Outdated Show resolved Hide resolved

Just a bit more

ebb33b6

jakebailey requested a review from gabritto October 3, 2025 18:44

gabritto approved these changes Oct 3, 2025

View reviewed changes

jakebailey added this pull request to the merge queue Oct 3, 2025

Merged via the queue into main with commit 2a336d0 Oct 3, 2025
22 checks passed

jakebailey deleted the jabaile/my-1772 branch October 3, 2025 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement more of doc highlights / find all refs #1796

Implement more of doc highlights / find all refs #1796

jakebailey commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jakebailey commented Oct 3, 2025

Uh oh!

gabritto Oct 3, 2025

Uh oh!

gabritto Oct 3, 2025

Uh oh!

jakebailey Oct 3, 2025

Uh oh!

gabritto Oct 3, 2025

Uh oh!

jakebailey Oct 3, 2025

Uh oh!

jakebailey Oct 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Implement more of doc highlights / find all refs #1796

Implement more of doc highlights / find all refs #1796

Conversation

jakebailey commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

jakebailey commented Oct 3, 2025

Uh oh!

gabritto Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

gabritto Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

gabritto Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!