refactor(compiler-cli): Return addEventListener symbol for native output bindings (#39312)

Rather than returning `null`, we can provide some useful information to the Language Service
by returning a symbol for the `addEventListener` function call when the consumer
of a binding as an element.

PR Close #39312
This commit is contained in:
Andrew Scott 2020-10-16 17:17:15 -07:00 committed by Misko Hevery
parent 634c393d35
commit 2f8a42036a
5 changed files with 148 additions and 88 deletions

View File

@ -176,41 +176,67 @@ export class SymbolBuilder {
} }
const consumer = this.templateData.boundTarget.getConsumerOfBinding(eventBinding); const consumer = this.templateData.boundTarget.getConsumerOfBinding(eventBinding);
if (consumer === null || consumer instanceof TmplAstTemplate || if (consumer === null) {
consumer instanceof TmplAstElement) {
// Bindings to element or template events produce `addEventListener` which
// we cannot get the field for.
return null;
}
const outputFieldAccess = TcbDirectiveOutputsOp.decodeOutputCallExpression(node);
if (outputFieldAccess === null) {
return null; return null;
} }
const tsSymbol = if (consumer instanceof TmplAstTemplate || consumer instanceof TmplAstElement) {
this.getTypeChecker().getSymbolAtLocation(outputFieldAccess.argumentExpression); if (!ts.isPropertyAccessExpression(node.expression) ||
if (tsSymbol === undefined) { node.expression.name.text !== 'addEventListener') {
return null; return null;
}
const addEventListener = node.expression.name;
const tsSymbol = this.getTypeChecker().getSymbolAtLocation(addEventListener);
const tsType = this.getTypeChecker().getTypeAtLocation(addEventListener);
const positionInShimFile = this.getShimPositionForNode(addEventListener);
const target = this.getSymbol(consumer);
if (target === null || tsSymbol === undefined) {
return null;
}
return {
kind: SymbolKind.Output,
bindings: [{
kind: SymbolKind.Binding,
tsSymbol,
tsType,
target,
shimLocation: {shimPath: this.shimPath, positionInShimFile},
}],
};
} else {
const outputFieldAccess = TcbDirectiveOutputsOp.decodeOutputCallExpression(node);
if (outputFieldAccess === null) {
return null;
}
const tsSymbol =
this.getTypeChecker().getSymbolAtLocation(outputFieldAccess.argumentExpression);
if (tsSymbol === undefined) {
return null;
}
const target = this.getDirectiveSymbolForAccessExpression(outputFieldAccess, consumer);
if (target === null) {
return null;
}
const positionInShimFile = this.getShimPositionForNode(outputFieldAccess);
const tsType = this.getTypeChecker().getTypeAtLocation(node);
return {
kind: SymbolKind.Output,
bindings: [{
kind: SymbolKind.Binding,
tsSymbol,
tsType,
target,
shimLocation: {shimPath: this.shimPath, positionInShimFile},
}],
};
} }
const target = this.getDirectiveSymbolForAccessExpression(outputFieldAccess, consumer);
if (target === null) {
return null;
}
const positionInShimFile = this.getShimPositionForNode(outputFieldAccess);
const tsType = this.getTypeChecker().getTypeAtLocation(node);
return {
kind: SymbolKind.Output,
bindings: [{
kind: SymbolKind.Binding,
tsSymbol,
tsType,
target,
shimLocation: {shimPath: this.shimPath, positionInShimFile},
}],
};
} }
private getSymbolOfInputBinding(binding: TmplAstBoundAttribute| private getSymbolOfInputBinding(binding: TmplAstBoundAttribute|

View File

@ -1247,38 +1247,29 @@ runInEachFileSystem(() => {
.toEqual('TestDir'); .toEqual('TestDir');
}); });
it('returns empty list when binding does not match any directive output', () => { it('returns addEventListener binding to native element when no match to any directive output',
const fileName = absoluteFrom('/main.ts'); () => {
const dirFile = absoluteFrom('/dir.ts'); const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([ const {program, templateTypeChecker} = setup([
{ {
fileName, fileName,
templates: {'Cmp': `<div dir (doesNotExist)="handle($event)"></div>`}, templates: {'Cmp': `<div (click)="handle($event)"></div>`},
declarations: [ },
{ ]);
name: 'TestDir', const sf = getSourceFileOrError(program, fileName);
selector: '[dir]', const cmp = getClass(sf, 'Cmp');
file: dirFile,
type: 'directive',
outputs: {outputA: 'outputA'},
},
]
},
{
fileName: dirFile,
source: `export class TestDir {outputA!: EventEmitter<string>;}`,
templates: {},
}
]);
const sf = getSourceFileOrError(program, fileName);
const cmp = getClass(sf, 'Cmp');
const nodes = templateTypeChecker.getTemplate(cmp)!; const nodes = templateTypeChecker.getTemplate(cmp)!;
const outputABinding = (nodes[0] as TmplAstElement).outputs[0]; const outputABinding = (nodes[0] as TmplAstElement).outputs[0];
const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp); const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp)!;
expect(symbol).toBeNull(); assertOutputBindingSymbol(symbol);
}); expect(program.getTypeChecker().symbolToString(symbol.bindings[0].tsSymbol!))
.toEqual('addEventListener');
const eventSymbol = templateTypeChecker.getSymbolOfNode(outputABinding.handler, cmp)!;
assertExpressionSymbol(eventSymbol);
});
it('returns empty list when checkTypeOfOutputEvents is false', () => { it('returns empty list when checkTypeOfOutputEvents is false', () => {
const fileName = absoluteFrom('/main.ts'); const fileName = absoluteFrom('/main.ts');

View File

@ -228,6 +228,18 @@ describe('definitions', () => {
expect(directiveDef.textSpan).toEqual('EventSelectorDirective'); expect(directiveDef.textSpan).toEqual('EventSelectorDirective');
expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective'); expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective');
}); });
it('should work for $event from native element', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `<div (cl¦ick)="myClick($event)"></div>`,
expectedSpanText: 'click',
});
expect(definitions!.length).toEqual(1);
expect(definitions[0].textSpan).toEqual('addEventListener');
expect(definitions[0].contextSpan)
.toContain('addEventListener<K extends keyof HTMLElementEventMap>');
expect(definitions[0].fileName).toContain('lib.dom.d.ts');
});
}); });
}); });

