Skip to content

Commit cedc0cb

Browse files
ahejlsbergahejlsberg
authored
Go-to-def should include both declaration(s) and target call signature (#1543)
1 parent e86ed12 commit cedc0cb

19 files changed

+123
-111
lines changed

‎internal/ls/definition.go

Lines changed: 46 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package ls
22

33
import (
44
"context"
5+
"slices"
56

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

49-
if calledDeclaration := tryGetSignatureDeclaration(c, node); calledDeclaration != nil {
50-
return l.createLocationsFromDeclarations([]*ast.Node{calledDeclaration}), nil
50+
declarations := getDeclarationsFromLocation(c, node)
51+
calledDeclaration := tryGetSignatureDeclaration(c, node)
52+
if calledDeclaration != nil {
53+
// If we can resolve a call signature, remove all function-like declarations and add that signature.
54+
nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) })
55+
declarations = append(nonFunctionDeclarations, calledDeclaration)
5156
}
52-
53-
if ast.IsIdentifier(node) && ast.IsShorthandPropertyAssignment(node.Parent) {
54-
return l.createLocationsFromDeclarations(c.GetResolvedSymbol(node).Declarations), nil
55-
}
56-
57-
node = getDeclarationNameForKeyword(node)
58-
59-
if symbol := c.GetSymbolAtLocation(node); symbol != nil {
60-
if symbol.Flags&ast.SymbolFlagsClass != 0 && symbol.Flags&(ast.SymbolFlagsFunction|ast.SymbolFlagsVariable) == 0 && node.Kind == ast.KindConstructorKeyword {
61-
if constructor := symbol.Members[ast.InternalSymbolNameConstructor]; constructor != nil {
62-
symbol = constructor
63-
}
64-
}
65-
if symbol.Flags&ast.SymbolFlagsAlias != 0 {
66-
if resolved, ok := c.ResolveAlias(symbol); ok {
67-
symbol = resolved
68-
}
69-
}
70-
if symbol.Flags&(ast.SymbolFlagsProperty|ast.SymbolFlagsMethod|ast.SymbolFlagsAccessor) != 0 && symbol.Parent != nil && symbol.Parent.Flags&ast.SymbolFlagsObjectLiteral != 0 {
71-
if objectLiteral := core.FirstOrNil(symbol.Parent.Declarations); objectLiteral != nil {
72-
if declarations := c.GetContextualDeclarationsForObjectLiteralElement(objectLiteral, symbol.Name); len(declarations) != 0 {
73-
return l.createLocationsFromDeclarations(declarations), nil
74-
}
75-
}
76-
}
77-
return l.createLocationsFromDeclarations(symbol.Declarations), nil
78-
}
79-
80-
if indexInfos := c.GetIndexSignaturesAtLocation(node); len(indexInfos) != 0 {
81-
return l.createLocationsFromDeclarations(indexInfos), nil
82-
}
83-
84-
return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{}, nil
57+
return l.createLocationsFromDeclarations(declarations), nil
8558
}
8659

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

129102
func (l *LanguageService) createLocationsFromDeclarations(declarations []*ast.Node) lsproto.DefinitionResponse {
130-
someHaveBody := core.Some(declarations, func(node *ast.Node) bool { return node.Body() != nil })
131103
locations := make([]lsproto.Location, 0, len(declarations))
132104
for _, decl := range declarations {
133-
if !someHaveBody || decl.Body() != nil {
134-
file := ast.GetSourceFileOfNode(decl)
135-
name := core.OrElse(ast.GetNameOfDeclaration(decl), decl)
136-
locations = append(locations, lsproto.Location{
137-
Uri: FileNameToDocumentURI(file.FileName()),
138-
Range: *l.createLspRangeFromNode(name, file),
139-
})
140-
}
105+
file := ast.GetSourceFileOfNode(decl)
106+
name := core.OrElse(ast.GetNameOfDeclaration(decl), decl)
107+
locations = core.AppendIfUnique(locations, lsproto.Location{
108+
Uri: FileNameToDocumentURI(file.FileName()),
109+
Range: *l.createLspRangeFromNode(name, file),
110+
})
141111
}
142112
return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{Locations: &locations}
143113
}
@@ -151,7 +121,38 @@ func (l *LanguageService) createLocationFromFileAndRange(file *ast.SourceFile, t
151121
}
152122
}
153123

