diff --git a/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts b/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts index 135be11753..36c7a34600 100644 --- a/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts +++ b/packages/compiler-cli/src/diagnostics/expression_diagnostics.ts @@ -90,52 +90,77 @@ function getDefinitionOf(info: DiagnosticTemplateInfo, ast: TemplateAst): Defini } } -function getVarDeclarations( - info: DiagnosticTemplateInfo, path: TemplateAstPath): SymbolDeclaration[] { - const result: SymbolDeclaration[] = []; - - let current = path.tail; - while (current) { - if (current instanceof EmbeddedTemplateAst) { - for (const variable of current.variables) { - const name = variable.name; - - // Find the first directive with a context. - const context = - current.directives.map(d => info.query.getTemplateContext(d.directive.type.reference)) - .find(c => !!c); - - // Determine the type of the context field referenced by variable.value. - let type: Symbol|undefined = undefined; - if (context) { - const value = context.get(variable.value); - if (value) { - type = value.type !; - let kind = info.query.getTypeKind(type); - if (kind === BuiltinType.Any || kind == BuiltinType.Unbound) { - // The any type is not very useful here. For special cases, such as ngFor, we can do - // better. - type = refinedVariableType(type, info, current); - } - } - } - if (!type) { - type = info.query.getBuiltinType(BuiltinType.Any); - } - result.push({ - name, - kind: 'variable', type, get definition() { return getDefinitionOf(info, variable); } - }); - } +/** + * Resolve the specified `variable` from the `directives` list and return the + * corresponding symbol. If resolution fails, return the `any` type. + * @param variable template variable to resolve + * @param directives template context + * @param query + */ +function findSymbolForVariableInDirectives( + variable: VariableAst, directives: DirectiveAst[], query: SymbolQuery): Symbol { + for (const d of directives) { + // Get the symbol table for the directive's StaticSymbol + const table = query.getTemplateContext(d.directive.type.reference); + if (!table) { + continue; + } + const symbol = table.get(variable.value); + if (symbol) { + return symbol; } - current = path.parentOf(current); } - - return result; + return query.getBuiltinType(BuiltinType.Any); } +/** + * Resolve all variable declarations in a template by traversing the specified + * `path`. + * @param info + * @param path template AST path + */ +function getVarDeclarations( + info: DiagnosticTemplateInfo, path: TemplateAstPath): SymbolDeclaration[] { + const results: SymbolDeclaration[] = []; + for (let current = path.head; current; current = path.childOf(current)) { + if (!(current instanceof EmbeddedTemplateAst)) { + continue; + } + const {directives, variables} = current; + for (const variable of variables) { + let symbol = findSymbolForVariableInDirectives(variable, directives, info.query); + 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. + // We can do better by resolving the binding value. + const symbolsInScope = info.query.mergeSymbolTable([ + info.members, + // Since we are traversing the AST path from head to tail, any variables + // that have been declared so far are also in scope. + info.query.createSymbolTable(results), + ]); + symbol = refinedVariableType(symbolsInScope, info.query, current); + } + results.push({ + name: variable.name, + kind: 'variable', + type: symbol, get definition() { return getDefinitionOf(info, variable); }, + }); + } + } + return results; +} + +/** + * Resolve a more specific type for the variable in `templateElement` by inspecting + * all variables that are in scope in the `mergedTable`. This function is a special + * case for `ngFor` and `ngIf`. If resolution fails, return the `any` type. + * @param mergedTable symbol table for all variables in scope + * @param query + * @param templateElement + */ function refinedVariableType( - type: Symbol, info: DiagnosticTemplateInfo, templateElement: EmbeddedTemplateAst): Symbol { + mergedTable: SymbolTable, query: SymbolQuery, templateElement: EmbeddedTemplateAst): Symbol { // Special case the ngFor directive const ngForDirective = templateElement.directives.find(d => { const name = identifierName(d.directive.type); @@ -144,9 +169,9 @@ function refinedVariableType( if (ngForDirective) { const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf'); if (ngForOfBinding) { - const bindingType = new AstType(info.members, info.query, {}).getType(ngForOfBinding.value); + const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value); if (bindingType) { - const result = info.query.getElementType(bindingType); + const result = query.getElementType(bindingType); if (result) { return result; } @@ -160,7 +185,7 @@ function refinedVariableType( if (ngIfDirective) { const ngIfBinding = ngIfDirective.inputs.find(i => i.directiveName === 'ngIf'); if (ngIfBinding) { - const bindingType = new AstType(info.members, info.query, {}).getType(ngIfBinding.value); + const bindingType = new AstType(mergedTable, query, {}).getType(ngIfBinding.value); if (bindingType) { return bindingType; } @@ -168,7 +193,7 @@ function refinedVariableType( } // We can't do better, return any - return info.query.getBuiltinType(BuiltinType.Any); + return query.getBuiltinType(BuiltinType.Any); } function getEventDeclaration(info: DiagnosticTemplateInfo, includeEvent?: boolean) { diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index b6ec2ac2a7..b8a2c3388b 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -283,6 +283,20 @@ describe('completions', () => { const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start); expectContain(completions, CompletionKind.PROPERTY, ['name', 'age', 'street']); }); + + it('should be able to resolve variable in nested loop', () => { + mockHost.override(TEST_TEMPLATE, ` +
+
+ {{member.~{position}}} +
+
+ `); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'position'); + const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); + // member variable of type Hero has properties 'id' and 'name'. + expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']); + }); }); describe('data binding', () => { diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index 4e309e6655..3b4f6d26a0 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -173,6 +173,23 @@ describe('diagnostics', () => { expect(diagnostics).toEqual([]); }); + it('should report diagnostic for invalid property in nested ngFor', () => { + const content = mockHost.override(TEST_TEMPLATE, ` +
+
+ {{member.xyz}} +
+
+ `); + const diagnostics = ngLS.getDiagnostics(TEST_TEMPLATE); + expect(diagnostics.length).toBe(1); + const {messageText, start, length} = diagnostics[0]; + expect(messageText) + .toBe(`Identifier 'xyz' is not defined. 'Hero' does not contain such a member`); + expect(start).toBe(content.indexOf('member.xyz')); + expect(length).toBe('member.xyz'.length); + }); + describe('with $event', () => { it('should accept an event', () => { const fileName = '/app/test.ng'; diff --git a/packages/language-service/test/project/app/parsing-cases.ts b/packages/language-service/test/project/app/parsing-cases.ts index 8418fea484..1fe3aa7ab6 100644 --- a/packages/language-service/test/project/app/parsing-cases.ts +++ b/packages/language-service/test/project/app/parsing-cases.ts @@ -189,6 +189,7 @@ export class TemplateReference { title = 'Some title'; hero: Hero = {id: 1, name: 'Windstorm'}; heroes: Hero[] = [this.hero]; + league: Hero[][] = [this.heroes]; anyValue: any; myClick(event: any) {} }