From 54fd33fb80566ae6a0a8de4cc9a08fd1dafe7d78 Mon Sep 17 00:00:00 2001 From: Pusztai Tibor Date: Wed, 19 Feb 2020 20:38:21 +0100 Subject: [PATCH] fix(language-service): infer context type of structural directives (#35537) (#35561) PR Close #35561 --- .../src/expression_diagnostics.ts | 87 ++++++++++--------- .../language-service/test/diagnostics_spec.ts | 36 ++++++++ .../language-service/test/project/app/main.ts | 1 + .../test/project/app/parsing-cases.ts | 15 ++++ 4 files changed, 98 insertions(+), 41 deletions(-) diff --git a/packages/language-service/src/expression_diagnostics.ts b/packages/language-service/src/expression_diagnostics.ts index 72fec3f9c4..373d6068ac 100644 --- a/packages/language-service/src/expression_diagnostics.ts +++ b/packages/language-service/src/expression_diagnostics.ts @@ -91,7 +91,10 @@ function getVarDeclarations( continue; } for (const variable of current.variables) { - let symbol = info.members.get(variable.value) || info.query.getBuiltinType(BuiltinType.Any); + let symbol = info.members.get(variable.value); + if (!symbol) { + symbol = getVariableTypeFromDirectiveContext(variable.value, info.query, current); + } const kind = info.query.getTypeKind(symbol); if (kind === BuiltinType.Any || kind === BuiltinType.Unbound) { // For special cases such as ngFor and ngIf, the any type is not very useful. @@ -115,22 +118,24 @@ function getVarDeclarations( } /** - * Gets the type of an ngFor exported value, as enumerated in - * https://angular.io/api/common/NgForOfContext - * @param value exported value name + * Resolve the type for the variable in `templateElement` by finding the structural + * directive which has the context member. Returns any when not found. + * @param value variable value name * @param query type symbol query + * @param templateElement */ -function getNgForExportedValueType(value: string, query: SymbolQuery): Symbol|undefined { - switch (value) { - case 'index': - case 'count': - return query.getBuiltinType(BuiltinType.Number); - case 'first': - case 'last': - case 'even': - case 'odd': - return query.getBuiltinType(BuiltinType.Boolean); +function getVariableTypeFromDirectiveContext( + value: string, query: SymbolQuery, templateElement: EmbeddedTemplateAst): Symbol { + for (const {directive} of templateElement.directives) { + const context = query.getTemplateContext(directive.type.reference); + if (context) { + const member = context.get(value); + if (member && member.type) { + return member.type; + } + } } + return query.getBuiltinType(BuiltinType.Any); } /** @@ -145,38 +150,38 @@ function getNgForExportedValueType(value: string, query: SymbolQuery): Symbol|un function refinedVariableType( value: string, mergedTable: SymbolTable, query: SymbolQuery, templateElement: EmbeddedTemplateAst): Symbol { - // Special case the ngFor directive - const ngForDirective = templateElement.directives.find(d => { - const name = identifierName(d.directive.type); - return name == 'NgFor' || name == 'NgForOf'; - }); - if (ngForDirective) { - const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf'); - if (ngForOfBinding) { - // Check if the variable value is a type exported by the ngFor statement. - let result = getNgForExportedValueType(value, query); - - // Otherwise, check if there is a known type for the ngFor binding. - const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value); - if (!result && bindingType) { - result = query.getElementType(bindingType); - } - - if (result) { - return result; + if (value === '$implicit') { + // Special case the ngFor directive + const ngForDirective = templateElement.directives.find(d => { + const name = identifierName(d.directive.type); + return name == 'NgFor' || name == 'NgForOf'; + }); + if (ngForDirective) { + const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf'); + if (ngForOfBinding) { + // Check if there is a known type for the ngFor binding. + const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value); + if (bindingType) { + const result = query.getElementType(bindingType); + if (result) { + return result; + } + } } } } // Special case the ngIf directive ( *ngIf="data$ | async as variable" ) - const ngIfDirective = - templateElement.directives.find(d => identifierName(d.directive.type) === 'NgIf'); - if (ngIfDirective) { - const ngIfBinding = ngIfDirective.inputs.find(i => i.directiveName === 'ngIf'); - if (ngIfBinding) { - const bindingType = new AstType(mergedTable, query, {}).getType(ngIfBinding.value); - if (bindingType) { - return bindingType; + if (value === 'ngIf') { + const ngIfDirective = + templateElement.directives.find(d => identifierName(d.directive.type) === 'NgIf'); + if (ngIfDirective) { + const ngIfBinding = ngIfDirective.inputs.find(i => i.directiveName === 'ngIf'); + if (ngIfBinding) { + const bindingType = new AstType(mergedTable, query, {}).getType(ngIfBinding.value); + if (bindingType) { + return bindingType; + } } } } diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index 9597b0dd62..2d2e63933e 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -286,6 +286,42 @@ describe('diagnostics', () => { expect(start).toBe(span.start); expect(length).toBe(span.length); }); + + it('report an unknown field in $implicit context', () => { + mockHost.override(TEST_TEMPLATE, ` +
+ {{ ~{start-emb}myVar.missingField ~{end-emb}}} +
+ `); + const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); + expect(diags.length).toBe(1); + const {messageText, start, length, category} = diags[0]; + expect(category).toBe(ts.DiagnosticCategory.Error); + expect(messageText) + .toBe( + `Identifier 'missingField' is not defined. '{ implicitPerson: Person; }' does not contain such a member`, ); + const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb'); + expect(start).toBe(span.start); + expect(length).toBe(span.length); + }); + + it('report an unknown field in non implicit context', () => { + mockHost.override(TEST_TEMPLATE, ` +
+ {{ ~{start-emb}myVar.missingField ~{end-emb}}} +
+ `); + const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); + expect(diags.length).toBe(1); + const {messageText, start, length, category} = diags[0]; + expect(category).toBe(ts.DiagnosticCategory.Error); + expect(messageText) + .toBe( + `Identifier 'missingField' is not defined. 'Person' does not contain such a member`, ); + const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb'); + expect(start).toBe(span.start); + expect(length).toBe(span.length); + }); }); // #17611 diff --git a/packages/language-service/test/project/app/main.ts b/packages/language-service/test/project/app/main.ts index 6de049e260..daa4ef3c0b 100644 --- a/packages/language-service/test/project/app/main.ts +++ b/packages/language-service/test/project/app/main.ts @@ -43,6 +43,7 @@ import * as ParsingCases from './parsing-cases'; ParsingCases.StringModel, ParsingCases.TemplateReference, ParsingCases.TestComponent, + ParsingCases.WithContextDirective, ] }) export class AppModule { diff --git a/packages/language-service/test/project/app/parsing-cases.ts b/packages/language-service/test/project/app/parsing-cases.ts index 7e62dbf6c1..988aa6c5a4 100644 --- a/packages/language-service/test/project/app/parsing-cases.ts +++ b/packages/language-service/test/project/app/parsing-cases.ts @@ -127,6 +127,21 @@ export class CounterDirective implements OnChanges { } } +interface WithContextDirectiveContext { + $implicit: {implicitPerson: Person;}; + nonImplicitPerson: Person; +} + +@Directive({selector: '[withContext]'}) +export class WithContextDirective { + constructor(_template: TemplateRef) {} + + static ngTemplateContextGuard(dir: WithContextDirective, ctx: unknown): + ctx is WithContextDirectiveContext { + return true; + } +} + /** * This Component provides the `test-comp` selector. */