Document highlights #1699

johnfav03 · 2025-09-09T22:24:33Z

This PR implements provideDocumentHighlights and the corresponding fourslash tests/functions; the implementation covers syntactic highlights fully, but is missing several cases in semantic highlights due to unimplemented cases in getReferencedSymbolsForNode. Some prevalent examples from the failing test cases include:

export logic in getReferencedSymbolsForSymbol
- cases: getDocumentHighlightInExport, getDocumentHighlightInTypeExport`
getReferencesForStringLiteral in getReferencedSymbolsForNode
- cases: getDocumentHighlightTemplateStrings, getOccurrencesStringLiterals, getOccurrencesStringLiteralTypes
constructor logic in getReferencesAtLocation
- cases: getOccurencesClassExpressionConstructor, getOccurrencesConstructor, getOccurrencesConstructor2

johnfav03 · 2025-09-09T22:30:57Z

@johnfav03 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

Copilot

Pull Request Overview

This PR adds support for "Document highlights" functionality in the testing framework. Document highlights show related occurrences of symbols in a document when the user hovers or selects a symbol.

Adds a new baseline test type for documentHighlights test cases
Creates extensive test coverage for various TypeScript language constructs including classes, interfaces, keywords, modifiers, and control flow statements
Includes test cases for edge cases like inherited properties, parameter properties, and JSX elements

Reviewed Changes

Copilot reviewed 224 out of 224 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
testdata/baselines/reference/submodule/fourslash/findRenameLocations/renameDefaultImportDifferentName.baseline.jsonc.diff	Diff file showing baseline changes for rename functionality
testdata/baselines/reference/submodule/fourslash/findRenameLocations/renameDefaultImportDifferentName.baseline.jsonc	Baseline for testing rename operations on default imports with different names
testdata/baselines/reference/fourslash/findAllReferences/renameDefaultImportDifferentName.baseline.jsonc	Baseline for find all references functionality on default imports
testdata/baselines/reference/fourslash/findAllReferences/findReferencesJSXTagName3.baseline.jsonc	Baseline for finding references in JSX tag names
testdata/baselines/reference/fourslash/documentHighlights/*.baseline.jsonc	100+ baseline files covering document highlights for various TypeScript constructs including keywords, modifiers, classes, interfaces, control flow, and edge cases

andrewbranch · 2025-09-18T18:11:11Z

internal/fourslash/_scripts/failingTests.txt

Is there a pattern among the failing tests, i.e. something they’re blocked on or a specific follow-up task?

These tests are giving an incorrect output due to a missing case in getReferencedSymbolsForNode() in findAllReferences - I'll update the PR description with cases that could be updated as a follow-up task

internal/fourslash/tests/documentHighlight_test.go

andrewbranch

This is a great first PR 👏

internal/ls/documenthighlights.go

iisaduan

Great work! Seems like you've quickly gotten a good hang of the codebase :D

Requesting changes because I noticed an incorrect port of some logic, and I also left comments on some refactors and code/comment clean up

internal/fourslash/_scripts/convertFourslash.mts

iisaduan · 2025-09-19T22:08:49Z

internal/fourslash/_scripts/convertFourslash.mts

+            }
+        }
+        else if (ts.isObjectLiteralExpression(arg)) {
+            // User preferences case, but multiple files isn't implemented in corsa yet


What do you mean by "multiple files isn't implemented in corsa yet"? Does this fourslash function need different options per file?

Document highlights has the option search multiple files for references, but Jake told me to ignore this feature; this else if branch is searching for an input parameter that looks like {filesToSearch: ...}

Yeah, LSP does not have the ability to return document highlights outside the file where the request occurred.

(I am not sure the old code to do so even worked in VS Code.)

yeah, vscode doesnt request/handle symbols across multiple files (old vscode extension) and is unused in any strada test, so totally removing/not porting it is probably the way to go

Well, https://github.com/microsoft/vscode/blob/07da241ca964f5bb4e618c68c828fcba815e1045/extensions/typescript-language-features/src/languageFeatures/documentHighlight.ts#L17 did actually do it, but that's not in the LSP. We could still do this in the future. I'm not sure it's worth it at the moment to do multi-file highlights, but we may want it for script tag reasons and for embedded languages and so on. But, needs to go into the LSP.

Ah, makes sense. I wasn't ever able to get it to trigger in vscode so I assumeed it was never fully implemented. We can leave it as a !!! todo when in lsp in the ls implementation, and later update to match the format in vscode.proposed, but I don't think we need to support it in fourslash conversion if we don't have tests that use it

internal/ls/documenthighlights.go

iisaduan · 2025-09-20T02:37:59Z

internal/ls/documenthighlights.go

+		return []*ast.Node{node}
+	}
+	if ast.IsFunctionLike(node) {
+		return []*ast.Node{}


Suggested change

return []*ast.Node{}

return nil

iisaduan · 2025-09-20T02:42:17Z

internal/ls/documenthighlights.go

+
+		breakAndContinueStatements := aggregateAllBreakAndContinueStatements(clause, sourceFile)
+		for _, statement := range breakAndContinueStatements {
+			if ownsBreakOrContinueStatement(switchStatement.AsNode(), statement) && statement.Kind == ast.KindBreakStatement {


to avoid unneded calls to findAncestor

Suggested change

if ownsBreakOrContinueStatement(switchStatement.AsNode(), statement) && statement.Kind == ast.KindBreakStatement {

if statement.Kind == ast.KindBreakStatement && ownsBreakOrContinueStatement(switchStatement.AsNode(), statement) {

iisaduan · 2025-09-20T02:46:48Z

internal/ls/documenthighlights.go

+		finallyBlock := statement.FinallyBlock
+
+		if catchClause != nil {
+			result = append(result, aggregateOwnedThrowStatements(catchClause, sourceFile)...)


could do this to all the cases so that we don't need to do the work of copying the slice again

Suggested change

result = append(result, aggregateOwnedThrowStatements(catchClause, sourceFile)...)

result = aggregateOwnedThrowStatements(catchClause, sourceFile)

iisaduan · 2025-09-20T02:47:07Z

internal/ls/documenthighlights.go

+		if catchClause != nil {
+			result = append(result, aggregateOwnedThrowStatements(catchClause, sourceFile)...)
+		} else if tryBlock != nil {
+			result = append(result, aggregateOwnedThrowStatements(tryBlock, sourceFile)...)


Suggested change

result = append(result, aggregateOwnedThrowStatements(tryBlock, sourceFile)...)

result = aggregateOwnedThrowStatements(tryBlock, sourceFile)

iisaduan · 2025-09-20T02:49:04Z

internal/ls/documenthighlights.go

+		return []*ast.Node{node}
+	}
+	if ast.IsTryStatement(node) {
+		var result []*ast.Node


I'd move this var declaration (and other similar declarations) to right before the block where it's first written to (this case, the if catchClause != nil)

johnfav03 added 4 commits August 28, 2025 11:21

merged server from main

7a4b3e2

implemented highlights and helper functions

0b437c2

Merge remote-tracking branch 'origin/main' into document-highlights

57fa39d

fourslash testing for doc highlights

7342e59

gabritto and others added 10 commits September 10, 2025 08:26

fix compare LSP positions

45906a2

fix tests

a20e054

update baselines

c076d4b

Merge remote-tracking branch 'origin/main' into document-highlights

b08db3a

Merge branch 'gabritto/fixcompare' into document-highlights

c0fac1d

merged main

a76671b

Merge remote-tracking branch 'origin/main' into document-highlights

2fd1904

documentHighlights with fourslash tests

6d814bb

fixed minor change

1dade87

removed unused baselines

e9d2bd6

johnfav03 marked this pull request as ready for review September 17, 2025 21:04

johnfav03 requested review from Copilot and andrewbranch September 17, 2025 21:04

Copilot AI reviewed Sep 17, 2025

View reviewed changes

johnfav03 requested a review from iisaduan September 17, 2025 21:10

andrewbranch reviewed Sep 18, 2025

View reviewed changes

internal/fourslash/tests/documentHighlight_test.go Outdated Show resolved Hide resolved

andrewbranch reviewed Sep 18, 2025

View reviewed changes

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

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

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

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

johnfav03 added 4 commits September 18, 2025 16:28

removed repeated test

45cd07b

fixed var declaration and nil checking nits

48e3163

Merge branch 'main' into document-highlights

9c1245e

moved isWriteAccess and helpers from checker to ast

318a6f7

johnfav03 requested a review from andrewbranch September 19, 2025 15:18

andrewbranch approved these changes Sep 19, 2025

View reviewed changes

iisaduan requested changes Sep 22, 2025

View reviewed changes

johnfav03 added 2 commits September 22, 2025 11:02

refactored renameArg and docHighlightArg into markerOrRangeArg

2d661ce

fixed jsx case handling

508be4d

	if ownsBreakOrContinueStatement(switchStatement.AsNode(), statement) && statement.Kind == ast.KindBreakStatement {
	if statement.Kind == ast.KindBreakStatement && ownsBreakOrContinueStatement(switchStatement.AsNode(), statement) {

	result = append(result, aggregateOwnedThrowStatements(catchClause, sourceFile)...)
	result = aggregateOwnedThrowStatements(catchClause, sourceFile)

	result = append(result, aggregateOwnedThrowStatements(tryBlock, sourceFile)...)
	result = aggregateOwnedThrowStatements(tryBlock, sourceFile)

Document highlights #1699

Are you sure you want to change the base?

Document highlights #1699

Conversation

johnfav03 commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnfav03 commented Sep 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewbranch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iisaduan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnfav03 commented Sep 9, 2025 •

edited

Loading