Skip to content

Commit af9b56b

Browse files
DanielRosenwasserDanielRosenwasser
committed
Merge pull request microsoft#1528 from Microsoft/goToDefOnUnionMethod
Fix crash on go-to-def on union method
2 parents 6fed3e2 + b3ccb56 commit af9b56b

10 files changed

Lines changed: 188 additions & 126 deletions

‎src/harness/fourslash.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,14 @@ module FourSlash {
15151515
}
15161516
}
15171517

1518+
public verifyDefinitionsCount(negative: boolean, expectedCount: number) {
1519+
var assertFn = negative ? assert.notEqual : assert.equal;
1520+
1521+
var definitions = this.languageService.getDefinitionAtPosition(this.activeFile.fileName, this.currentCaretPosition);
1522+
1523+
assertFn(definitions.length, expectedCount, this.messageAtLastKnownMarker("Definitions Count"));
1524+
}
1525+
15181526
public verifyDefinitionsName(negative: boolean, expectedName: string, expectedContainerName: string) {
15191527
this.taoInvalidReason = 'verifyDefinititionsInfo NYI';
15201528

@@ -1523,10 +1531,10 @@ module FourSlash {
15231531
var actualDefinitionContainerName = definitions && definitions.length ? definitions[0].containerName : "";
15241532
if (negative) {
15251533
assert.notEqual(actualDefinitionName, expectedName, this.messageAtLastKnownMarker("Definition Info Name"));
1526-
assert.notEqual(actualDefinitionName, expectedName, this.messageAtLastKnownMarker("Definition Info Container Name"));
1534+
assert.notEqual(actualDefinitionContainerName, expectedContainerName, this.messageAtLastKnownMarker("Definition Info Container Name"));
15271535
} else {
15281536
assert.equal(actualDefinitionName, expectedName, this.messageAtLastKnownMarker("Definition Info Name"));
1529-
assert.equal(actualDefinitionName, expectedName, this.messageAtLastKnownMarker("Definition Info Container Name"));
1537+
assert.equal(actualDefinitionContainerName, expectedContainerName, this.messageAtLastKnownMarker("Definition Info Container Name"));
15301538
}
15311539
}
15321540

‎src/services/services.ts‎

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,7 +2598,7 @@ module ts {
25982598
}
25992599

26002600
// TODO(drosen): use contextual SemanticMeaning.
2601-
function getSymbolKind(symbol: Symbol, typeResolver: TypeChecker, location?: Node): string {
2601+
function getSymbolKind(symbol: Symbol, typeResolver: TypeChecker, location: Node): string {
26022602
var flags = symbol.getFlags();
26032603

26042604
if (flags & SymbolFlags.Class) return ScriptElementKind.classElement;
@@ -3097,62 +3097,6 @@ module ts {
30973097

30983098
/// Goto definition
30993099
function getDefinitionAtPosition(filename: string, position: number): DefinitionInfo[] {
3100-
function getDefinitionInfo(node: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
3101-
return {
3102-
fileName: node.getSourceFile().filename,
3103-
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
3104-
kind: symbolKind,
3105-
name: symbolName,
3106-
containerKind: undefined,
3107-
containerName
3108-
};
3109-
}
3110-
3111-
function tryAddSignature(signatureDeclarations: Declaration[], selectConstructors: boolean, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3112-
var declarations: Declaration[] = [];
3113-
var definition: Declaration;
3114-
3115-
forEach(signatureDeclarations, d => {
3116-
if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
3117-
(!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature))) {
3118-
declarations.push(d);
3119-
if ((<FunctionLikeDeclaration>d).body) definition = d;
3120-
}
3121-
});
3122-
3123-
if (definition) {
3124-
result.push(getDefinitionInfo(definition, symbolKind, symbolName, containerName));
3125-
return true;
3126-
}
3127-
else if (declarations.length) {
3128-
result.push(getDefinitionInfo(declarations[declarations.length - 1], symbolKind, symbolName, containerName));
3129-
return true;
3130-
}
3131-
3132-
return false;
3133-
}
3134-
3135-
function tryAddConstructSignature(symbol: Symbol, location: Node, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3136-
// Applicable only if we are in a new expression, or we are on a constructor declaration
3137-
// and in either case the symbol has a construct signature definition, i.e. class
3138-
if (isNewExpressionTarget(location) || location.kind === SyntaxKind.ConstructorKeyword) {
3139-
if (symbol.flags & SymbolFlags.Class) {
3140-
var classDeclaration = <ClassDeclaration>symbol.getDeclarations()[0];
3141-
Debug.assert(classDeclaration && classDeclaration.kind === SyntaxKind.ClassDeclaration);
3142-
3143-
return tryAddSignature(classDeclaration.members, /*selectConstructors*/ true, symbolKind, symbolName, containerName, result);
3144-
}
3145-
}
3146-
return false;
3147-
}
3148-
3149-
function tryAddCallSignature(symbol: Symbol, location: Node, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3150-
if (isCallExpressionTarget(location) || isNewExpressionTarget(location) || isNameOfFunctionDeclaration(location)) {
3151-
return tryAddSignature(symbol.declarations, /*selectConstructors*/ false, symbolKind, symbolName, containerName, result);
3152-
}
3153-
return false;
3154-
}
3155-
31563100
synchronizeHostData();
31573101

31583102
filename = normalizeSlashes(filename);
@@ -3205,7 +3149,7 @@ module ts {
32053149
if (node.parent.kind === SyntaxKind.ShorthandPropertyAssignment) {
32063150
var shorthandSymbol = typeInfoResolver.getShorthandAssignmentValueSymbol(symbol.valueDeclaration);
32073151
var shorthandDeclarations = shorthandSymbol.getDeclarations();
3208-
var shorthandSymbolKind = getSymbolKind(shorthandSymbol, typeInfoResolver);
3152+
var shorthandSymbolKind = getSymbolKind(shorthandSymbol, typeInfoResolver, node);
32093153
var shorthandSymbolName = typeInfoResolver.symbolToString(shorthandSymbol);
32103154
var shorthandContainerName = typeInfoResolver.symbolToString(symbol.parent, node);
32113155
forEach(shorthandDeclarations, declaration => {
@@ -3216,7 +3160,7 @@ module ts {
32163160

32173161
var declarations = symbol.getDeclarations();
32183162
var symbolName = typeInfoResolver.symbolToString(symbol); // Do not get scoped name, just the name of the symbol
3219-
var symbolKind = getSymbolKind(symbol, typeInfoResolver);
3163+
var symbolKind = getSymbolKind(symbol, typeInfoResolver, node);
32203164
var containerSymbol = symbol.parent;
32213165
var containerName = containerSymbol ? typeInfoResolver.symbolToString(containerSymbol, node) : "";
32223166

@@ -3229,6 +3173,62 @@ module ts {
32293173
}
32303174

32313175
return result;
3176+
3177+
function getDefinitionInfo(node: Node, symbolKind: string, symbolName: string, containerName: string): DefinitionInfo {
3178+
return {
3179+
fileName: node.getSourceFile().filename,
3180+
textSpan: createTextSpanFromBounds(node.getStart(), node.getEnd()),
3181+
kind: symbolKind,
3182+
name: symbolName,
3183+
containerKind: undefined,
3184+
containerName
3185+
};
3186+
}
3187+
3188+
function tryAddSignature(signatureDeclarations: Declaration[], selectConstructors: boolean, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3189+
var declarations: Declaration[] = [];
3190+
var definition: Declaration;
3191+
3192+
forEach(signatureDeclarations, d => {
3193+
if ((selectConstructors && d.kind === SyntaxKind.Constructor) ||
3194+
(!selectConstructors && (d.kind === SyntaxKind.FunctionDeclaration || d.kind === SyntaxKind.MethodDeclaration || d.kind === SyntaxKind.MethodSignature))) {
3195+
declarations.push(d);
3196+
if ((<FunctionLikeDeclaration>d).body) definition = d;
3197+
}
3198+
});
3199+
3200+
if (definition) {
3201+
result.push(getDefinitionInfo(definition, symbolKind, symbolName, containerName));
3202+
return true;
3203+
}
3204+
else if (declarations.length) {
3205+
result.push(getDefinitionInfo(declarations[declarations.length - 1], symbolKind, symbolName, containerName));
3206+
return true;
3207+
}
3208+
3209+
return false;
3210+
}
3211+
3212+
function tryAddConstructSignature(symbol: Symbol, location: Node, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3213+
// Applicable only if we are in a new expression, or we are on a constructor declaration
3214+
// and in either case the symbol has a construct signature definition, i.e. class
3215+
if (isNewExpressionTarget(location) || location.kind === SyntaxKind.ConstructorKeyword) {
3216+
if (symbol.flags & SymbolFlags.Class) {
3217+
var classDeclaration = <ClassDeclaration>symbol.getDeclarations()[0];
3218+
Debug.assert(classDeclaration && classDeclaration.kind === SyntaxKind.ClassDeclaration);
3219+
3220+
return tryAddSignature(classDeclaration.members, /*selectConstructors*/ true, symbolKind, symbolName, containerName, result);
3221+
}
3222+
}
3223+
return false;
3224+
}
3225+
3226+
function tryAddCallSignature(symbol: Symbol, location: Node, symbolKind: string, symbolName: string, containerName: string, result: DefinitionInfo[]) {
3227+
if (isCallExpressionTarget(location) || isNewExpressionTarget(location) || isNameOfFunctionDeclaration(location)) {
3228+
return tryAddSignature(symbol.declarations, /*selectConstructors*/ false, symbolKind, symbolName, containerName, result);
3229+
}
3230+
return false;
3231+
}
32323232
}
32333233

32343234
/// References and Occurrences
@@ -5273,7 +5273,7 @@ module ts {
52735273

52745274
// Only allow a symbol to be renamed if it actually has at least one declaration.
52755275
if (symbol && symbol.getDeclarations() && symbol.getDeclarations().length > 0) {
5276-
var kind = getSymbolKind(symbol, typeInfoResolver);
5276+
var kind = getSymbolKind(symbol, typeInfoResolver, node);
52775277
if (kind) {
52785278
return getRenameInfo(symbol.name, typeInfoResolver.getFullyQualifiedName(symbol), kind,
52795279
getSymbolModifiers(symbol),

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,10 @@ module FourSlashInterface {
217217
FourSlash.currentTestState.verifyQuickInfoExists(this.negative);
218218
}
219219

220+
public definitionCountIs(expectedCount: number) {
221+
FourSlash.currentTestState.verifyDefinitionsCount(this.negative, expectedCount);
222+
}
223+
220224
public definitionLocationExists() {
221225
FourSlash.currentTestState.verifyDefinitionLocationExists(this.negative);
222226
}

‎tests/cases/fourslash/goToDefinitionExternamModuleName4.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/// <reference path='fourslash.ts'/>
1+
/// <reference path='fourslash.ts'/>
22

33
// @Filename: b.ts
44
////import n = require('unknown/*1*/');

‎tests/cases/fourslash/goToDefinitionFunctionOverloads.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
1-
/// <reference path='fourslash.ts' />
2-
3-
/////*functionOverload1*/function /*functionOverload*/functionOverload();
4-
/////*functionOverload2*/function functionOverload(value: string);
5-
/////*functionOverloadDefinition*/function functionOverload() {}
6-
////
1+
/// <reference path='fourslash.ts' />
2+
3+
/////*functionOverload1*/function /*functionOverload*/functionOverload();
4+
/////*functionOverload2*/function functionOverload(value: string);
5+
/////*functionOverloadDefinition*/function functionOverload() {}
6+
////
77
/////*functionOverloadReference1*/functionOverload();
88
/////*functionOverloadReference2*/functionOverload("123");
99

10-
goTo.marker('functionOverloadReference1');
11-
goTo.definition();
12-
verify.caretAtMarker('functionOverloadDefinition');
13-
14-
goTo.marker('functionOverloadReference2');
15-
goTo.definition();
16-
verify.caretAtMarker('functionOverloadDefinition');
17-
18-
goTo.marker('functionOverload');
19-
goTo.definition();
20-
verify.caretAtMarker('functionOverloadDefinition');
10+
goTo.marker('functionOverloadReference1');
11+
goTo.definition();
12+
verify.caretAtMarker('functionOverloadDefinition');
13+
14+
goTo.marker('functionOverloadReference2');
15+
goTo.definition();
16+
verify.caretAtMarker('functionOverloadDefinition');
17+
18+
goTo.marker('functionOverload');
19+
goTo.definition();
20+
verify.caretAtMarker('functionOverloadDefinition');

‎tests/cases/fourslash/goToDefinitionFunctionOverloadsInClass.ts‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
1-
/// <reference path='fourslash.ts'/>
2-
3-
////class clsInOverload {
4-
//// static fnOverload();
5-
//// static /*staticFunctionOverload*/fnOverload(foo: string);
6-
//// /*staticFunctionOverloadDefinition*/static fnOverload(foo: any) { }
7-
//// public /*functionOverload*/fnOverload(): any;
8-
//// public fnOverload(foo: string);
9-
//// /*functionOverloadDefinition*/public fnOverload(foo: any) { return "foo" }
10-
////
11-
//// constructor() { }
12-
////}
13-
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class clsInOverload {
4+
//// static fnOverload();
5+
//// static /*staticFunctionOverload*/fnOverload(foo: string);
6+
//// /*staticFunctionOverloadDefinition*/static fnOverload(foo: any) { }
7+
//// public /*functionOverload*/fnOverload(): any;
8+
//// public fnOverload(foo: string);
9+
//// /*functionOverloadDefinition*/public fnOverload(foo: any) { return "foo" }
10+
////
11+
//// constructor() { }
12+
////}
13+
1414
// this line triggers a semantic/syntactic error check, remove line when 788570 is fixed
15-
edit.insert('');
16-
17-
goTo.marker('staticFunctionOverload');
18-
goTo.definition();
19-
verify.caretAtMarker('staticFunctionOverloadDefinition');
20-
21-
goTo.marker('functionOverload');
22-
goTo.definition();
15+
edit.insert('');
16+
17+
goTo.marker('staticFunctionOverload');
18+
goTo.definition();
19+
verify.caretAtMarker('staticFunctionOverloadDefinition');
20+
21+
goTo.marker('functionOverload');
22+
goTo.definition();
2323
verify.caretAtMarker('functionOverloadDefinition');

tests/cases/fourslash/goToDefinitionUnionTypeProperty.ts renamed to tests/cases/fourslash/goToDefinitionUnionTypeProperty1.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
/// <reference path='fourslash.ts' />
2-
1+
/// <reference path='fourslash.ts' />
2+
33
////interface One {
44
//// /*propertyDefinition1*/commonProperty: number;
55
//// commonFunction(): number;
@@ -13,12 +13,14 @@
1313
////var x : One | Two;
1414
////
1515
////x./*propertyReference*/commonProperty;
16-
////x./*3*/commonFunction;
17-
18-
goTo.marker("propertyReference");
19-
goTo.definition(0);
20-
verify.caretAtMarker("propertyDefinition1");
21-
22-
goTo.marker("propertyReference");
23-
goTo.definition(1);
24-
verify.caretAtMarker("propertyDefinition2");
16+
////x./*3*/commonFunction;
17+
18+
19+
goTo.marker("propertyReference");
20+
verify.definitionCountIs(2);
21+
goTo.definition(0);
22+
verify.caretAtMarker("propertyDefinition1");
23+
24+
goTo.marker("propertyReference");
25+
goTo.definition(1);
26+
verify.caretAtMarker("propertyDefinition2");

‎tests/cases/fourslash/goToDefinitionUnionTypeProperty2.ts‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
/// <reference path='fourslash.ts' />
2-
////interface HasAOrB {
3-
//// /*propertyDefinition1*/a: string;
4-
//// b: string;
5-
////}
6-
////
1+
/// <reference path='fourslash.ts' />
2+
////interface HasAOrB {
3+
//// /*propertyDefinition1*/a: string;
4+
//// b: string;
5+
////}
6+
////
77
////interface One {
88
//// common: { /*propertyDefinition2*/a : number; };
99
////}
@@ -15,11 +15,12 @@
1515
////var x : One | Two;
1616
////
1717
////x.common./*propertyReference*/a;
18-
19-
goTo.marker("propertyReference");
20-
goTo.definition(0);
21-
verify.caretAtMarker("propertyDefinition2");
22-
23-
goTo.marker("propertyReference");
24-
goTo.definition(1);
25-
verify.caretAtMarker("propertyDefinition1");
18+
19+
goTo.marker("propertyReference");
20+
verify.definitionCountIs(2);
21+
goTo.definition(0);
22+
verify.caretAtMarker("propertyDefinition2");
23+
24+
goTo.marker("propertyReference");
25+
goTo.definition(1);
26+
verify.caretAtMarker("propertyDefinition1");

‎tests/cases/fourslash/goToDefinitionUnionTypeProperty3.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////interface Array<T> {
4+
//// /*definition*/specialPop(): T
5+
////}
6+
////
7+
////var strings: string[];
8+
////var numbers: number[];
9+
////
10+
////var x = (strings || numbers)./*usage*/specialPop()
11+
12+
goTo.marker("usage");
13+
verify.definitionCountIs(1);
14+
goTo.definition();
15+
verify.caretAtMarker("definition");

0 commit comments

Comments
 (0)