Skip to content

Go-to-def should include both declaration(s) and target call signature #1543

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+123 −111
Open

Go-to-def should include both declaration(s) and target call signature #1543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39c4f1e
Go to def should include both declaration(s) and target call signature
ahejlsberg Aug 8, 2025
651268b
Return only unique locations
ahejlsberg Aug 9, 2025
183d02b
Accept new baselines
ahejlsberg Aug 9, 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
91 changes: 46 additions & 45 deletions internal/ls/definition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ls

import (
"context"
"slices"

"github.com/microsoft/typescript-go/internal/ast"
"github.com/microsoft/typescript-go/internal/astnav"
Expand Down Expand Up @@ -46,42 +47,14 @@ func (l *LanguageService) ProvideDefinition(ctx context.Context, documentURI lsp
}
}

if calledDeclaration := tryGetSignatureDeclaration(c, node); calledDeclaration != nil {
return l.createLocationsFromDeclarations([]*ast.Node{calledDeclaration}), nil
declarations := getDeclarationsFromLocation(c, node)
calledDeclaration := tryGetSignatureDeclaration(c, node)
if calledDeclaration != nil {
// If we can resolve a call signature, remove all function-like declarations and add that signature.
nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) })
Copy link
Preview

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Using slices.Clip(declarations) creates an unnecessary copy of the slice. Since core.Filter likely creates a new slice anyway, you can pass declarations directly to avoid the extra allocation.

Suggested change
nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) })
nonFunctionDeclarations := core.Filter(declarations, func(node *ast.Node) bool { return !ast.IsFunctionLike(node) })

Copilot uses AI. Check for mistakes.

Copy link
Member Author

Choose a reason for hiding this comment

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

core.Filter only creates a new slice if something was removed. Thus the need for slices.Clip.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just make filter always clip its input for safety?

declarations = append(nonFunctionDeclarations, calledDeclaration)
}

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

node = getDeclarationNameForKeyword(node)

if symbol := c.GetSymbolAtLocation(node); symbol != nil {
if symbol.Flags&ast.SymbolFlagsClass != 0 && symbol.Flags&(ast.SymbolFlagsFunction|ast.SymbolFlagsVariable) == 0 && node.Kind == ast.KindConstructorKeyword {
if constructor := symbol.Members[ast.InternalSymbolNameConstructor]; constructor != nil {
symbol = constructor
}
}
if symbol.Flags&ast.SymbolFlagsAlias != 0 {
if resolved, ok := c.ResolveAlias(symbol); ok {
symbol = resolved
}
}
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.createLocationsFromDeclarations(symbol.Declarations), nil
}

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

return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{}, nil
return l.createLocationsFromDeclarations(declarations), nil
}

func (l *LanguageService) ProvideTypeDefinition(ctx context.Context, documentURI lsproto.DocumentUri, position lsproto.Position) (lsproto.DefinitionResponse, error) {
Expand Down Expand Up @@ -127,17 +100,14 @@ func getDeclarationNameForKeyword(node *ast.Node) *ast.Node {
}

func (l *LanguageService) createLocationsFromDeclarations(declarations []*ast.Node) lsproto.DefinitionResponse {
someHaveBody := core.Some(declarations, func(node *ast.Node) bool { return node.Body() != nil })
locations := make([]lsproto.Location, 0, len(declarations))
for _, decl := range declarations {
if !someHaveBody || decl.Body() != nil {
file := ast.GetSourceFileOfNode(decl)
name := core.OrElse(ast.GetNameOfDeclaration(decl), decl)
locations = append(locations, lsproto.Location{
Uri: FileNameToDocumentURI(file.FileName()),
Range: *l.createLspRangeFromNode(name, file),
})
}
file := ast.GetSourceFileOfNode(decl)
name := core.OrElse(ast.GetNameOfDeclaration(decl), decl)
locations = core.AppendIfUnique(locations, lsproto.Location{
Uri: FileNameToDocumentURI(file.FileName()),
Range: *l.createLspRangeFromNode(name, file),
})
}
return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{Locations: &locations}
}
Expand All @@ -151,7 +121,38 @@ func (l *LanguageService) createLocationFromFileAndRange(file *ast.SourceFile, t
}
}