View File

@ -32,7 +32,7 @@ describe('type definitions', () => {
describe('elements', () => { describe('elements', () => {
it('should work for native elements', () => { it('should work for native elements', () => {
const defs = getTypeDefinitionsAndAssertBoundSpan({ const defs = getTypeDefinitions({
templateOverride: `<butt¦on></button>`, templateOverride: `<butt¦on></button>`,
}); });
expect(defs.length).toEqual(2); expect(defs.length).toEqual(2);
@ -42,7 +42,7 @@ describe('type definitions', () => {
}); });
it('should return directives which match the element tag', () => { it('should return directives which match the element tag', () => {
const defs = getTypeDefinitionsAndAssertBoundSpan({ const defs = getTypeDefinitions({
templateOverride: `<butt¦on compound custom-button></button>`, templateOverride: `<butt¦on compound custom-button></button>`,
}); });
expect(defs.length).toEqual(3); expect(defs.length).toEqual(3);
@ -63,7 +63,7 @@ describe('type definitions', () => {
describe('directives', () => { describe('directives', () => {
it('should work for directives', () => { it('should work for directives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div string-model¦></div>`, templateOverride: `<div string-model¦></div>`,
}); });
expect(definitions.length).toEqual(1); expect(definitions.length).toEqual(1);
@ -73,7 +73,7 @@ describe('type definitions', () => {
}); });
it('should work for components', () => { it('should work for components', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<t¦est-comp></test-comp>`, templateOverride: `<t¦est-comp></test-comp>`,
}); });
expect(definitions.length).toEqual(1); expect(definitions.length).toEqual(1);
@ -82,7 +82,7 @@ describe('type definitions', () => {
}); });
it('should work for structural directives', () => { it('should work for structural directives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *¦ngFor="let item of heroes"></div>`, templateOverride: `<div *¦ngFor="let item of heroes"></div>`,
}); });
expect(definitions.length).toEqual(1); expect(definitions.length).toEqual(1);
@ -94,12 +94,12 @@ describe('type definitions', () => {
}); });
it('should work for directives with compound selectors', () => { it('should work for directives with compound selectors', () => {
let defs = getTypeDefinitionsAndAssertBoundSpan({ let defs = getTypeDefinitions({
templateOverride: `<button com¦pound custom-button></button>`, templateOverride: `<button com¦pound custom-button></button>`,
}); });
expect(defs.length).toEqual(1); expect(defs.length).toEqual(1);
expect(defs[0].contextSpan).toContain('export class CompoundCustomButtonDirective'); expect(defs[0].contextSpan).toContain('export class CompoundCustomButtonDirective');
defs = getTypeDefinitionsAndAssertBoundSpan({ defs = getTypeDefinitions({
templateOverride: `<button compound cu¦stom-button></button>`, templateOverride: `<button compound cu¦stom-button></button>`,
}); });
expect(defs.length).toEqual(1); expect(defs.length).toEqual(1);
@ -110,7 +110,7 @@ describe('type definitions', () => {
describe('bindings', () => { describe('bindings', () => {
describe('inputs', () => { describe('inputs', () => {
it('should return something for input providers with non-primitive types', () => { it('should return something for input providers with non-primitive types', () => {
const defs = getTypeDefinitionsAndAssertBoundSpan({ const defs = getTypeDefinitions({
templateOverride: `<button compound custom-button [config¦]="{}"></button>`, templateOverride: `<button compound custom-button [config¦]="{}"></button>`,
}); });
expect(defs.length).toEqual(1); expect(defs.length).toEqual(1);
@ -118,7 +118,7 @@ describe('type definitions', () => {
}); });
it('should work for structural directive inputs ngForTrackBy', () => { it('should work for structural directive inputs ngForTrackBy', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let item of heroes; tr¦ackBy: test;"></div>`, templateOverride: `<div *ngFor="let item of heroes; tr¦ackBy: test;"></div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -129,7 +129,7 @@ describe('type definitions', () => {
}); });
it('should work for structural directive inputs ngForOf', () => { it('should work for structural directive inputs ngForOf', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let item o¦f heroes"></div>`, templateOverride: `<div *ngFor="let item o¦f heroes"></div>`,
}); });
// In addition to all the array defs, this will also return the NgForOf def because the // In addition to all the array defs, this will also return the NgForOf def because the
@ -140,7 +140,7 @@ describe('type definitions', () => {
}); });
it('should return nothing for two-way binding providers', () => { it('should return nothing for two-way binding providers', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<test-comp string-model [(mo¦del)]="title"></test-comp>`, templateOverride: `<test-comp string-model [(mo¦del)]="title"></test-comp>`,
}); });
// TODO(atscott): This should actually return EventEmitter type but we only match the input // TODO(atscott): This should actually return EventEmitter type but we only match the input
@ -151,7 +151,7 @@ describe('type definitions', () => {
describe('outputs', () => { describe('outputs', () => {
it('should work for event providers', () => { it('should work for event providers', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<test-comp (te¦st)="myClick($event)"></test-comp>`, templateOverride: `<test-comp (te¦st)="myClick($event)"></test-comp>`,
}); });
expect(definitions!.length).toEqual(2); expect(definitions!.length).toEqual(2);
@ -167,7 +167,7 @@ describe('type definitions', () => {
}); });
it('should return the directive when the event is part of the selector', () => { it('should return the directive when the event is part of the selector', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div (eventSelect¦or)="title = ''"></div>`, templateOverride: `<div (eventSelect¦or)="title = ''"></div>`,
}); });
expect(definitions!.length).toEqual(3); expect(definitions!.length).toEqual(3);
@ -176,12 +176,24 @@ describe('type definitions', () => {
const directiveDef = definitions[2]; const directiveDef = definitions[2];
expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective'); expect(directiveDef.contextSpan).toContain('export class EventSelectorDirective');
}); });
it('should work for native event outputs', () => {
const definitions = getTypeDefinitions({
templateOverride: `<div (cl¦ick)="myClick($event)"></div>`,
});
expect(definitions!.length).toEqual(1);
expect(definitions[0].textSpan).toEqual('addEventListener');
expect(definitions[0].contextSpan)
.toEqual(
'addEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions): void;');
expect(definitions[0].fileName).toContain('lib.dom.d.ts');
});
}); });
}); });
describe('references', () => { describe('references', () => {
it('should work for element references', () => { it('should work for element references', () => {
const defs = getTypeDefinitionsAndAssertBoundSpan({ const defs = getTypeDefinitions({
templateOverride: `<div #chart></div>{{char¦t}}`, templateOverride: `<div #chart></div>{{char¦t}}`,
}); });
expect(defs.length).toEqual(2); expect(defs.length).toEqual(2);
@ -190,7 +202,7 @@ describe('type definitions', () => {
}); });
it('should work for directive references', () => { it('should work for directive references', () => {
const defs = getTypeDefinitionsAndAssertBoundSpan({ const defs = getTypeDefinitions({
templateOverride: `<div #mod¦el="stringModel" string-model></div>`, templateOverride: `<div #mod¦el="stringModel" string-model></div>`,
}); });
expect(defs.length).toEqual(1); expect(defs.length).toEqual(1);
@ -201,7 +213,7 @@ describe('type definitions', () => {
describe('variables', () => { describe('variables', () => {
it('should work for array members', () => { it('should work for array members', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let hero of heroes">{{her¦o}}</div>`, templateOverride: `<div *ngFor="let hero of heroes">{{her¦o}}</div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -214,7 +226,7 @@ describe('type definitions', () => {
describe('pipes', () => { describe('pipes', () => {
it('should work for pipes', () => { it('should work for pipes', () => {
const templateOverride = `<p>The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}</p>`; const templateOverride = `<p>The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}</p>`;
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride, templateOverride,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -227,7 +239,7 @@ describe('type definitions', () => {
describe('expressions', () => { describe('expressions', () => {
it('should return nothing for primitives', () => { it('should return nothing for primitives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div>{{ tit¦le }}</div>`, templateOverride: `<div>{{ tit¦le }}</div>`,
}); });
expect(definitions!.length).toEqual(0); expect(definitions!.length).toEqual(0);
@ -236,7 +248,7 @@ describe('type definitions', () => {
// TODO(atscott): Investigate why this returns nothing in the test environment. This actually // TODO(atscott): Investigate why this returns nothing in the test environment. This actually
// works in the extension. // works in the extension.
xit('should work for functions on primitives', () => { xit('should work for functions on primitives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div>{{ title.toLower¦case() }}</div>`, templateOverride: `<div>{{ title.toLower¦case() }}</div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -245,7 +257,7 @@ describe('type definitions', () => {
}); });
it('should work for accessed property reads', () => { it('should work for accessed property reads', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div>{{heroes[0].addre¦ss}}</div>`, templateOverride: `<div>{{heroes[0].addre¦ss}}</div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -256,7 +268,7 @@ describe('type definitions', () => {
}); });
it('should work for $event', () => { it('should work for $event', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<button (click)="title=$ev¦ent"></button>`, templateOverride: `<button (click)="title=$ev¦ent"></button>`,
}); });
expect(definitions!.length).toEqual(2); expect(definitions!.length).toEqual(2);
@ -269,7 +281,7 @@ describe('type definitions', () => {
}); });
it('should work for method calls', () => { it('should work for method calls', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div (click)="setT¦itle('title')"></div>`, templateOverride: `<div (click)="setT¦itle('title')"></div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -280,7 +292,7 @@ describe('type definitions', () => {
}); });
it('should work for accessed properties in writes', () => { it('should work for accessed properties in writes', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div (click)="hero.add¦ress = undefined"></div>`, templateOverride: `<div (click)="hero.add¦ress = undefined"></div>`,
}); });
expect(definitions!.length).toEqual(1); expect(definitions!.length).toEqual(1);
@ -291,21 +303,21 @@ describe('type definitions', () => {
}); });
it('should work for variables in structural directives', () => { it('should work for variables in structural directives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let item of heroes as her¦oes2; trackBy: test;"></div>`, templateOverride: `<div *ngFor="let item of heroes as her¦oes2; trackBy: test;"></div>`,
}); });
expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles); expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
}); });
it('should work for uses of members in structural directives', () => { it('should work for uses of members in structural directives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let item of heroes as heroes2">{{her¦oes2}}</div>`, templateOverride: `<div *ngFor="let item of heroes as heroes2">{{her¦oes2}}</div>`,
}); });
expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles); expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
}); });
it('should work for members in structural directives', () => { it('should work for members in structural directives', () => {
const definitions = getTypeDefinitionsAndAssertBoundSpan({ const definitions = getTypeDefinitions({
templateOverride: `<div *ngFor="let item of her¦oes; trackBy: test;"></div>`, templateOverride: `<div *ngFor="let item of her¦oes; trackBy: test;"></div>`,
}); });
expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles); expectAllDefinitions(definitions, new Set(['Array']), possibleArrayDefFiles);
@ -319,7 +331,7 @@ describe('type definitions', () => {
}); });
}); });
function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}): function getTypeDefinitions({templateOverride}: {templateOverride: string}):
HumanizedDefinitionInfo[] { HumanizedDefinitionInfo[] {
const {position} = service.overwriteInlineTemplate(APP_COMPONENT, templateOverride); const {position} = service.overwriteInlineTemplate(APP_COMPONENT, templateOverride);
const defs = ngLS.getTypeDefinitionAtPosition(APP_COMPONENT, position); const defs = ngLS.getTypeDefinitionAtPosition(APP_COMPONENT, position);

View File

@ -309,6 +309,25 @@ describe('quick info', () => {
expectedDisplayString: '(reference) chart: HTMLDivElement' expectedDisplayString: '(reference) chart: HTMLDivElement'
}); });
}); });
it('should work for $event from native element', () => {
expectQuickInfo({
templateOverride: `<div (click)="myClick($e¦vent)"></div>`,
expectedSpanText: '$event',
expectedDisplayString: '(parameter) $event: MouseEvent'
});
});
it('should work for click output from native element', () => {
expectQuickInfo({
templateOverride: `<div (cl¦ick)="myClick($event)"></div>`,
expectedSpanText: 'click',
expectedDisplayString:
'(event) HTMLDivElement.addEventListener<"click">(type: "click", ' +
'listener: (this: HTMLDivElement, ev: MouseEvent) => any, ' +
'options?: boolean | AddEventListenerOptions | undefined): void (+1 overload)'
});
});
}); });
describe('variables', () => { describe('variables', () => {