From ddc9e8e47a38ea63eeac3a1eff65f6b3c3f0032d Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 28 Sep 2020 11:25:20 -0700 Subject: [PATCH] refactor(compiler): refactor template symbol builder (#39047) * Add `templateNode` to `ElementSymbol` and `TemplateSymbol` so callers can use the information about the attributes on the `TmplAstElement`/`TmplAstTemplate` for directive matching * Remove helper function `getSymbolOfVariableDeclaration` and favor more specific handling for scenarios. The generic function did not easily handle different scenarios for all types of variable declarations in the TCB PR Close #39047 --- .../src/ngtsc/typecheck/api/symbols.ts | 4 ++ .../typecheck/src/template_symbol_builder.ts | 52 +++++++------------ ...ecker__get_symbol_of_template_node_spec.ts | 36 +++++++++++++ 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts index fd4fb74749..b74d6520d3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts @@ -204,6 +204,8 @@ export interface ElementSymbol { /** The location in the shim file for the variable that holds the type of the element. */ shimLocation: ShimLocation; + + templateNode: TmplAstElement; } export interface TemplateSymbol { @@ -211,6 +213,8 @@ export interface TemplateSymbol { /** A list of directives applied to the element. */ directives: DirectiveSymbol[]; + + templateNode: TmplAstTemplate; } /** diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index 6ce9b6ee8e..d9bb5429d9 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -54,7 +54,7 @@ export class SymbolBuilder { private getSymbolOfAstTemplate(template: TmplAstTemplate): TemplateSymbol|null { const directives = this.getDirectivesOfNode(template); - return {kind: SymbolKind.Template, directives}; + return {kind: SymbolKind.Template, directives, templateNode: template}; } private getSymbolOfElement(element: TmplAstElement): ElementSymbol|null { @@ -66,7 +66,7 @@ export class SymbolBuilder { return null; } - const symbolFromDeclaration = this.getSymbolOfVariableDeclaration(node); + const symbolFromDeclaration = this.getSymbolOfTsNode(node); if (symbolFromDeclaration === null || symbolFromDeclaration.tsSymbol === null) { return null; } @@ -79,6 +79,7 @@ export class SymbolBuilder { ...symbolFromDeclaration, kind: SymbolKind.Element, directives, + templateNode: element, }; } @@ -89,21 +90,18 @@ export class SymbolBuilder { // - var _t1: TestDir /*T:D*/ = (null!); // - var _t1 /*T:D*/ = _ctor1({}); const isDirectiveDeclaration = (node: ts.Node): node is ts.TypeNode|ts.Identifier => - (ts.isTypeNode(node) || ts.isIdentifier(node)) && + (ts.isTypeNode(node) || ts.isIdentifier(node)) && ts.isVariableDeclaration(node.parent) && hasExpressionIdentifier(tcbSourceFile, node, ExpressionIdentifier.DIRECTIVE); const nodes = findAllMatchingNodes( this.typeCheckBlock, {withSpan: elementSourceSpan, filter: isDirectiveDeclaration}); return nodes .map(node => { - const symbol = (ts.isIdentifier(node) && ts.isVariableDeclaration(node.parent)) ? - this.getSymbolOfVariableDeclaration(node.parent) : - this.getSymbolOfTsNode(node); + const symbol = this.getSymbolOfTsNode(node.parent); if (symbol === null || symbol.tsSymbol === null || symbol.tsSymbol.declarations.length === 0) { return null; } - const meta = this.getDirectiveMeta(element, symbol.tsSymbol.declarations[0]); if (meta === null) { return null; @@ -240,7 +238,7 @@ export class SymbolBuilder { return null; } - const symbol = this.getSymbolOfVariableDeclaration(declaration); + const symbol = this.getSymbolOfTsNode(declaration); if (symbol === null || symbol.tsSymbol === null) { return null; } @@ -258,11 +256,11 @@ export class SymbolBuilder { private getSymbolOfVariable(variable: TmplAstVariable): VariableSymbol|null { const node = findFirstMatchingNode( this.typeCheckBlock, {withSpan: variable.sourceSpan, filter: ts.isVariableDeclaration}); - if (node === null) { + if (node === null || node.initializer === undefined) { return null; } - const expressionSymbol = this.getSymbolOfVariableDeclaration(node); + const expressionSymbol = this.getSymbolOfTsNode(node.initializer); if (expressionSymbol === null) { return null; } @@ -279,8 +277,18 @@ export class SymbolBuilder { return null; } - // TODO(atscott): Shim location will need to be adjusted - const symbol = this.getSymbolOfTsNode(node.name); + // Get the original declaration for the references variable, with the exception of template refs + // which are of the form var _t3 = (_t2 as any as i2.TemplateRef) + // TODO(atscott): Consider adding an `ExpressionIdentifier` to tag variable declaration + // initializers as invalid for symbol retrieval. + const originalDeclaration = ts.isParenthesizedExpression(node.initializer) && + ts.isAsExpression(node.initializer.expression) ? + this.typeChecker.getSymbolAtLocation(node.name) : + this.typeChecker.getSymbolAtLocation(node.initializer); + if (originalDeclaration === undefined || originalDeclaration.valueDeclaration === undefined) { + return null; + } + const symbol = this.getSymbolOfTsNode(originalDeclaration.valueDeclaration); if (symbol === null || symbol.tsSymbol === null) { return null; } @@ -393,26 +401,6 @@ export class SymbolBuilder { }; } - private getSymbolOfVariableDeclaration(declaration: ts.VariableDeclaration): TsNodeSymbolInfo - |null { - // Instead of returning the Symbol for the temporary variable, we want to get the `ts.Symbol` - // for: - // - The type reference for `var _t2: MyDir = xyz` (prioritize/trust the declared type) - // - The initializer for `var _t2 = _t1.index`. - if (declaration.type && ts.isTypeReferenceNode(declaration.type)) { - return this.getSymbolOfTsNode(declaration.type.typeName); - } - if (declaration.initializer === undefined) { - return null; - } - - const symbol = this.getSymbolOfTsNode(declaration.initializer); - if (symbol === null) { - return null; - } - return symbol; - } - private getShimPositionForNode(node: ts.Node): number { if (ts.isTypeReferenceNode(node)) { return this.getShimPositionForNode(node.typeName); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index d1b5d3fb7f..5c7eb20e4f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -133,6 +133,7 @@ runInEachFileSystem(() => { it('should get a symbol for local ref which refers to a directive', () => { const symbol = templateTypeChecker.getSymbolOfNode(templateNode.references[1], cmp)!; assertReferenceSymbol(symbol); + expect(program.getTypeChecker().symbolToString(symbol.tsSymbol)).toEqual('TestDir'); assertDirectiveReference(symbol); }); @@ -140,6 +141,7 @@ runInEachFileSystem(() => { const symbol = templateTypeChecker.getSymbolOfNode( (templateNode.children[0] as TmplAstTemplate).inputs[2].value, cmp)!; assertReferenceSymbol(symbol); + expect(program.getTypeChecker().symbolToString(symbol.tsSymbol)).toEqual('TestDir'); assertDirectiveReference(symbol); }); @@ -737,6 +739,40 @@ runInEachFileSystem(() => { }); describe('input bindings', () => { + it('can get a symbol for empty binding', () => { + const fileName = absoluteFrom('/main.ts'); + const dirFile = absoluteFrom('/dir.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: {'Cmp': `
`}, + declarations: [{ + name: 'TestDir', + selector: '[dir]', + file: dirFile, + type: 'directive', + inputs: {inputA: 'inputA'}, + }] + }, + { + fileName: dirFile, + source: `export class TestDir {inputA?: string; }`, + templates: {}, + } + ]); + const sf = getSourceFileOrError(program, fileName); + const cmp = getClass(sf, 'Cmp'); + + const nodes = templateTypeChecker.getTemplate(cmp)!; + + const inputAbinding = (nodes[0] as TmplAstElement).inputs[0]; + const aSymbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp)!; + assertInputBindingSymbol(aSymbol); + expect((aSymbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration) + .name.getText()) + .toEqual('inputA'); + }); + it('can retrieve a symbol for an input binding', () => { const fileName = absoluteFrom('/main.ts'); const dirFile = absoluteFrom('/dir.ts');