Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7a4b3e2
merged server from main
johnfav03 Aug 28, 2025
0b437c2
implemented highlights and helper functions
johnfav03 Aug 28, 2025
57fa39d
Merge remote-tracking branch 'origin/main' into document-highlights
johnfav03 Sep 9, 2025
7342e59
fourslash testing for doc highlights
johnfav03 Sep 9, 2025
45906a2
fix compare LSP positions
gabritto Sep 10, 2025
a20e054
fix tests
gabritto Sep 10, 2025
c076d4b
update baselines
gabritto Sep 10, 2025
b08db3a
Merge remote-tracking branch 'origin/main' into document-highlights
johnfav03 Sep 10, 2025
c0fac1d
Merge branch 'gabritto/fixcompare' into document-highlights
johnfav03 Sep 10, 2025
a76671b
merged main
johnfav03 Sep 16, 2025
2fd1904
Merge remote-tracking branch 'origin/main' into document-highlights
johnfav03 Sep 17, 2025
6d814bb
documentHighlights with fourslash tests
johnfav03 Sep 17, 2025
1dade87
fixed minor change
johnfav03 Sep 17, 2025
e9d2bd6
removed unused baselines
johnfav03 Sep 17, 2025
45cd07b
removed repeated test
johnfav03 Sep 18, 2025
48e3163
fixed var declaration and nil checking nits
johnfav03 Sep 18, 2025
9c1245e
Merge branch 'main' into document-highlights
johnfav03 Sep 18, 2025
318a6f7
moved isWriteAccess and helpers from checker to ast
johnfav03 Sep 18, 2025
2d661ce
refactored renameArg and docHighlightArg into markerOrRangeArg
johnfav03 Sep 22, 2025
508be4d
fixed jsx case handling
johnfav03 Sep 22, 2025
a13337c
Merge remote-tracking branch 'origin' into document-highlights
johnfav03 Sep 24, 2025
9b512a5
added isWriteAccessForReference logic
johnfav03 Sep 24, 2025
ea32d43
cleaned up comments
johnfav03 Sep 24, 2025
d446a93
documenthighlights.go code and comment cleanup
johnfav03 Sep 24, 2025
1dcecaf
Update internal/fourslash/_scripts/convertFourslash.mts
johnfav03 Sep 24, 2025
0ec6d31
removed extraneous newline
johnfav03 Sep 24, 2025
2dd5588
minor test fix
johnfav03 Sep 25, 2025
f8c43e1
fixed out of bounds in findReferencedSymbolsForNode
johnfav03 Sep 25, 2025
e903fae
reupdated failing tests
johnfav03 Sep 25, 2025
016ab91
fixed ci check for convertfourslash
johnfav03 Sep 25, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -3149,6 +3149,10 @@ func (node *ThrowStatement) computeSubtreeFacts() SubtreeFacts {
return propagateSubtreeFacts(node.Expression)
}

func IsThrowStatement(node *Node) bool {
return node.Kind == KindThrowStatement
}

// TryStatement

