test(compiler-cli): make the `getDeclaration()` utility more resilient to code format (#38959)

Previously `getDeclaration()` would only return the first node that matched
the name passed in and then assert the predicate on this single node.
It also only considered a subset of possible declaration types that we might
care about.

Now the function will parse the whole tree collecting an array of all the
nodes that match the name. It then filters this array based on the predicate
and only errors if the filtered array is empty.

This makes this function much more resilient to more esoteric code formats
such as UMD.

PR Close #38959
This commit is contained in:
Pete Bacon Darwin 2020-09-27 13:03:21 +01:00 committed by atscott
parent 65997c0649
commit 6650d71fe2
2 changed files with 44 additions and 32 deletions

View File

@ -13,7 +13,7 @@ import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/test
import {MockLogger} from '../../../src/ngtsc/logging/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection'; import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing'; import {getDeclaration} from '../../../src/ngtsc/testing';
import {walkForDeclaration} from '../../../src/ngtsc/testing/src/utils'; import {walkForDeclarations} from '../../../src/ngtsc/testing/src/utils';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers'; import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {DelegatingReflectionHost} from '../../src/host/delegating_host'; import {DelegatingReflectionHost} from '../../src/host/delegating_host';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
@ -1732,13 +1732,13 @@ runInEachFileSystem(() => {
const classDeclaration = getDeclaration( const classDeclaration = getDeclaration(
bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass',
ts.isVariableDeclaration); ts.isVariableDeclaration);
const innerClassDeclaration = const innerClassDeclarations =
walkForDeclaration('InnerDecoratedWrappedClass', classDeclaration); walkForDeclarations('InnerDecoratedWrappedClass', classDeclaration);
if (innerClassDeclaration === null) { if (innerClassDeclarations.length === 0) {
throw new Error('Expected InnerDecoratedWrappedClass to exist'); throw new Error('Expected InnerDecoratedWrappedClass to exist');
} }
const aliasedClassIdentifier = const aliasedClassIdentifier =
(innerClassDeclaration.parent as ts.BinaryExpression).left as ts.Identifier; (innerClassDeclarations[0].parent as ts.BinaryExpression).left as ts.Identifier;
expect(aliasedClassIdentifier.text).toBe('DecoratedWrappedClass_1'); expect(aliasedClassIdentifier.text).toBe('DecoratedWrappedClass_1');
const d = host.getDeclarationOfIdentifier(aliasedClassIdentifier); const d = host.getDeclarationOfIdentifier(aliasedClassIdentifier);
expect(d!.node).toBe(classDeclaration); expect(d!.node).toBe(classDeclaration);
@ -2065,18 +2065,18 @@ runInEachFileSystem(() => {
const outerNode = getDeclaration( const outerNode = getDeclaration(
bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass',
isNamedVariableDeclaration); isNamedVariableDeclaration);
const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode);
if (innerNode === null) { if (innerNodes.length === 0) {
throw new Error('Expected to find InnerDecoratedWrappedClass'); throw new Error('Expected to find InnerDecoratedWrappedClass');
} }
const classSymbol = host.getClassSymbol(innerNode); const classSymbol = host.getClassSymbol(innerNodes[0]);
if (classSymbol === undefined) { if (classSymbol === undefined) {
return fail('Expected classSymbol to be defined'); return fail('Expected classSymbol to be defined');
} }
expect(classSymbol.name).toEqual('DecoratedWrappedClass'); expect(classSymbol.name).toEqual('DecoratedWrappedClass');
expect(classSymbol.declaration.valueDeclaration).toBe(outerNode); expect(classSymbol.declaration.valueDeclaration).toBe(outerNode);
expect(classSymbol.implementation.valueDeclaration).toBe(innerNode); expect(classSymbol.implementation.valueDeclaration).toBe(innerNodes[0]);
if (classSymbol.adjacent === undefined || if (classSymbol.adjacent === undefined ||
!isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) { !isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) {
@ -2096,8 +2096,8 @@ runInEachFileSystem(() => {
const outerNode = getDeclaration( const outerNode = getDeclaration(
bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass',
isNamedVariableDeclaration); isNamedVariableDeclaration);
const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode);
if (innerNode === null) { if (innerNodes.length === 0) {
throw new Error('Expected to find InnerDecoratedWrappedClass'); throw new Error('Expected to find InnerDecoratedWrappedClass');
} }
const adjacentNode: ts.ClassExpression = const adjacentNode: ts.ClassExpression =
@ -2112,7 +2112,7 @@ runInEachFileSystem(() => {
} }
expect(classSymbol.name).toEqual('DecoratedWrappedClass'); expect(classSymbol.name).toEqual('DecoratedWrappedClass');
expect(classSymbol.declaration.valueDeclaration).toBe(outerNode); expect(classSymbol.declaration.valueDeclaration).toBe(outerNode);
expect(classSymbol.implementation.valueDeclaration).toBe(innerNode); expect(classSymbol.implementation.valueDeclaration).toBe(innerNodes[0]);
if (classSymbol.adjacent === undefined || if (classSymbol.adjacent === undefined ||
!isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) { !isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) {
@ -2131,12 +2131,12 @@ runInEachFileSystem(() => {
const outerNode = getDeclaration( const outerNode = getDeclaration(
bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass',
isNamedVariableDeclaration); isNamedVariableDeclaration);
const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode);
if (innerNode === null) { if (innerNodes.length === 0) {
throw new Error('Expected to find InnerDecoratedWrappedClass'); throw new Error('Expected to find InnerDecoratedWrappedClass');
} }
const innerSymbol = host.getClassSymbol(innerNode)!; const innerSymbol = host.getClassSymbol(innerNodes[0])!;
const outerSymbol = host.getClassSymbol(outerNode)!; const outerSymbol = host.getClassSymbol(outerNode)!;
expect(innerSymbol.declaration).toBe(outerSymbol.declaration); expect(innerSymbol.declaration).toBe(outerSymbol.declaration);
expect(innerSymbol.implementation).toBe(outerSymbol.implementation); expect(innerSymbol.implementation).toBe(outerSymbol.implementation);

View File

@ -50,52 +50,64 @@ export function makeProgram(
return {program, host: compilerHost, options: compilerOptions}; return {program, host: compilerHost, options: compilerOptions};
} }
/**
* Search the file specified by `fileName` in the given `program` for a declaration that has the
* name `name` and passes the `predicate` function.
*
* An error will be thrown if there is not at least one AST node with the given `name` and passes
* the `predicate` test.
*/
export function getDeclaration<T extends ts.Declaration>( export function getDeclaration<T extends ts.Declaration>(
program: ts.Program, fileName: AbsoluteFsPath, name: string, program: ts.Program, fileName: AbsoluteFsPath, name: string,
assert: (value: any) => value is T): T { assert: (value: any) => value is T): T {
const sf = getSourceFileOrError(program, fileName); const sf = getSourceFileOrError(program, fileName);
const chosenDecl = walkForDeclaration(name, sf); const chosenDecls = walkForDeclarations(name, sf);
if (chosenDecl === null) { if (chosenDecls.length === 0) {
throw new Error(`No such symbol: ${name} in ${fileName}`); throw new Error(`No such symbol: ${name} in ${fileName}`);
} }
if (!assert(chosenDecl)) { const chosenDecl = chosenDecls.find(assert);
throw new Error(`Symbol ${name} from ${fileName} is a ${ if (chosenDecl === undefined) {
ts.SyntaxKind[chosenDecl.kind]}. Expected it to pass predicate "${assert.name}()".`); throw new Error(`Symbols with name ${name} in ${fileName} have types: ${
chosenDecls.map(decl => ts.SyntaxKind[decl.kind])}. Expected one to pass predicate "${
assert.name}()".`);
} }
return chosenDecl; return chosenDecl;
} }
// We walk the AST tree looking for a declaration that matches /**
export function walkForDeclaration(name: string, rootNode: ts.Node): ts.Declaration|null { * Walk the AST tree from the `rootNode` looking for a declaration that has the given `name`.
let chosenDecl: ts.Declaration|null = null; */
export function walkForDeclarations(name: string, rootNode: ts.Node): ts.Declaration[] {
const chosenDecls: ts.Declaration[] = [];
rootNode.forEachChild(node => { rootNode.forEachChild(node => {
if (chosenDecl !== null) {
return;
}
if (ts.isVariableStatement(node)) { if (ts.isVariableStatement(node)) {
node.declarationList.declarations.forEach(decl => { node.declarationList.declarations.forEach(decl => {
if (bindingNameEquals(decl.name, name)) { if (bindingNameEquals(decl.name, name)) {
chosenDecl = decl; chosenDecls.push(decl);
if (decl.initializer) {
chosenDecls.push(...walkForDeclarations(name, decl.initializer));
}
} else { } else {
chosenDecl = walkForDeclaration(name, node); chosenDecls.push(...walkForDeclarations(name, node));
} }
}); });
} else if ( } else if (
ts.isClassDeclaration(node) || ts.isFunctionDeclaration(node) || ts.isClassDeclaration(node) || ts.isFunctionDeclaration(node) ||
ts.isInterfaceDeclaration(node) || ts.isClassExpression(node)) { ts.isInterfaceDeclaration(node) || ts.isClassExpression(node)) {
if (node.name !== undefined && node.name.text === name) { if (node.name !== undefined && node.name.text === name) {
chosenDecl = node; chosenDecls.push(node);
} }
chosenDecls.push(...walkForDeclarations(name, node));
} else if ( } else if (
ts.isImportDeclaration(node) && node.importClause !== undefined && ts.isImportDeclaration(node) && node.importClause !== undefined &&
node.importClause.name !== undefined && node.importClause.name.text === name) { node.importClause.name !== undefined && node.importClause.name.text === name) {
chosenDecl = node.importClause; chosenDecls.push(node.importClause);
} else { } else {
chosenDecl = walkForDeclaration(name, node); chosenDecls.push(...walkForDeclarations(name, node));
} }
}); });
return chosenDecl; return chosenDecls;
} }
const COMPLETE_REUSE_FAILURE_MESSAGE = const COMPLETE_REUSE_FAILURE_MESSAGE =