From 69f87ca075b046d45760538bf12bb78cb597954d Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Thu, 10 Nov 2016 11:58:55 -0800 Subject: [PATCH] fix(tools): harden colletor against invalid asts (#12793) --- tools/@angular/tsc-wrapped/src/collector.ts | 35 +++++++++++-------- .../tsc-wrapped/test/collector.spec.ts | 24 ++++++++++++- .../tsc-wrapped/test/typescript.mocks.ts | 18 +++++++++- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 28db8f5b63..381043ce54 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -205,12 +205,14 @@ export class MetadataCollector { switch (node.kind) { case ts.SyntaxKind.ClassDeclaration: const classDeclaration = node; - const className = classDeclaration.name.text; - if (node.flags & ts.NodeFlags.Export) { - locals.define(className, {__symbolic: 'reference', name: className}); - } else { - locals.define( - className, errorSym('Reference to non-exported class', node, {className})); + if (classDeclaration.name) { + const className = classDeclaration.name.text; + if (node.flags & ts.NodeFlags.Export) { + locals.define(className, {__symbolic: 'reference', name: className}); + } else { + locals.define( + className, errorSym('Reference to non-exported class', node, {className})); + } } break; case ts.SyntaxKind.FunctionDeclaration: @@ -218,9 +220,12 @@ export class MetadataCollector { // Report references to this function as an error. const functionDeclaration = node; const nameNode = functionDeclaration.name; - locals.define( - nameNode.text, - errorSym('Reference to a non-exported function', nameNode, {name: nameNode.text})); + if (nameNode && nameNode.text) { + locals.define( + nameNode.text, + errorSym( + 'Reference to a non-exported function', nameNode, {name: nameNode.text})); + } } break; } @@ -248,11 +253,13 @@ export class MetadataCollector { break; case ts.SyntaxKind.ClassDeclaration: const classDeclaration = node; - const className = classDeclaration.name.text; - if (node.flags & ts.NodeFlags.Export) { - if (classDeclaration.decorators) { - if (!metadata) metadata = {}; - metadata[className] = classMetadataOf(classDeclaration); + if (classDeclaration.name) { + const className = classDeclaration.name.text; + if (node.flags & ts.NodeFlags.Export) { + if (classDeclaration.decorators) { + if (!metadata) metadata = {}; + metadata[className] = classMetadataOf(classDeclaration); + } } } // Otherwise don't record metadata for the class. diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index 7b2d5201ee..931bc32892 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/tools/@angular/tsc-wrapped/test/collector.spec.ts @@ -15,7 +15,7 @@ import {Directory, Host, expectValidSources} from './typescript.mocks'; describe('Collector', () => { let documentRegistry = ts.createDocumentRegistry(); - let host: ts.LanguageServiceHost; + let host: Host; let service: ts.LanguageService; let program: ts.Program; let collector: MetadataCollector; @@ -589,6 +589,28 @@ describe('Collector', () => { .toThrowError(/Reference to non-exported class/); }); }); + + describe('with invalid input', () => { + it('should not throw with a class with no name', () => { + const fileName = '/invalid-class.ts'; + override(fileName, 'export class'); + let invalidClass = program.getSourceFile(fileName); + expect(() => collector.getMetadata(invalidClass)).not.toThrow(); + }); + + it('should not throw with a function with no name', () => { + const fileName = '/invalid-function.ts'; + override(fileName, 'export function'); + let invalidFunction = program.getSourceFile(fileName); + expect(() => collector.getMetadata(invalidFunction)).not.toThrow(); + }); + }); + + function override(fileName: string, content: string) { + host.overrideFile(fileName, content); + host.addFile(fileName); + program = service.getProgram(); + } }); // TODO: Do not use \` in a template literal as it confuses clang-format diff --git a/tools/@angular/tsc-wrapped/test/typescript.mocks.ts b/tools/@angular/tsc-wrapped/test/typescript.mocks.ts index d3f7e01485..72b78ca96e 100644 --- a/tools/@angular/tsc-wrapped/test/typescript.mocks.ts +++ b/tools/@angular/tsc-wrapped/test/typescript.mocks.ts @@ -5,6 +5,9 @@ import * as ts from 'typescript'; export interface Directory { [name: string]: (Directory|string); } export class Host implements ts.LanguageServiceHost { + private overrides = new Map(); + private version = 1; + constructor(private directory: Directory, private scripts: string[]) {} getCompilationSettings(): ts.CompilerOptions { @@ -17,7 +20,7 @@ export class Host implements ts.LanguageServiceHost { getScriptFileNames(): string[] { return this.scripts; } - getScriptVersion(fileName: string): string { return '1'; } + getScriptVersion(fileName: string): string { return this.version.toString(); } getScriptSnapshot(fileName: string): ts.IScriptSnapshot { let content = this.getFileContent(fileName); @@ -28,7 +31,20 @@ export class Host implements ts.LanguageServiceHost { getDefaultLibFileName(options: ts.CompilerOptions): string { return 'lib.d.ts'; } + overrideFile(fileName: string, content: string) { + this.overrides.set(fileName, content); + this.version++; + } + + addFile(fileName: string) { + this.scripts.push(fileName); + this.version++; + } + private getFileContent(fileName: string): string { + if (this.overrides.has(fileName)) { + return this.overrides.get(fileName); + } const names = fileName.split('/'); if (names[names.length - 1] === 'lib.d.ts') { return fs.readFileSync(ts.getDefaultLibFilePath(this.getCompilationSettings()), 'utf8');