154-
/** Returns a CallLikeExpression where `node` is the target being invoked. */
124+
func getDeclarationsFromLocation(c *checker.Checker, node *ast.Node) []*ast.Node {
125+
if ast.IsIdentifier(node) && ast.IsShorthandPropertyAssignment(node.Parent) {
126+
return c.GetResolvedSymbol(node).Declarations
127+
}
128+
node = getDeclarationNameForKeyword(node)
129+
if symbol := c.GetSymbolAtLocation(node); symbol != nil {
130+
if symbol.Flags&ast.SymbolFlagsClass != 0 && symbol.Flags&(ast.SymbolFlagsFunction|ast.SymbolFlagsVariable) == 0 && node.Kind == ast.KindConstructorKeyword {
131+
if constructor := symbol.Members[ast.InternalSymbolNameConstructor]; constructor != nil {
132+
symbol = constructor
133+
}
134+
}
135+
if symbol.Flags&ast.SymbolFlagsAlias != 0 {
136+
if resolved, ok := c.ResolveAlias(symbol); ok {
137+
symbol = resolved
138+
}
139+
}
140+
if symbol.Flags&(ast.SymbolFlagsProperty|ast.SymbolFlagsMethod|ast.SymbolFlagsAccessor) != 0 && symbol.Parent != nil && symbol.Parent.Flags&ast.SymbolFlagsObjectLiteral != 0 {
141+
if objectLiteral := core.FirstOrNil(symbol.Parent.Declarations); objectLiteral != nil {
142+
if declarations := c.GetContextualDeclarationsForObjectLiteralElement(objectLiteral, symbol.Name); len(declarations) != 0 {
143+
return declarations
144+
}
145+
}
146+
}
147+
return symbol.Declarations
148+
}
149+
if indexInfos := c.GetIndexSignaturesAtLocation(node); len(indexInfos) != 0 {
150+
return indexInfos
151+
}
152+
return nil
153+
}
154+
155+
// Returns a CallLikeExpression where `node` is the target being invoked.
155156
func getAncestorCallLikeExpression(node *ast.Node) *ast.Node {
156157
target := ast.FindAncestor(node, func(n *ast.Node) bool {
157158
return !isRightSideOfPropertyAccess(n)

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionAmbiants.baseline.jsonc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
// declare var ambientVar;
4545
// declare function ambientFunction();
46-
// declare class ambientClass {
46+
// declare class [|ambientClass|] {
4747
// [|constructor();|]
4848
// static method();
4949
// public method();

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionClassConstructors.baseline.jsonc

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
// [|constructor(protected readonly cArg: string) {}|]
66
// }
77
//
8-
// export class Derived extends Base {
9-
// // --- (line: 6) skipped ---
8+
// export class [|Derived|] extends Base {
9+
// readonly email = this.cArg.getByLabel('Email')
10+
// readonly password = this.cArg.getByLabel('Password')
11+
// }
1012

1113

1214
// === /main.ts ===
@@ -18,6 +20,16 @@
1820

1921

2022
// === goToDefinition ===
23+
// === /defInSameFile.ts ===
24+
25+
// import { Base } from './definitions'
26+
// class [|SameFile|] extends Base {
27+
// readonly name: string = 'SameFile'
28+
// }
29+
// const SameFile = new /*GO TO DEFINITION*/SameFile(cArg)
30+
// const wrapper = new Base(cArg)
31+
32+
2133
// === /definitions.ts ===
2234

2335
// export class Base {
@@ -28,23 +40,13 @@
2840
// // --- (line: 6) skipped ---
2941

3042

31-
// === /defInSameFile.ts ===
32-
33-
// import { Base } from './definitions'
34-
// class SameFile extends Base {
35-
// readonly name: string = 'SameFile'
36-
// }
37-
// const SameFile = new /*GO TO DEFINITION*/SameFile(cArg)
38-
// const wrapper = new Base(cArg)
39-
40-
4143

4244

4345
// === goToDefinition ===
4446
// === /hasConstructor.ts ===
4547

4648
// import { Base } from './definitions'
47-
// class HasConstructor extends Base {
49+
// class [|HasConstructor|] extends Base {
4850
// [|constructor() {}|]
4951
// readonly name: string = '';
5052
// }
@@ -56,7 +58,7 @@
5658
// === goToDefinition ===
5759
// === /definitions.ts ===
5860

59-
// export class Base {
61+
// export class [|Base|] {
6062
// [|constructor(protected readonly cArg: string) {}|]
6163
// }
6264
//

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionConstructorOfClassExpression01.baseline.jsonc

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// === goToDefinition ===
22
// === /goToDefinitionConstructorOfClassExpression01.ts ===
33

4-
// var x = class C {
4+
// var x = class [|C|] {
55
// [|constructor() {
66
// var other = new /*GO TO DEFINITION*/C;
77
// }|]
@@ -16,10 +16,11 @@
1616
// === goToDefinition ===
1717
// === /goToDefinitionConstructorOfClassExpression01.ts ===
1818

19-
// --- (line: 4) skipped ---
19+
// --- (line: 3) skipped ---
20+
// }
2021
// }
2122
//
22-
// var y = class C extends x {
23+
// var y = class [|C|] extends x {
2324
// [|constructor() {
2425
// super();
2526
// var other = new /*GO TO DEFINITION*/C;
@@ -42,12 +43,12 @@
4243
// }
4344
//
4445
// var y = class C extends x {
45-
// // --- (line: 8) skipped ---
46-
47-
48-
// --- (line: 11) skipped ---
46+
// constructor() {
47+
// super();
48+
// var other = new C;
49+
// }
4950
// }
50-
// var z = class C extends x {
51+
// var z = class [|C|] extends x {
5152
// m() {
5253
// return new /*GO TO DEFINITION*/C;
5354
// }
@@ -76,7 +77,7 @@
7677
// === goToDefinition ===
7778
// === /goToDefinitionConstructorOfClassExpression01.ts ===
7879

79-
// var x = class C {
80+
// var [|x|] = class C {
8081
// [|constructor() {
8182
// var other = new C;
8283
// }|]
@@ -100,10 +101,11 @@
100101
// === goToDefinition ===
101102
// === /goToDefinitionConstructorOfClassExpression01.ts ===
102103

103-
// --- (line: 4) skipped ---
104+
// --- (line: 3) skipped ---
105+
// }
104106
// }
105107
//
106-
// var y = class C extends x {
108+
// var [|y|] = class C extends x {
107109
// [|constructor() {
108110
// super();
109111
// var other = new C;
@@ -133,10 +135,17 @@
133135
// }
134136
//
135137
// var y = class C extends x {
136-
// // --- (line: 8) skipped ---
137-
138-
139-
// --- (line: 18) skipped ---
138+
// constructor() {
139+
// super();
140+
// var other = new C;
141+
// }
142+
// }
143+
// var [|z|] = class C extends x {
144+
// m() {
145+
// return new C;
146+
// }
147+
// }
148+
//
140149
// var x1 = new C();
141150
// var x2 = new x();
142151
// var y1 = new y();

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.baseline.jsonc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// === goToDefinition ===
22
// === /goToDefinitionConstructorOfClassWhenClassIsPrecededByNamespace01.ts ===
33

4-
// namespace Foo {
4+
// namespace [|Foo|] {
55
// export var x;
66
// }
77
//
8-
// class Foo {
8+
// class [|Foo|] {
99
// [|constructor() {
1010
// }|]
1111
// }

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionConstructorOverloads.baseline.jsonc

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// === goToDefinition ===
22
// === /goToDefinitionConstructorOverloads.ts ===
33

4-
// class ConstructorOverload {
4+
// class [|ConstructorOverload|] {
55
// [|constructor();|]
66
// constructor(foo: string);
77
// constructor(foo: any) { }
@@ -19,7 +19,7 @@
1919
// === goToDefinition ===
2020
// === /goToDefinitionConstructorOverloads.ts ===
2121

22-
// class ConstructorOverload {
22+
// class [|ConstructorOverload|] {
2323
// constructor();
2424
// [|constructor(foo: string);|]
2525
// constructor(foo: any) { }
@@ -39,8 +39,8 @@
3939
// === /goToDefinitionConstructorOverloads.ts ===
4040

4141
// class ConstructorOverload {
42-
// /*GO TO DEFINITION*/constructor();
43-
// constructor(foo: string);
42+
// /*GO TO DEFINITION*/[|constructor();|]
43+
// [|constructor(foo: string);|]
4444
// [|constructor(foo: any) { }|]
4545
// }
4646
//
@@ -58,11 +58,11 @@
5858
// constructor(foo: string);
5959
// constructor(foo: any) { }
6060
// }
61-
// // --- (line: 6) skipped ---
62-
63-
64-
// --- (line: 9) skipped ---
65-
// class Extended extends ConstructorOverload {
61+
//
62+
// var constructorOverload = new ConstructorOverload();
63+
// var constructorOverload = new ConstructorOverload("foo");
64+
//
65+
// class [|Extended|] extends ConstructorOverload {
6666
// readonly name = "extended";
6767
// }
6868
// var extended1 = new /*GO TO DEFINITION*/Extended();
@@ -80,10 +80,10 @@
8080
// constructor(foo: any) { }
8181
// }
8282
//
83-
// // --- (line: 7) skipped ---
84-
85-
86-
// --- (line: 10) skipped ---
83+
// var constructorOverload = new ConstructorOverload();
84+
// var constructorOverload = new ConstructorOverload("foo");
85+
//
86+
// class [|Extended|] extends ConstructorOverload {
8787
// readonly name = "extended";
8888
// }
8989
// var extended1 = new Extended();

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionExpandoClass2.baseline.jsonc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
// const Core = {}
55
//
6-
// Core.Test = class {
6+
// Core.[|Test|] = class {
77
// [|constructor() { }|]
88
// }
99
//

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionFunctionOverloads.baseline.jsonc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@
4343
// === goToDefinition ===
4444
// === /goToDefinitionFunctionOverloads.ts ===
4545

46-
// function /*GO TO DEFINITION*/functionOverload(value: number);
47-
// function functionOverload(value: string);
46+
// function /*GO TO DEFINITION*/[|functionOverload|](value: number);
47+
// function [|functionOverload|](value: string);
4848
// function [|functionOverload|]() {}
4949
//
5050
// functionOverload(123);

‎testdata/baselines/reference/fourslash/goToDef/GoToDefinitionFunctionOverloadsInClass.baseline.jsonc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// === /goToDefinitionFunctionOverloadsInClass.ts ===
33

44
// class clsInOverload {
5-
// static fnOverload();
6-
// static /*GO TO DEFINITION*/fnOverload(foo: string);
5+
// static [|fnOverload|]();
6+
// static /*GO TO DEFINITION*/[|fnOverload|](foo: string);
77
// static [|fnOverload|](foo: any) { }
88
// public fnOverload(): any;
99
// public fnOverload(foo: string);
@@ -20,8 +20,8 @@
2020
// static fnOverload();
2121
// static fnOverload(foo: string);
2222
// static fnOverload(foo: any) { }
23-
// public /*GO TO DEFINITION*/fnOverload(): any;
24-
// public fnOverload(foo: string);
23+
// public /*GO TO DEFINITION*/[|fnOverload|](): any;
24+
// public [|fnOverload|](foo: string);
2525
// public [|fnOverload|](foo: any) { return "foo" }
2626
//
2727
// constructor() { }

0 commit comments

Comments
 (0)