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
This commit is contained in:
Andrew Scott 2020-09-28 11:25:20 -07:00 committed by Alex Rickabaugh
parent 1e3f810f8f
commit ddc9e8e47a
3 changed files with 60 additions and 32 deletions

View File

@ -204,6 +204,8 @@ export interface ElementSymbol {
/** The location in the shim file for the variable that holds the type of the element. */ /** The location in the shim file for the variable that holds the type of the element. */
shimLocation: ShimLocation; shimLocation: ShimLocation;
templateNode: TmplAstElement;
} }
export interface TemplateSymbol { export interface TemplateSymbol {
@ -211,6 +213,8 @@ export interface TemplateSymbol {
/** A list of directives applied to the element. */ /** A list of directives applied to the element. */
directives: DirectiveSymbol[]; directives: DirectiveSymbol[];
templateNode: TmplAstTemplate;
} }
/** /**

View File

@ -54,7 +54,7 @@ export class SymbolBuilder {
private getSymbolOfAstTemplate(template: TmplAstTemplate): TemplateSymbol|null { private getSymbolOfAstTemplate(template: TmplAstTemplate): TemplateSymbol|null {
const directives = this.getDirectivesOfNode(template); const directives = this.getDirectivesOfNode(template);
return {kind: SymbolKind.Template, directives}; return {kind: SymbolKind.Template, directives, templateNode: template};
} }
private getSymbolOfElement(element: TmplAstElement): ElementSymbol|null { private getSymbolOfElement(element: TmplAstElement): ElementSymbol|null {
@ -66,7 +66,7 @@ export class SymbolBuilder {
return null; return null;
} }
const symbolFromDeclaration = this.getSymbolOfVariableDeclaration(node); const symbolFromDeclaration = this.getSymbolOfTsNode(node);
if (symbolFromDeclaration === null || symbolFromDeclaration.tsSymbol === null) { if (symbolFromDeclaration === null || symbolFromDeclaration.tsSymbol === null) {
return null; return null;
} }
@ -79,6 +79,7 @@ export class SymbolBuilder {
...symbolFromDeclaration, ...symbolFromDeclaration,
kind: SymbolKind.Element, kind: SymbolKind.Element,
directives, directives,
templateNode: element,
}; };
} }
@ -89,21 +90,18 @@ export class SymbolBuilder {
// - var _t1: TestDir /*T:D*/ = (null!); // - var _t1: TestDir /*T:D*/ = (null!);
// - var _t1 /*T:D*/ = _ctor1({}); // - var _t1 /*T:D*/ = _ctor1({});
const isDirectiveDeclaration = (node: ts.Node): node is ts.TypeNode|ts.Identifier => 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); hasExpressionIdentifier(tcbSourceFile, node, ExpressionIdentifier.DIRECTIVE);
const nodes = findAllMatchingNodes( const nodes = findAllMatchingNodes(
this.typeCheckBlock, {withSpan: elementSourceSpan, filter: isDirectiveDeclaration}); this.typeCheckBlock, {withSpan: elementSourceSpan, filter: isDirectiveDeclaration});
return nodes return nodes
.map(node => { .map(node => {
const symbol = (ts.isIdentifier(node) && ts.isVariableDeclaration(node.parent)) ? const symbol = this.getSymbolOfTsNode(node.parent);
this.getSymbolOfVariableDeclaration(node.parent) :
this.getSymbolOfTsNode(node);
if (symbol === null || symbol.tsSymbol === null || if (symbol === null || symbol.tsSymbol === null ||
symbol.tsSymbol.declarations.length === 0) { symbol.tsSymbol.declarations.length === 0) {
return null; return null;
} }
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.declarations[0]); const meta = this.getDirectiveMeta(element, symbol.tsSymbol.declarations[0]);
if (meta === null) { if (meta === null) {
return null; return null;
@ -240,7 +238,7 @@ export class SymbolBuilder {
return null; return null;
} }
const symbol = this.getSymbolOfVariableDeclaration(declaration); const symbol = this.getSymbolOfTsNode(declaration);
if (symbol === null || symbol.tsSymbol === null) { if (symbol === null || symbol.tsSymbol === null) {
return null; return null;
} }
@ -258,11 +256,11 @@ export class SymbolBuilder {
private getSymbolOfVariable(variable: TmplAstVariable): VariableSymbol|null { private getSymbolOfVariable(variable: TmplAstVariable): VariableSymbol|null {
const node = findFirstMatchingNode( const node = findFirstMatchingNode(
this.typeCheckBlock, {withSpan: variable.sourceSpan, filter: ts.isVariableDeclaration}); this.typeCheckBlock, {withSpan: variable.sourceSpan, filter: ts.isVariableDeclaration});
if (node === null) { if (node === null || node.initializer === undefined) {
return null; return null;
} }
const expressionSymbol = this.getSymbolOfVariableDeclaration(node); const expressionSymbol = this.getSymbolOfTsNode(node.initializer);
if (expressionSymbol === null) { if (expressionSymbol === null) {
return null; return null;
} }
@ -279,8 +277,18 @@ export class SymbolBuilder {
return null; return null;
} }
// TODO(atscott): Shim location will need to be adjusted // Get the original declaration for the references variable, with the exception of template refs
const symbol = this.getSymbolOfTsNode(node.name); // which are of the form var _t3 = (_t2 as any as i2.TemplateRef<any>)
// 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) { if (symbol === null || symbol.tsSymbol === null) {
return 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 { private getShimPositionForNode(node: ts.Node): number {
if (ts.isTypeReferenceNode(node)) { if (ts.isTypeReferenceNode(node)) {
return this.getShimPositionForNode(node.typeName); return this.getShimPositionForNode(node.typeName);

View File

@ -133,6 +133,7 @@ runInEachFileSystem(() => {
it('should get a symbol for local ref which refers to a directive', () => { it('should get a symbol for local ref which refers to a directive', () => {
const symbol = templateTypeChecker.getSymbolOfNode(templateNode.references[1], cmp)!; const symbol = templateTypeChecker.getSymbolOfNode(templateNode.references[1], cmp)!;
assertReferenceSymbol(symbol); assertReferenceSymbol(symbol);
expect(program.getTypeChecker().symbolToString(symbol.tsSymbol)).toEqual('TestDir');
assertDirectiveReference(symbol); assertDirectiveReference(symbol);
}); });
@ -140,6 +141,7 @@ runInEachFileSystem(() => {
const symbol = templateTypeChecker.getSymbolOfNode( const symbol = templateTypeChecker.getSymbolOfNode(
(templateNode.children[0] as TmplAstTemplate).inputs[2].value, cmp)!; (templateNode.children[0] as TmplAstTemplate).inputs[2].value, cmp)!;
assertReferenceSymbol(symbol); assertReferenceSymbol(symbol);
expect(program.getTypeChecker().symbolToString(symbol.tsSymbol)).toEqual('TestDir');
assertDirectiveReference(symbol); assertDirectiveReference(symbol);
}); });
@ -737,6 +739,40 @@ runInEachFileSystem(() => {
}); });
describe('input bindings', () => { 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': `<div dir [inputA]=""></div>`},
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', () => { it('can retrieve a symbol for an input binding', () => {
const fileName = absoluteFrom('/main.ts'); const fileName = absoluteFrom('/main.ts');
const dirFile = absoluteFrom('/dir.ts'); const dirFile = absoluteFrom('/dir.ts');