Skip to content

Support LocationLink in go-to-definition with default enablement #1490

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.

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 19 additions & 3 deletions internal/fourslash/fourslash.go
Original file line number Diff line number Diff line change
Expand Up @@ -886,17 +886,33 @@ func (f *FourslashTest) VerifyBaselineGoToDefinition(
}

var resultAsLocations []lsproto.Location
var additionalSpan *lsproto.Location
if result.Locations != nil {
resultAsLocations = *result.Locations
} else if result.Location != nil {
resultAsLocations = []lsproto.Location{*result.Location}
} else if result.DefinitionLinks != nil {
t.Fatalf("Unexpected definition response type at marker '%s': %T", *f.lastKnownMarkerName, result.DefinitionLinks)
// For DefinitionLinks, extract the target locations and optionally set additionalSpan
resultAsLocations = core.Map(*result.DefinitionLinks, func(link *lsproto.LocationLink) lsproto.Location {
return lsproto.Location{
Uri: link.TargetUri,
Range: link.TargetSelectionRange,
}
})

// If there's a single result and it has an origin selection range, use it as additionalSpan
if len(*result.DefinitionLinks) == 1 && (*result.DefinitionLinks)[0].OriginSelectionRange != nil {
additionalSpan = &lsproto.Location{
Uri: ls.FileNameToDocumentURI(markerOrRange.GetMarker().FileName()),
Range: *(*result.DefinitionLinks)[0].OriginSelectionRange,
}
}
}

f.baseline.addResult("goToDefinition", f.getBaselineForLocationsWithFileContents(resultAsLocations, baselineFourslashLocationsOptions{
marker: markerOrRange.GetMarker(),
markerName: "/*GO TO DEFINITION*/",
marker: markerOrRange.GetMarker(),
markerName: "/*GO TO DEFINITION*/",
additionalSpan: additionalSpan,
}))
}

Expand Down
63 changes: 54 additions & 9 deletions internal/ls/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/microsoft/typescript-go/internal/scanner"
)