/** Returns a CallLikeExpression where `node` is the target being invoked. */
func getDeclarationsFromLocation(c *checker.Checker, node *ast.Node) []*ast.Node {
if ast.IsIdentifier(node) && ast.IsShorthandPropertyAssignment(node.Parent) {
return c.GetResolvedSymbol(node).Declarations
}
node = getDeclarationNameForKeyword(node)
if symbol := c.GetSymbolAtLocation(node); symbol != nil {
if symbol.Flags&ast.SymbolFlagsClass != 0 && symbol.Flags&(ast.SymbolFlagsFunction|ast.SymbolFlagsVariable) == 0 && node.Kind == ast.KindConstructorKeyword {
if constructor := symbol.Members[ast.InternalSymbolNameConstructor]; constructor != nil {
symbol = constructor
}
}
if symbol.Flags&ast.SymbolFlagsAlias != 0 {
if resolved, ok := c.ResolveAlias(symbol); ok {
symbol = resolved
}
}
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 declarations
}
}
}
return symbol.Declarations
}
if indexInfos := c.GetIndexSignaturesAtLocation(node); len(indexInfos) != 0 {
return indexInfos
}
return nil
}

// Returns a CallLikeExpression where `node` is the target being invoked.
func getAncestorCallLikeExpression(node *ast.Node) *ast.Node {
target := ast.FindAncestor(node, func(n *ast.Node) bool {
return !isRightSideOfPropertyAccess(n)
Expand Down
2 changes: 1 addition & 1 deletion testdata/baselines/reference/fourslash/goToDef/GoToDefinitionAmbiants.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

// declare var ambientVar;
// declare function ambientFunction();
// declare class ambientClass {
// declare class [|ambientClass|] {
// [|constructor();|]
// static method();
// public method();
Expand Down
30 changes: 16 additions & 14 deletions ...data/baselines/reference/fourslash/goToDef/GoToDefinitionClassConstructors.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
// [|constructor(protected readonly cArg: string) {}|]
// }
//
// export class Derived extends Base {
// // --- (line: 6) skipped ---
// export class [|Derived|] extends Base {
// readonly email = this.cArg.getByLabel('Email')
// readonly password = this.cArg.getByLabel('Password')
// }


// === /main.ts ===
Expand All @@ -18,6 +20,16 @@


// === goToDefinition ===
// === /defInSameFile.ts ===

// import { Base } from './definitions'
// class [|SameFile|] extends Base {
// readonly name: string = 'SameFile'
// }
// const SameFile = new /*GO TO DEFINITION*/SameFile(cArg)
// const wrapper = new Base(cArg)


// === /definitions.ts ===

// export class Base {
Expand All @@ -28,23 +40,13 @@
// // --- (line: 6) skipped ---


// === /defInSameFile.ts ===

// import { Base } from './definitions'
// class SameFile extends Base {
// readonly name: string = 'SameFile'
// }
// const SameFile = new /*GO TO DEFINITION*/SameFile(cArg)
// const wrapper = new Base(cArg)




// === goToDefinition ===
// === /hasConstructor.ts ===

// import { Base } from './definitions'
// class HasConstructor extends Base {
// class [|HasConstructor|] extends Base {
// [|constructor() {}|]
// readonly name: string = '';
// }
Expand All @@ -56,7 +58,7 @@
// === goToDefinition ===
// === /definitions.ts ===

// export class Base {
// export class [|Base|] {
// [|constructor(protected readonly cArg: string) {}|]
// }
//
Expand Down
39 changes: 24 additions & 15 deletions ...s/reference/fourslash/goToDef/GoToDefinitionConstructorOfClassExpression01.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOfClassExpression01.ts ===

// var x = class C {
// var x = class [|C|] {
// [|constructor() {
// var other = new /*GO TO DEFINITION*/C;
// }|]
Expand All @@ -16,10 +16,11 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOfClassExpression01.ts ===

// --- (line: 4) skipped ---
// --- (line: 3) skipped ---
// }
// }
//
// var y = class C extends x {
// var y = class [|C|] extends x {
// [|constructor() {
// super();
// var other = new /*GO TO DEFINITION*/C;
Expand All @@ -42,12 +43,12 @@
// }
//
// var y = class C extends x {
// // --- (line: 8) skipped ---


// --- (line: 11) skipped ---
// constructor() {
// super();
// var other = new C;
// }
// }
// var z = class C extends x {
// var z = class [|C|] extends x {
// m() {
// return new /*GO TO DEFINITION*/C;
// }
Expand Down Expand Up @@ -76,7 +77,7 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOfClassExpression01.ts ===

// var x = class C {
// var [|x|] = class C {
// [|constructor() {
// var other = new C;
// }|]
Expand All @@ -100,10 +101,11 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOfClassExpression01.ts ===

// --- (line: 4) skipped ---
// --- (line: 3) skipped ---
// }
// }
//
// var y = class C extends x {
// var [|y|] = class C extends x {
// [|constructor() {
// super();
// var other = new C;
Expand Down Expand Up @@ -133,10 +135,17 @@
// }
//
// var y = class C extends x {
// // --- (line: 8) skipped ---


// --- (line: 18) skipped ---
// constructor() {
// super();
// var other = new C;
// }
// }
// var [|z|] = class C extends x {
// m() {
// return new C;
// }
// }
//
// var x1 = new C();
// var x2 = new x();
// var y1 = new y();
Expand Down
4 changes: 2 additions & 2 deletions ...h/goToDef/GoToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.ts ===

// namespace Foo {
// namespace [|Foo|] {
// export var x;
// }
//
// class Foo {
// class [|Foo|] {
// [|constructor() {
// }|]
// }
Expand Down
26 changes: 13 additions & 13 deletions ...a/baselines/reference/fourslash/goToDef/GoToDefinitionConstructorOverloads.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOverloads.ts ===

// class ConstructorOverload {
// class [|ConstructorOverload|] {
// [|constructor();|]
// constructor(foo: string);
// constructor(foo: any) { }
Expand All @@ -19,7 +19,7 @@
// === goToDefinition ===
// === /goToDefinitionConstructorOverloads.ts ===

// class ConstructorOverload {
// class [|ConstructorOverload|] {
// constructor();
// [|constructor(foo: string);|]
// constructor(foo: any) { }
Expand All @@ -39,8 +39,8 @@
// === /goToDefinitionConstructorOverloads.ts ===

// class ConstructorOverload {
// /*GO TO DEFINITION*/constructor();
// constructor(foo: string);
// /*GO TO DEFINITION*/[|constructor();|]
// [|constructor(foo: string);|]
// [|constructor(foo: any) { }|]
// }
//
Expand All @@ -58,11 +58,11 @@
// constructor(foo: string);
// constructor(foo: any) { }
// }
// // --- (line: 6) skipped ---


// --- (line: 9) skipped ---
// class Extended extends ConstructorOverload {
//
// var constructorOverload = new ConstructorOverload();
// var constructorOverload = new ConstructorOverload("foo");
//
// class [|Extended|] extends ConstructorOverload {
// readonly name = "extended";
// }
// var extended1 = new /*GO TO DEFINITION*/Extended();
Expand All @@ -80,10 +80,10 @@
// constructor(foo: any) { }
// }
//
// // --- (line: 7) skipped ---


// --- (line: 10) skipped ---
// var constructorOverload = new ConstructorOverload();
// var constructorOverload = new ConstructorOverload("foo");
//
// class [|Extended|] extends ConstructorOverload {
// readonly name = "extended";
// }
// var extended1 = new Extended();
Expand Down
2 changes: 1 addition & 1 deletion testdata/baselines/reference/fourslash/goToDef/GoToDefinitionExpandoClass2.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

// const Core = {}
//
// Core.Test = class {
// Core.[|Test|] = class {
// [|constructor() { }|]
// }
//
Expand Down
4 changes: 2 additions & 2 deletions ...data/baselines/reference/fourslash/goToDef/GoToDefinitionFunctionOverloads.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
// === goToDefinition ===
// === /goToDefinitionFunctionOverloads.ts ===

// function /*GO TO DEFINITION*/functionOverload(value: number);
// function functionOverload(value: string);
// function /*GO TO DEFINITION*/[|functionOverload|](value: number);
// function [|functionOverload|](value: string);
// function [|functionOverload|]() {}
//
// functionOverload(123);
Expand Down
8 changes: 4 additions & 4 deletions ...selines/reference/fourslash/goToDef/GoToDefinitionFunctionOverloadsInClass.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// === /goToDefinitionFunctionOverloadsInClass.ts ===

// class clsInOverload {
// static fnOverload();
// static /*GO TO DEFINITION*/fnOverload(foo: string);
// static [|fnOverload|]();
// static /*GO TO DEFINITION*/[|fnOverload|](foo: string);
// static [|fnOverload|](foo: any) { }
// public fnOverload(): any;
// public fnOverload(foo: string);
Expand All @@ -20,8 +20,8 @@
// static fnOverload();
// static fnOverload(foo: string);
// static fnOverload(foo: any) { }
// public /*GO TO DEFINITION*/fnOverload(): any;
// public fnOverload(foo: string);
// public /*GO TO DEFINITION*/[|fnOverload|](): any;
// public [|fnOverload|](foo: string);
// public [|fnOverload|](foo: any) { return "foo" }
//
// constructor() { }
Expand Down
Loading
Loading