fix(tools): harden colletor against invalid asts (#12793)

angular · Nov 10, 2016 · 69f87ca · 69f87ca
1 parent f224ca1
commit 69f87ca
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 16 deletions.
diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts
@@ -205,22 +205,27 @@ export class MetadataCollector {
       switch (node.kind) {
         case ts.SyntaxKind.ClassDeclaration:
           const classDeclaration = <ts.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:
           if (!(node.flags & ts.NodeFlags.Export)) {
             // Report references to this function as an error.
             const functionDeclaration = <ts.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 = <ts.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
@@ -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
@@ -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<string, string>();
+  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');