From 6650d71fe2a958d277e7a7f409251fe648236a76 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sun, 27 Sep 2020 13:03:21 +0100 Subject: [PATCH] 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 --- .../ngcc/test/host/esm2015_host_spec.ts | 30 ++++++------ .../src/ngtsc/testing/src/utils.ts | 46 ++++++++++++------- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts index a0be48dc70..d0c209f871 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -13,7 +13,7 @@ import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/test import {MockLogger} from '../../../src/ngtsc/logging/testing'; import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection'; 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 {DelegatingReflectionHost} from '../../src/host/delegating_host'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; @@ -1732,13 +1732,13 @@ runInEachFileSystem(() => { const classDeclaration = getDeclaration( bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', ts.isVariableDeclaration); - const innerClassDeclaration = - walkForDeclaration('InnerDecoratedWrappedClass', classDeclaration); - if (innerClassDeclaration === null) { + const innerClassDeclarations = + walkForDeclarations('InnerDecoratedWrappedClass', classDeclaration); + if (innerClassDeclarations.length === 0) { throw new Error('Expected InnerDecoratedWrappedClass to exist'); } 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'); const d = host.getDeclarationOfIdentifier(aliasedClassIdentifier); expect(d!.node).toBe(classDeclaration); @@ -2065,18 +2065,18 @@ runInEachFileSystem(() => { const outerNode = getDeclaration( bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', isNamedVariableDeclaration); - const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); - if (innerNode === null) { + const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode); + if (innerNodes.length === 0) { throw new Error('Expected to find InnerDecoratedWrappedClass'); } - const classSymbol = host.getClassSymbol(innerNode); + const classSymbol = host.getClassSymbol(innerNodes[0]); if (classSymbol === undefined) { return fail('Expected classSymbol to be defined'); } expect(classSymbol.name).toEqual('DecoratedWrappedClass'); expect(classSymbol.declaration.valueDeclaration).toBe(outerNode); - expect(classSymbol.implementation.valueDeclaration).toBe(innerNode); + expect(classSymbol.implementation.valueDeclaration).toBe(innerNodes[0]); if (classSymbol.adjacent === undefined || !isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) { @@ -2096,8 +2096,8 @@ runInEachFileSystem(() => { const outerNode = getDeclaration( bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', isNamedVariableDeclaration); - const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); - if (innerNode === null) { + const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode); + if (innerNodes.length === 0) { throw new Error('Expected to find InnerDecoratedWrappedClass'); } const adjacentNode: ts.ClassExpression = @@ -2112,7 +2112,7 @@ runInEachFileSystem(() => { } expect(classSymbol.name).toEqual('DecoratedWrappedClass'); expect(classSymbol.declaration.valueDeclaration).toBe(outerNode); - expect(classSymbol.implementation.valueDeclaration).toBe(innerNode); + expect(classSymbol.implementation.valueDeclaration).toBe(innerNodes[0]); if (classSymbol.adjacent === undefined || !isNamedVariableDeclaration(classSymbol.adjacent.valueDeclaration)) { @@ -2131,12 +2131,12 @@ runInEachFileSystem(() => { const outerNode = getDeclaration( bundle.program, WRAPPED_CLASS_EXPRESSION_FILE.name, 'DecoratedWrappedClass', isNamedVariableDeclaration); - const innerNode = walkForDeclaration('InnerDecoratedWrappedClass', outerNode); - if (innerNode === null) { + const innerNodes = walkForDeclarations('InnerDecoratedWrappedClass', outerNode); + if (innerNodes.length === 0) { throw new Error('Expected to find InnerDecoratedWrappedClass'); } - const innerSymbol = host.getClassSymbol(innerNode)!; + const innerSymbol = host.getClassSymbol(innerNodes[0])!; const outerSymbol = host.getClassSymbol(outerNode)!; expect(innerSymbol.declaration).toBe(outerSymbol.declaration); expect(innerSymbol.implementation).toBe(outerSymbol.implementation); diff --git a/packages/compiler-cli/src/ngtsc/testing/src/utils.ts b/packages/compiler-cli/src/ngtsc/testing/src/utils.ts index d1597de5a6..fa339eefce 100644 --- a/packages/compiler-cli/src/ngtsc/testing/src/utils.ts +++ b/packages/compiler-cli/src/ngtsc/testing/src/utils.ts @@ -50,52 +50,64 @@ export function makeProgram( 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( program: ts.Program, fileName: AbsoluteFsPath, name: string, assert: (value: any) => value is T): T { 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}`); } - if (!assert(chosenDecl)) { - throw new Error(`Symbol ${name} from ${fileName} is a ${ - ts.SyntaxKind[chosenDecl.kind]}. Expected it to pass predicate "${assert.name}()".`); + const chosenDecl = chosenDecls.find(assert); + if (chosenDecl === undefined) { + 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; } -// We walk the AST tree looking for a declaration that matches -export function walkForDeclaration(name: string, rootNode: ts.Node): ts.Declaration|null { - let chosenDecl: ts.Declaration|null = null; +/** + * Walk the AST tree from the `rootNode` looking for a declaration that has the given `name`. + */ +export function walkForDeclarations(name: string, rootNode: ts.Node): ts.Declaration[] { + const chosenDecls: ts.Declaration[] = []; rootNode.forEachChild(node => { - if (chosenDecl !== null) { - return; - } if (ts.isVariableStatement(node)) { node.declarationList.declarations.forEach(decl => { if (bindingNameEquals(decl.name, name)) { - chosenDecl = decl; + chosenDecls.push(decl); + if (decl.initializer) { + chosenDecls.push(...walkForDeclarations(name, decl.initializer)); + } } else { - chosenDecl = walkForDeclaration(name, node); + chosenDecls.push(...walkForDeclarations(name, node)); } }); } else if ( ts.isClassDeclaration(node) || ts.isFunctionDeclaration(node) || ts.isInterfaceDeclaration(node) || ts.isClassExpression(node)) { if (node.name !== undefined && node.name.text === name) { - chosenDecl = node; + chosenDecls.push(node); } + chosenDecls.push(...walkForDeclarations(name, node)); } else if ( ts.isImportDeclaration(node) && node.importClause !== undefined && node.importClause.name !== undefined && node.importClause.name.text === name) { - chosenDecl = node.importClause; + chosenDecls.push(node.importClause); } else { - chosenDecl = walkForDeclaration(name, node); + chosenDecls.push(...walkForDeclarations(name, node)); } }); - return chosenDecl; + return chosenDecls; } const COMPLETE_REUSE_FAILURE_MESSAGE =