fix(language-service): Support 'go to definition' for two-way bindings (#40185)

Rather than expecting that a position in a template only targets a
single node, this commit adjusts the approach to account for two way
bindings. In particular, we attempt to get definitions for each targeted
node and then return the combination of all results, or `undefined` if
none of the target nodes had definitions.

PR Close #40185
This commit is contained in:
Andrew Scott 2020-12-17 14:43:20 -08:00 committed by atscott
parent d70c26cc06
commit a9d8c228d9
2 changed files with 88 additions and 62 deletions

View File

@ -41,17 +41,29 @@ export class DefinitionBuilder {
}
return getDefinitionForExpressionAtPosition(fileName, position, this.compiler);
}
const definitionMeta = this.getDefinitionMetaAtPosition(templateInfo, position);
const definitionMetas = this.getDefinitionMetaAtPosition(templateInfo, position);
if (definitionMetas === undefined) {
return undefined;
}
const definitions: ts.DefinitionInfo[] = [];
for (const definitionMeta of definitionMetas) {
// The `$event` of event handlers would point to the $event parameter in the shim file, as in
// `_outputHelper(_t3["x"]).subscribe(function ($event): any { $event }) ;`
// If we wanted to return something for this, it would be more appropriate for something like
// `getTypeDefinition`.
if (definitionMeta === undefined || isDollarEvent(definitionMeta.node)) {
if (isDollarEvent(definitionMeta.node)) {
continue;
}
definitions.push(
...(this.getDefinitionsForSymbol({...definitionMeta, ...templateInfo}) ?? []));
}
if (definitions.length === 0) {
return undefined;
}
const definitions = this.getDefinitionsForSymbol({...definitionMeta, ...templateInfo});
return {definitions, textSpan: getTextSpanOfNode(definitionMeta.node)};
return {definitions, textSpan: getTextSpanOfNode(definitionMetas[0].node)};
}
private getDefinitionsForSymbol({symbol, node, parent, component}: DefinitionMeta&
@ -123,42 +135,55 @@ export class DefinitionBuilder {
if (templateInfo === undefined) {
return;
}
const definitionMeta = this.getDefinitionMetaAtPosition(templateInfo, position);
if (definitionMeta === undefined) {
const definitionMetas = this.getDefinitionMetaAtPosition(templateInfo, position);
if (definitionMetas === undefined) {
return undefined;
}
const {symbol, node} = definitionMeta;
const definitions: ts.DefinitionInfo[] = [];
for (const {symbol, node, parent} of definitionMetas) {
switch (symbol.kind) {
case SymbolKind.Directive:
case SymbolKind.DomBinding:
case SymbolKind.Element:
case SymbolKind.Template:
return this.getTypeDefinitionsForTemplateInstance(symbol, node);
definitions.push(...this.getTypeDefinitionsForTemplateInstance(symbol, node));
break;
case SymbolKind.Output:
case SymbolKind.Input: {
const bindingDefs = this.getTypeDefinitionsForSymbols(...symbol.bindings);
definitions.push(...bindingDefs);
// Also attempt to get directive matches for the input name. If there is a directive that
// has the input name as part of the selector, we want to return that as well.
const directiveDefs = this.getDirectiveTypeDefsForBindingNode(
node, definitionMeta.parent, templateInfo.component);
return [...bindingDefs, ...directiveDefs];
const directiveDefs =
this.getDirectiveTypeDefsForBindingNode(node, parent, templateInfo.component);
definitions.push(...directiveDefs);
break;
}
case SymbolKind.Pipe: {
if (symbol.tsSymbol !== null) {
return this.getTypeDefinitionsForSymbols(symbol);
definitions.push(...this.getTypeDefinitionsForSymbols(symbol));
} else {
// If there is no `ts.Symbol` for the pipe transform, we want to return the
// type definition (the pipe class).
return this.getTypeDefinitionsForSymbols(symbol.classSymbol);
definitions.push(...this.getTypeDefinitionsForSymbols(symbol.classSymbol));
}
break;
}
case SymbolKind.Reference:
return this.getTypeDefinitionsForSymbols({shimLocation: symbol.targetLocation});
definitions.push(
...this.getTypeDefinitionsForSymbols({shimLocation: symbol.targetLocation}));
break;
case SymbolKind.Expression:
return this.getTypeDefinitionsForSymbols(symbol);
case SymbolKind.Variable:
return this.getTypeDefinitionsForSymbols({shimLocation: symbol.initializerLocation});
definitions.push(...this.getTypeDefinitionsForSymbols(symbol));
break;
case SymbolKind.Variable: {
definitions.push(
...this.getTypeDefinitionsForSymbols({shimLocation: symbol.initializerLocation}));
break;
}
}
return definitions;
}
}
@ -221,21 +246,26 @@ export class DefinitionBuilder {
}
private getDefinitionMetaAtPosition({template, component}: TemplateInfo, position: number):
DefinitionMeta|undefined {
DefinitionMeta[]|undefined {
const target = getTargetAtPosition(template, position);
if (target === null) {
return undefined;
}
const {context, parent} = target;
const node =
context.kind === TargetNodeKind.TwoWayBindingContext ? context.nodes[0] : context.node;
const nodes =
context.kind === TargetNodeKind.TwoWayBindingContext ? context.nodes : [context.node];
const definitionMetas: DefinitionMeta[] = [];
for (const node of nodes) {
const symbol = this.compiler.getTemplateTypeChecker().getSymbolOfNode(node, component);
if (symbol === null) {
return undefined;
continue;
}
return {node, parent, symbol};
definitionMetas.push({node, parent, symbol});
}
return definitionMetas.length > 0 ? definitionMetas : undefined;
}
}

View File

@ -43,7 +43,7 @@ describe('definitions', () => {
const {position} =
service.overwriteInlineTemplate(APP_COMPONENT, `<ng-templ¦ate></ng-template>`);
const definitionAndBoundSpan = ngLS.getDefinitionAndBoundSpan(APP_COMPONENT, position);
expect(definitionAndBoundSpan!.definitions).toEqual([]);
expect(definitionAndBoundSpan).toBeUndefined();
});
});
@ -181,17 +181,14 @@ describe('definitions', () => {
templateOverride: `<test-comp string-model [(mo¦del)]="title"></test-comp>`,
expectedSpanText: 'model',
});
// TODO(atscott): This should really return 2 definitions, 1 for the input and 1 for the
// output.
// The TemplateTypeChecker also only returns the first match in the TCB for a given
// sourceSpan so even if we also requested the TmplAstBoundEvent, we'd still get back the
// symbol for the
// @Input because the input appears first in the TCB and they have the same sourceSpan.
expect(definitions!.length).toEqual(1);
expect(definitions!.length).toEqual(2);
const [def] = definitions;
expect(def.textSpan).toEqual('model');
expect(def.contextSpan).toEqual(`@Input() model: string = 'model';`);
const [inputDef, outputDef] = definitions;
expect(inputDef.textSpan).toEqual('model');
expect(inputDef.contextSpan).toEqual(`@Input() model: string = 'model';`);
expect(outputDef.textSpan).toEqual('modelChange');
expect(outputDef.contextSpan)
.toEqual(`@Output() modelChange: EventEmitter<string> = new EventEmitter();`);
});
});
@ -245,12 +242,11 @@ describe('definitions', () => {
describe('references', () => {
it('should work for element reference declarations', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `<div #cha¦rt></div>{{chart}}`,
expectedSpanText: 'chart',
});
const {position} =
service.overwriteInlineTemplate(APP_COMPONENT, `<div #cha¦rt></div>{{chart}}`);
const definitionAndBoundSpan = ngLS.getDefinitionAndBoundSpan(APP_COMPONENT, position);
// We're already at the definition, so nothing is returned
expect(definitions).toEqual([]);
expect(definitionAndBoundSpan).toBeUndefined();
});
it('should work for element reference uses', () => {