From e7c74cbd69534a7eefd392bab0bf7f320a2316eb Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Thu, 26 Dec 2019 13:26:54 -0600 Subject: [PATCH] feat(language-service): completions for output $event properties in (#34570) This commit adds support for completions of properties on `$event` variables in bound outputs. This is the second major PR to support completions for `$event` variables (https://github.com/angular/vscode-ng-language-service/issues/531). The final completion support that must be provided is for `$event` variables in bindings targeting DOM events, like `(click)`. PR Close #34570 --- packages/language-service/src/completions.ts | 1 - .../src/expression_diagnostics.ts | 47 ++++++++++++++----- .../language-service/src/locate_symbol.ts | 2 +- packages/language-service/src/utils.ts | 6 +-- .../language-service/test/completions_spec.ts | 11 ++++- .../language-service/test/diagnostics_spec.ts | 21 ++++++--- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 97bca50faa..64abe85f5b 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -316,7 +316,6 @@ function attributeValueCompletions(info: AstResult, htmlPath: HtmlAstPath): ng.C templatePath.tail.visit(visitor, null); return visitor.results; } - // In order to provide accurate attribute value completion, we need to know // what the LHS is, and construct the proper AST if it is missing. const htmlAttr = htmlPath.tail as Attribute; diff --git a/packages/language-service/src/expression_diagnostics.ts b/packages/language-service/src/expression_diagnostics.ts index 84a6ff480f..1d5023d57e 100644 --- a/packages/language-service/src/expression_diagnostics.ts +++ b/packages/language-service/src/expression_diagnostics.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {AstType, ExpressionDiagnosticsContext, TypeDiagnostic} from './expression_type'; import {BuiltinType, Definition, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './symbols'; import {Diagnostic} from './types'; -import {getPathToNodeAtPosition} from './utils'; +import {findOutputBinding, getPathToNodeAtPosition} from './utils'; export interface DiagnosticTemplateInfo { fileName?: string; @@ -193,26 +193,51 @@ function refinedVariableType( return query.getBuiltinType(BuiltinType.Any); } -function getEventDeclaration(info: DiagnosticTemplateInfo, path: TemplateAstPath) { - let result: SymbolDeclaration[] = []; - if (path.tail instanceof BoundEventAst) { - // TODO: Determine the type of the event parameter based on the Observable or EventEmitter - // of the event. - result = [{name: '$event', kind: 'variable', type: info.query.getBuiltinType(BuiltinType.Any)}]; +function getEventDeclaration( + info: DiagnosticTemplateInfo, path: TemplateAstPath): SymbolDeclaration|undefined { + const event = path.tail; + if (!(event instanceof BoundEventAst)) { + // No event available in this context. + return; } - return result; + + const genericEvent: SymbolDeclaration = { + name: '$event', + kind: 'variable', + type: info.query.getBuiltinType(BuiltinType.Any), + }; + + const outputSymbol = findOutputBinding(event, path, info.query); + if (!outputSymbol) { + // The `$event` variable doesn't belong to an output, so its type can't be refined. + // TODO: type `$event` variables in bindings to DOM events. + return genericEvent; + } + + // The raw event type is wrapped in a generic, like EventEmitter or Observable. + const ta = outputSymbol.typeArguments(); + if (!ta || ta.length !== 1) return genericEvent; + const eventType = ta[0]; + + return {...genericEvent, type: eventType}; } +/** + * Returns the symbols available in a particular scope of a template. + * @param info parsed template information + * @param path path of template nodes narrowing to the context the expression scope should be + * derived for. + */ export function getExpressionScope( info: DiagnosticTemplateInfo, path: TemplateAstPath): SymbolTable { let result = info.members; const references = getReferences(info); const variables = getVarDeclarations(info, path); - const events = getEventDeclaration(info, path); - if (references.length || variables.length || events.length) { + const event = getEventDeclaration(info, path); + if (references.length || variables.length || event) { const referenceTable = info.query.createSymbolTable(references); const variableTable = info.query.createSymbolTable(variables); - const eventsTable = info.query.createSymbolTable(events); + const eventsTable = info.query.createSymbolTable(event ? [event] : []); result = info.query.mergeSymbolTable([result, referenceTable, variableTable, eventsTable]); } return result; diff --git a/packages/language-service/src/locate_symbol.ts b/packages/language-service/src/locate_symbol.ts index 1ff01cb19d..65591ea880 100644 --- a/packages/language-service/src/locate_symbol.ts +++ b/packages/language-service/src/locate_symbol.ts @@ -109,7 +109,7 @@ function locateSymbol(ast: TemplateAst, path: TemplateAstPath, info: AstResult): visitVariable(ast) {}, visitEvent(ast) { if (!attributeValueSymbol()) { - symbol = findOutputBinding(info, path, ast); + symbol = findOutputBinding(ast, path, info.template.query); symbol = symbol && new OverrideKindSymbol(symbol, DirectiveKind.EVENT); span = spanOf(ast); } diff --git a/packages/language-service/src/utils.ts b/packages/language-service/src/utils.ts index 6d89d70d56..4b8c062ee1 100644 --- a/packages/language-service/src/utils.ts +++ b/packages/language-service/src/utils.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {AstResult, SelectorInfo} from './common'; import {DiagnosticTemplateInfo} from './expression_diagnostics'; -import {Span, Symbol} from './types'; +import {Span, Symbol, SymbolQuery} from './types'; export interface SpanHolder { sourceSpan: ParseSourceSpan; @@ -268,14 +268,14 @@ export function invertMap(obj: {[name: string]: string}): {[name: string]: strin * @param path narrowing */ export function findOutputBinding( - info: AstResult, path: TemplateAstPath, binding: BoundEventAst): Symbol|undefined { + binding: BoundEventAst, path: TemplateAstPath, query: SymbolQuery): Symbol|undefined { const element = path.first(ElementAst); if (element) { for (const directive of element.directives) { const invertedOutputs = invertMap(directive.directive.outputs); const fieldName = invertedOutputs[binding.name]; if (fieldName) { - const classSymbol = info.template.query.getTypeSymbol(directive.directive.type.reference); + const classSymbol = query.getTypeSymbol(directive.directive.type.reference); if (classSymbol) { return classSymbol.members().get(fieldName); } diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 5d27e46c17..2d6967570c 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -758,10 +758,17 @@ describe('completions', () => { it('should suggest $event in event bindings', () => { mockHost.override(TEST_TEMPLATE, `
`); const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); - debugger; - const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); + const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); expectContain(completions, CompletionKind.VARIABLE, ['$event']); }); + + it('should suggest $event completions in output bindings', () => { + mockHost.override(TEST_TEMPLATE, `
`); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); + const completions = ngLS.getCompletionsAtPosition(TEST_TEMPLATE, marker.start); + // Expect string properties + expectContain(completions, CompletionKind.METHOD, ['charAt', 'substring']); + }); }); }); diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index bc75d329de..1340bd8c81 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -311,16 +311,14 @@ describe('diagnostics', () => { describe('with $event', () => { it('should accept an event', () => { - const fileName = '/app/test.ng'; - mockHost.override(fileName, '
Click me!
'); - const diagnostics = ngLS.getSemanticDiagnostics(fileName); + mockHost.override(TEST_TEMPLATE, '
Click me!
'); + const diagnostics = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); expect(diagnostics).toEqual([]); }); it('should reject it when not in an event binding', () => { - const fileName = '/app/test.ng'; - const content = mockHost.override(fileName, '
'); - const diagnostics = ngLS.getSemanticDiagnostics(fileName) !; + const content = mockHost.override(TEST_TEMPLATE, '
'); + const diagnostics = ngLS.getSemanticDiagnostics(TEST_TEMPLATE) !; expect(diagnostics.length).toBe(1); const {messageText, start, length} = diagnostics[0]; expect(messageText) @@ -330,6 +328,17 @@ describe('diagnostics', () => { expect(start).toBe(content.lastIndexOf(keyword)); expect(length).toBe(keyword.length); }); + + it('should reject invalid properties on an event type', () => { + const content = mockHost.override( + TEST_TEMPLATE, '
'); + const diagnostics = ngLS.getSemanticDiagnostics(TEST_TEMPLATE) !; + expect(diagnostics.length).toBe(1); + const {messageText, start, length} = diagnostics[0]; + expect(messageText).toBe(`Unknown method 'notSubstring'`); + expect(start).toBe(content.indexOf('$event')); + expect(length).toBe('$event.notSubstring()'.length); + }); }); it('should not crash with a incomplete *ngFor', () => {