func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsproto.DocumentUri, position lsproto.Position) (lsproto.DefinitionResponse, error) {
func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsproto.DocumentUri, position lsproto.Position, clientCapabilities *lsproto.DefinitionClientCapabilities) (lsproto.DefinitionResponse, error) {
program, file := l.getProgramAndFile(documentURI)
node := astnav.GetTouchingPropertyName(file, int(l.converters.LineAndCharacterToPosition(file, position)))
if node.Kind == ast.KindSourceFile {
Expand All @@ -23,13 +23,13 @@ func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsp

if node.Kind == ast.KindOverrideKeyword {
if sym := getSymbolForOverriddenMember(c, node); sym != nil {
return l.createLocationsFromDeclarations(sym.Declarations), nil
return l.createDefinitionResponse(sym.Declarations, node, file, clientCapabilities), nil
}
}

if ast.IsJumpStatementTarget(node) {
if label := getTargetLabel(node.Parent, node.Text()); label != nil {
return l.createLocationsFromDeclarations([]*ast.Node{label}), nil
return l.createDefinitionResponse([]*ast.Node{label}, node, file, clientCapabilities), nil
}
}

Expand All @@ -42,16 +42,16 @@ func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsp

if node.Kind == ast.KindReturnKeyword || node.Kind == ast.KindYieldKeyword || node.Kind == ast.KindAwaitKeyword {
if fn := ast.FindAncestor(node, ast.IsFunctionLikeDeclaration); fn != nil {
return l.createLocationsFromDeclarations([]*ast.Node{fn}), nil
return l.createDefinitionResponse([]*ast.Node{fn}, node, file, clientCapabilities), nil
}
}

if calledDeclaration := tryGetSignatureDeclaration(c, node); calledDeclaration != nil {
return l.createLocationsFromDeclarations([]*ast.Node{calledDeclaration}), nil
return l.createDefinitionResponse([]*ast.Node{calledDeclaration}, node, file, clientCapabilities), nil
}

if ast.IsIdentifier(node) && ast.IsShorthandPropertyAssignment(node.Parent) {
return l.createLocationsFromDeclarations(c.GetResolvedSymbol(node).Declarations), nil
return l.createDefinitionResponse(c.GetResolvedSymbol(node).Declarations, node, file, clientCapabilities), nil
}

node = getDeclarationNameForKeyword(node)
Expand All @@ -70,15 +70,15 @@ func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsp
if symbol.Flags&(ast.SymbolFlagsProperty|ast.SymbolFlagsMethod|ast.SymbolFlagsAccessor) != 0 && symbol.Parent != nil && symbol.Parent.Flags&ast.SymbolFlagsObjectLiteral != 0 {
if objectLiteral := core.FirstOrNil(symbol.Parent.Declarations); objectLiteral != nil {
if declarations := c.GetContextualDeclarationsForObjectLiteralElement(objectLiteral, symbol.Name); len(declarations) != 0 {
return l.createLocationsFromDeclarations(declarations), nil
return l.createDefinitionResponse(declarations, node, file, clientCapabilities), nil
}
}
}
return l.createLocationsFromDeclarations(symbol.Declarations), nil
return l.createDefinitionResponse(symbol.Declarations, node, file, clientCapabilities), nil
}

if indexInfos := c.GetIndexSignaturesAtLocation(node); len(indexInfos) != 0 {
return l.createLocationsFromDeclarations(indexInfos), nil
return l.createDefinitionResponse(indexInfos, node, file, clientCapabilities), nil
}

return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{}, nil
Expand Down Expand Up @@ -126,6 +126,51 @@ func getDeclarationNameForKeyword(node *ast.Node) *ast.Node {
return node
}

func (l *LanguageService) createDefinitionResponse(declarations []*ast.Node, originNode *ast.Node, originFile *ast.SourceFile, clientCapabilities *lsproto.DefinitionClientCapabilities) lsproto.DefinitionResponse {
// Check if client supports LocationLink
if clientCapabilities != nil && clientCapabilities.LinkSupport != nil && *clientCapabilities.LinkSupport {
return l.createLocationLinksFromDeclarations(declarations, originNode, originFile)
}
// Fall back to traditional Location response
return l.createLocationsFromDeclarations(declarations)
}

func (l *LanguageService) createLocationLinksFromDeclarations(declarations []*ast.Node, originNode *ast.Node, originFile *ast.SourceFile) lsproto.DefinitionResponse {
someHaveBody := core.Some(declarations, func(node *ast.Node) bool { return node.Body() != nil })
links := make([]*lsproto.LocationLink, 0, len(declarations))

// Calculate origin selection range (the "bound span")
originSelectionRange := l.createLspRangeFromNode(originNode, originFile)

for _, decl := range declarations {
if !someHaveBody || decl.Body() != nil {
file := ast.GetSourceFileOfNode(decl)
name := core.OrElse(ast.GetNameOfDeclaration(decl), decl)

// For targetRange, use the full declaration range
var targetRange *lsproto.Range
if decl.Body() != nil {
// For declarations with body, include the full declaration
targetRange = l.createLspRangeFromBounds(scanner.GetTokenPosOfNode(decl, file, false), decl.End(), file)
} else {
// For declarations without body, use the declaration itself
targetRange = l.createLspRangeFromNode(decl, file)
}

// For targetSelectionRange, use just the name/identifier part
targetSelectionRange := l.createLspRangeFromNode(name, file)

links = append(links, &lsproto.LocationLink{
OriginSelectionRange: originSelectionRange,
TargetUri: FileNameToDocumentURI(file.FileName()),
TargetRange: *targetRange,
TargetSelectionRange: *targetSelectionRange,
})
}
}
return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{DefinitionLinks: &links}
}

func (l *LanguageService) createLocationsFromDeclarations(declarations []*ast.Node) lsproto.DefinitionResponse {
Copy link
Member

Choose a reason for hiding this comment

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

Who is using this function anymore? Can it be used in the new code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The createLocationsFromDeclarations function is still being used as the fallback when LinkSupport is false in the createDefinitionResponse method. It provides backwards compatibility for clients that don't support LocationLink.

someHaveBody := core.Some(declarations, func(node *ast.Node) bool { return node.Body() != nil })
locations := make([]lsproto.Location, 0, len(declarations))
Expand Down
38 changes: 38 additions & 0 deletions internal/ls/definition_link_support_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package ls

import (
"testing"

"github.com/microsoft/typescript-go/internal/lsp/lsproto"
"gotest.tools/v3/assert"
)

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

// Simple integration test to ensure LocationLink support works
// without causing import cycles

// Test that client capabilities are correctly used
linkSupport := true
capabilities := &lsproto.DefinitionClientCapabilities{
LinkSupport: &linkSupport,
}

// Test that the capability checking logic works
assert.Assert(t, capabilities != nil)
assert.Assert(t, capabilities.LinkSupport != nil)
assert.Assert(t, *capabilities.LinkSupport)

// Test with capabilities disabled
linkSupportFalse := false
capabilitiesDisabled := &lsproto.DefinitionClientCapabilities{
LinkSupport: &linkSupportFalse,
}
assert.Assert(t, capabilitiesDisabled.LinkSupport != nil)
assert.Assert(t, !*capabilitiesDisabled.LinkSupport)

// Test with nil capabilities
var nilCapabilities *lsproto.DefinitionClientCapabilities
assert.Assert(t, nilCapabilities == nil)
}
3 changes: 2 additions & 1 deletion internal/ls/definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func runDefinitionTest(t *testing.T, input string, expected map[string]lsproto.D
locations, err := languageService.ProvideDefinition(
ctx,
ls.FileNameToDocumentURI(file),
marker.LSPosition)
marker.LSPosition,
nil) // No client capabilities in test
assert.NilError(t, err)
assert.DeepEqual(t, locations, expectedResult)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/ls/untitled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ x++;`
// Also test definition using ProvideDefinition
uri := ls.FileNameToDocumentURI("/Untitled-2.ts")
lspPosition := lsproto.Position{Line: 2, Character: 0}
definition, err := service.ProvideDefinition(t.Context(), uri, lspPosition)
definition, err := service.ProvideDefinition(t.Context(), uri, lspPosition, nil)
assert.NilError(t, err)
if definition.Locations != nil {
t.Logf("Definition found: %d locations", len(*definition.Locations))
Expand Down
29 changes: 28 additions & 1 deletion internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ func (s *Server) handleDefinition(ctx context.Context, params *lsproto.Definitio
project := s.projectService.EnsureDefaultProjectForURI(params.TextDocument.Uri)
languageService, done := project.GetLanguageServiceForRequest(ctx)
defer done()
return languageService.ProvideDefinition(ctx, params.TextDocument.Uri, params.Position)
return languageService.ProvideDefinition(ctx, params.TextDocument.Uri, params.Position, definitionCapabilities(s.initializeParams))
}

func (s *Server) handleTypeDefinition(ctx context.Context, params *lsproto.TypeDefinitionParams) (lsproto.TypeDefinitionResponse, error) {
Expand Down Expand Up @@ -878,3 +878,30 @@ func getCompletionClientCapabilities(params *lsproto.InitializeParams) *lsproto.
}
return params.Capabilities.TextDocument.Completion
}

func definitionCapabilities(params *lsproto.InitializeParams) *lsproto.DefinitionClientCapabilities {
if params == nil || params.Capabilities == nil || params.Capabilities.TextDocument == nil {
// Return default capabilities with LinkSupport enabled
return &lsproto.DefinitionClientCapabilities{
LinkSupport: ptrTo(true),
}
}

capabilities := params.Capabilities.TextDocument.Definition
Copy link
Member

Choose a reason for hiding this comment

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

call this definitionCapabilities because it looks like it's checked above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to definitionCapabilities. Commit a8ba72c.

if capabilities == nil {
// Return default capabilities with LinkSupport enabled
return &lsproto.DefinitionClientCapabilities{
LinkSupport: ptrTo(true),
}
}

// If capabilities exist but LinkSupport is not specified, default to true
if capabilities.LinkSupport == nil {
// Copy existing capabilities and override LinkSupport
result := *capabilities
result.LinkSupport = ptrTo(true)
return &result
}

return capabilities
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
// === /foo.ts ===

// export function bar() { return "bar"; }
// import('./foo').then(({ ba/*GO TO DEFINITION*/[|bar|] }) => undefined);
// import('./foo').then(({ ba/*GO TO DEFINITION*/[|{ textSpan: true |}bar|] }) => undefined);
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
// === /foo.ts ===

// export function bar() { return "bar"; }
// import('./foo').then(({ ba/*GO TO DEFINITION*/[|bar|] }) => undefined);
// import('./foo').then(({ ba/*GO TO DEFINITION*/[|{ textSpan: true |}bar|] }) => undefined);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// === goToDefinition ===
// === /a.ts ===

// declare module "external/*GO TO DEFINITION*/[|"external"|] {
// declare module "external/*GO TO DEFINITION*/[|{ textSpan: true |}"external"|] {
// class Foo { }
// }
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
// === /a.ts ===

// class A {
// private z/*GO TO DEFINITION*/[|z|]: string;
// private z/*GO TO DEFINITION*/[|{ textSpan: true |}z|]: string;
// }
Loading