type TryStatement struct {
Expand Down Expand Up @@ -6111,6 +6115,10 @@ func (node *YieldExpression) computeSubtreeFacts() SubtreeFacts {
return propagateSubtreeFacts(node.Expression) | SubtreeContainsForAwaitOrAsyncGenerator
}

func IsYieldExpression(node *Node) bool {
return node.Kind == KindYieldExpression
}

// ArrowFunction

type ArrowFunction struct {
Expand Down
8 changes: 4 additions & 4 deletions internal/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -10873,7 +10873,7 @@ func (c *Checker) checkPropertyAccessExpressionOrQualifiedName(node *ast.Node, l
c.checkPropertyNotUsedBeforeDeclaration(prop, node, right)
c.markPropertyAsReferenced(prop, node, c.isSelfTypeAccess(left, parentSymbol))
c.symbolNodeLinks.Get(node).resolvedSymbol = prop
c.checkPropertyAccessibility(node, left.Kind == ast.KindSuperKeyword, isWriteAccess(node), apparentType, prop)
c.checkPropertyAccessibility(node, left.Kind == ast.KindSuperKeyword, IsWriteAccess(node), apparentType, prop)
if c.isAssignmentToReadonlyEntity(node, prop, assignmentKind) {
c.error(right, diagnostics.Cannot_assign_to_0_because_it_is_a_read_only_property, right.Text())
return c.errorType
Expand Down Expand Up @@ -15715,9 +15715,9 @@ func (c *Checker) GetTypeOfSymbolAtLocation(symbol *ast.Symbol, location *ast.No
if ast.IsRightSideOfQualifiedNameOrPropertyAccess(location) {
location = location.Parent
}
if ast.IsExpressionNode(location) && (!ast.IsAssignmentTarget(location) || isWriteAccess(location)) {
if ast.IsExpressionNode(location) && (!ast.IsAssignmentTarget(location) || IsWriteAccess(location)) {
var t *Type
if isWriteAccess(location) && location.Kind == ast.KindPropertyAccessExpression {
if IsWriteAccess(location) && location.Kind == ast.KindPropertyAccessExpression {
t = c.checkPropertyAccessExpression(location, CheckModeNormal, true /*writeOnly*/)
} else {
t = c.getTypeOfExpression(location)
Expand All @@ -15735,7 +15735,7 @@ func (c *Checker) GetTypeOfSymbolAtLocation(symbol *ast.Symbol, location *ast.No
// to it at the given location. Since we have no control flow information for the
// hypothetical reference (control flow information is created and attached by the
// binder), we simply return the declared type of the symbol.
if isRightSideOfAccessExpression(location) && isWriteAccess(location.Parent) {
if isRightSideOfAccessExpression(location) && IsWriteAccess(location.Parent) {
return c.getWriteTypeOfSymbol(symbol)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checker/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ func isWriteOnlyAccess(node *ast.Node) bool {
return accessKind(node) == AccessKindWrite
}

func isWriteAccess(node *ast.Node) bool {
func IsWriteAccess(node *ast.Node) bool {
return accessKind(node) != AccessKindRead
}

Expand Down
99 changes: 99 additions & 0 deletions internal/fourslash/_scripts/convertFourslash.mts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ function parseFourslashStatement(statement: ts.Statement): Cmd[] | undefined {
case "baselineFindAllReferences":
// `verify.baselineFindAllReferences(...)`
return parseBaselineFindAllReferencesArgs(callExpression.arguments);
case "baselineDocumentHighlights":
return parseBaselineDocumentHighlightsArgs(callExpression.arguments);
case "baselineQuickInfo":
return [parseBaselineQuickInfo(callExpression.arguments)];
case "baselineSignatureHelp":
Expand Down Expand Up @@ -778,6 +780,90 @@ function parseBaselineFindAllReferencesArgs(args: readonly ts.Expression[]): [Ve
}];
}

function parseBaselineDocumentHighlightsArgs(args: readonly ts.Expression[]): [VerifyBaselineDocumentHighlightsCmd] | undefined {
const newArgs: string[] = [];
let preferences: string | undefined;
for (const arg of args) {
let strArg;
if (strArg = getArrayLiteralExpression(arg)) {
for (const elem of strArg.elements) {
const newArg = parseBaselineDocumentHighlightsArg(elem);
if (!newArg) {
return undefined;
}
newArgs.push(newArg);
}
}
else if (ts.isObjectLiteralExpression(arg)) {
// User preferences case, but multiple files isn't implemented in corsa yet
}
else if (strArg = parseBaselineDocumentHighlightsArg(arg)) {
newArgs.push(strArg);
}
else {
console.error(`Unrecognized argument in verify.baselineDocumentHighlights: ${arg.getText()}`);
return undefined;
}
}

if (newArgs.length === 0) {
newArgs.push("ToAny(f.Ranges())...");
}

return [{
kind: "verifyBaselineDocumentHighlights",
args: newArgs,
preferences: preferences ? preferences : "nil /*preferences*/",
}];
}

function parseBaselineDocumentHighlightsArg(arg: ts.Expression): string | undefined {
if (ts.isStringLiteral(arg)) {
return getGoStringLiteral(arg.text);
}
else if (ts.isIdentifier(arg) || (ts.isElementAccessExpression(arg) && ts.isIdentifier(arg.expression))) {
const argName = ts.isIdentifier(arg) ? arg.text : (arg.expression as ts.Identifier).text;
const file = arg.getSourceFile();
const varStmts = file.statements.filter(ts.isVariableStatement);
for (const varStmt of varStmts) {
for (const decl of varStmt.declarationList.declarations) {
if (ts.isArrayBindingPattern(decl.name) && decl.initializer?.getText().includes("ranges")) {
for (let i = 0; i < decl.name.elements.length; i++) {
const elem = decl.name.elements[i];
if (ts.isBindingElement(elem) && ts.isIdentifier(elem.name) && elem.name.text === argName) {
// `const [range_0, ..., range_n, ...] = test.ranges();` and arg is `range_n`
if (elem.dotDotDotToken === undefined) {
return `f.Ranges()[${i}]`;
}
// `const [range_0, ..., ...rest] = test.ranges();` and arg is `rest[n]`
if (ts.isElementAccessExpression(arg)) {
return `f.Ranges()[${i + parseInt(arg.argumentExpression!.getText())}]`;
}
// `const [range_0, ..., ...rest] = test.ranges();` and arg is `rest`
return `ToAny(f.Ranges()[${i}:])...`;
}
}
}
}
}
const init = getNodeOfKind(arg, ts.isCallExpression);
if (init) {
const result = getRangesByTextArg(init);
if (result) {
return result;
}
}
}
else if (ts.isCallExpression(arg)) {
const result = getRangesByTextArg(arg);
if (result) {
return result;
}
}
console.error(`Unrecognized argument in verify.baselineRename: ${arg.getText()}`);
return undefined;
}

function parseBaselineGoToDefinitionArgs(args: readonly ts.Expression[]): [VerifyBaselineGoToDefinitionCmd] | undefined {
const newArgs = [];
for (const arg of args) {
Expand Down Expand Up @@ -1262,6 +1348,12 @@ interface VerifyBaselineRenameCmd {
preferences: string;
}

interface VerifyBaselineDocumentHighlightsCmd {
kind: "verifyBaselineDocumentHighlights";
args: string[];
preferences: string;
}

interface GoToCmd {
kind: "goTo";
// !!! `selectRange` and `rangeStart` require parsing variables and `test.ranges()[n]`
Expand Down Expand Up @@ -1289,6 +1381,7 @@ interface VerifyRenameInfoCmd {
type Cmd =
| VerifyCompletionsCmd
| VerifyBaselineFindAllReferencesCmd
| VerifyBaselineDocumentHighlightsCmd
| VerifyBaselineGoToDefinitionCmd
| VerifyBaselineQuickInfoCmd
| VerifyBaselineSignatureHelpCmd
Expand Down Expand Up @@ -1332,6 +1425,10 @@ function generateBaselineFindAllReferences({ markers, ranges }: VerifyBaselineFi
return `f.VerifyBaselineFindAllReferences(t, ${markers.join(", ")})`;
}

function generateBaselineDocumentHighlights({ args, preferences }: VerifyBaselineDocumentHighlightsCmd): string {
return `f.VerifyBaselineDocumentHighlights(t, ${preferences}, ${args.join(", ")})`;
}

function generateBaselineGoToDefinition({ markers, ranges }: VerifyBaselineGoToDefinitionCmd): string {
if (ranges || markers.length === 0) {
return `f.VerifyBaselineGoToDefinition(t)`;
Expand Down Expand Up @@ -1372,6 +1469,8 @@ function generateCmd(cmd: Cmd): string {
return generateVerifyCompletions(cmd);
case "verifyBaselineFindAllReferences":
return generateBaselineFindAllReferences(cmd);
case "verifyBaselineDocumentHighlights":
return generateBaselineDocumentHighlights(cmd);
case "verifyBaselineGoToDefinition":
return generateBaselineGoToDefinition(cmd);
case "verifyBaselineQuickInfo":
Expand Down
18 changes: 18 additions & 0 deletions internal/fourslash/_scripts/failingTests.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,15 @@ TestCompletionsUniqueSymbol1
TestConstEnumQuickInfoAndCompletionList
TestConstQuickInfoAndCompletionList
TestContextuallyTypedFunctionExpressionGeneric1
TestDocumentHighlightInExport1
TestDocumentHighlightInTypeExport
TestDocumentHighlightJSDocTypedef
TestDocumentHighlightTemplateStrings
TestDocumentHighlightsInvalidGlobalThis
TestDocumentHighlights_40082
TestDoubleUnderscoreCompletions
TestEditJsdocType
TestEmptyExportFindReferences
TestExportDefaultClass
TestExportDefaultFunction
TestFindAllReferencesDynamicImport1
Expand All @@ -173,6 +180,7 @@ TestFindAllRefsCommonJsRequire2
TestFindAllRefsCommonJsRequire3
TestFindAllRefsExportEquals
TestFindAllRefsForDefaultExport03
TestFindAllRefsForModule
TestFindAllRefsModuleDotExports
TestFindAllRefs_importType_typeofImport
TestFindReferencesAfterEdit
Expand Down Expand Up @@ -201,6 +209,14 @@ TestGetJavaScriptQuickInfo6
TestGetJavaScriptQuickInfo7
TestGetJavaScriptQuickInfo8
TestGetJavaScriptSyntacticDiagnostics24
TestGetOccurrencesAsyncAwait3
TestGetOccurrencesClassExpressionConstructor
TestGetOccurrencesConstructor
TestGetOccurrencesConstructor2
TestGetOccurrencesIfElseBroken
TestGetOccurrencesOfAnonymousFunction
TestGetOccurrencesStringLiteralTypes
TestGetOccurrencesStringLiterals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, what kinds of failures are these? Crashes? If they're crashes, I would definitely want to double check these because document highlight is something that happens automatically on cursor move, which means we could be spamming panics.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, these must be https://github.com/microsoft/typescript-go/pull/1699/files#r2360590494, which I didn't notice since it was attached to an old rev.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are because of unimplemented cases in FAR, so the test fails when no highlights are returned (which is fine in editor)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String references and a lot of the keyword references aren't implemented

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only two tests fail, and it's due to unexpected responses in fourslash.go from modules that arent documentHighlights. The tests are FindAllRefsForModule and TestJsdocTypedefTagServices, which return an unexpected response from FindAllReferences and QuickInfo respectively.

TestGetQuickInfoForIntersectionTypes
TestHoverOverComment
TestImportCompletionsPackageJsonExportsSpecifierEndsInTs
Expand Down Expand Up @@ -264,6 +280,7 @@ TestJsdocThrowsTagCompletion
TestJsdocTypedefTag
TestJsdocTypedefTag2
TestJsdocTypedefTagNamespace
TestJsdocTypedefTagServices
TestJsxFindAllReferencesOnRuntimeImportWithPaths1
TestLetQuickInfoAndCompletionList
TestLocalFunction
Expand Down Expand Up @@ -329,6 +346,7 @@ TestPathCompletionsTypesVersionsWildcard4
TestPathCompletionsTypesVersionsWildcard5
TestPathCompletionsTypesVersionsWildcard6
TestProtoVarVisibleWithOuterScopeUnderscoreProto
TestQualifiedName_import_declaration_with_variable_entity_names
TestQuickInfoAlias
TestQuickInfoAssertionNodeNotReusedWhenTypeNotEquivalent1
TestQuickInfoBindingPatternInJsdocNoCrash1
Expand Down
77 changes: 77 additions & 0 deletions internal/fourslash/fourslash.go
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,83 @@ func (f *FourslashTest) VerifyBaselineSignatureHelp(t *testing.T) {
}
}

func (f *FourslashTest) VerifyBaselineDocumentHighlights(
t *testing.T,
preferences *ls.UserPreferences,
markerOrRangeOrNames ...MarkerOrRangeOrName,
) {
var markerOrRanges []MarkerOrRange
for _, markerOrRangeOrName := range markerOrRangeOrNames {
switch markerOrNameOrRange := markerOrRangeOrName.(type) {
case string:
marker, ok := f.testData.MarkerPositions[markerOrNameOrRange]
if !ok {
t.Fatalf("Marker '%s' not found", markerOrNameOrRange)
}
markerOrRanges = append(markerOrRanges, marker)
case *Marker:
markerOrRanges = append(markerOrRanges, markerOrNameOrRange)
case *RangeMarker:
markerOrRanges = append(markerOrRanges, markerOrNameOrRange)
default:
t.Fatalf("Invalid marker or range type: %T. Expected string, *Marker, or *RangeMarker.", markerOrNameOrRange)
}
}

f.verifyBaselineDocumentHighlights(t, preferences, markerOrRanges)
}

func (f *FourslashTest) verifyBaselineDocumentHighlights(
t *testing.T,
preferences *ls.UserPreferences,
markerOrRanges []MarkerOrRange,
) {
for _, markerOrRange := range markerOrRanges {
f.goToMarker(t, markerOrRange)

params := &lsproto.DocumentHighlightParams{
TextDocument: lsproto.TextDocumentIdentifier{
Uri: ls.FileNameToDocumentURI(f.activeFilename),
},
Position: f.currentCaretPosition,
}
resMsg, result, resultOk := sendRequest(t, f, lsproto.TextDocumentDocumentHighlightInfo, params)
if resMsg == nil {
if f.lastKnownMarkerName == nil {
t.Fatalf("Nil response received for document highlights request at pos %v", f.currentCaretPosition)
} else {
t.Fatalf("Nil response received for document highlights request at marker '%s'", *f.lastKnownMarkerName)
}
}
if !resultOk {
if f.lastKnownMarkerName == nil {
t.Fatalf("Unexpected document highlights response type at pos %v: %T", f.currentCaretPosition, resMsg.AsResponse().Result)
} else {
t.Fatalf("Unexpected document highlights response type at marker '%s': %T", *f.lastKnownMarkerName, resMsg.AsResponse().Result)
}
}

highlights := result.DocumentHighlights
if highlights == nil {
highlights = &[]*lsproto.DocumentHighlight{}
}

var spans []lsproto.Location
for _, h := range *highlights {
spans = append(spans, lsproto.Location{
Uri: ls.FileNameToDocumentURI(f.activeFilename),
Range: h.Range,
})
}

// Add result to baseline
f.addResultToBaseline(t, "documentHighlights", f.getBaselineForLocationsWithFileContents(spans, baselineFourslashLocationsOptions{
marker: markerOrRange,
markerName: "/*HIGHLIGHTS*/",
}))
}
}

// Collects all named markers if provided, or defaults to anonymous ranges
func (f *FourslashTest) lookupMarkersOrGetRanges(t *testing.T, markers []string) []MarkerOrRange {
var referenceLocations []MarkerOrRange
Expand Down
19 changes: 19 additions & 0 deletions internal/fourslash/tests/documentHighlight_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package fourslash

import (
"testing"

"github.com/microsoft/typescript-go/internal/fourslash"
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
"github.com/microsoft/typescript-go/internal/testutil"
)

func TestDocumentHighlights(t *testing.T) {
t.Parallel()
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `[|function|] [|f|](x: typeof [|f|]) {
[|f|]([|f|]);
}`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
f.VerifyBaselineDocumentHighlights(t, nil /*preferences*/, ToAny(f.Ranges())...)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package fourslash_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/fourslash"
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
"github.com/microsoft/typescript-go/internal/testutil"
)

func TestDocumentHighlightAtInheritedProperties1(t *testing.T) {
t.Parallel()

defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `// @Filename: file1.ts
interface interface1 extends interface1 {
[|doStuff|](): void;
[|propName|]: string;
}`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
f.VerifyBaselineDocumentHighlights(t, nil /*preferences*/, ToAny(f.Ranges())...)
}
Loading
Loading