refactor: remove files property

I added it as a property of UnitTestTree so it can be used for testing
where its actually useful. Otherwsie the visit() function acts in a
similar fashion.

Author: hansl

packages/angular_devkit/schematics/src/engine/schematic_spec.ts

@@ -42,6 +42,14 @@ const collection = {
 } as CollectionDescription<CollectionT>;
 
 
+function files(tree: Tree) {
+  const files: string[] = [];
+  tree.visit(x => files.push(x));
+
+  return files;
+}
+
+
 describe('Schematic', () => {
   it('works with a rule', done => {
     let inner: Tree | null = null;
@@ -62,8 +70,8 @@ describe('Schematic', () => {
     schematic.call({}, Observable.of(empty()))
       .toPromise()
       .then(x => {
-        expect(inner !.files).toEqual([]);
-        expect(x.files).toEqual(['/a/b/c']);
+        expect(files(inner !)).toEqual([]);
+        expect(files(x)).toEqual(['/a/b/c']);
       })
       .then(done, done.fail);
   });
@@ -87,8 +95,8 @@ describe('Schematic', () => {
     schematic.call({}, Observable.of(empty()))
       .toPromise()
       .then(x => {
-        expect(inner !.files).toEqual([]);
-        expect(x.files).toEqual([]);
+        expect(files(inner !)).toEqual([]);
+        expect(files(x)).toEqual([]);
         expect(inner).not.toBe(x);
       })
       .then(done, done.fail);

packages/angular_devkit/schematics/src/index.ts

@@ -24,6 +24,7 @@ export * from './rules/random';
 export * from './rules/schematic';
 export * from './rules/template';
 export * from './rules/url';
+export * from './tree/delegate';
 export * from './tree/empty';
 export * from './tree/filesystem';
 export * from './tree/memory-host';

packages/angular_devkit/schematics/src/rules/base.ts

@@ -133,8 +133,7 @@ export function partitionApplyMerge(
 
 export function forEach(operator: FileOperator): Rule {
   return (tree: Tree) => {
-    tree.files.forEach(path => {
-      const entry = tree.get(path);
+    tree.visit((path, entry) => {
       if (!entry) {
         return;
       }

packages/angular_devkit/schematics/src/tree/delegate.ts

@@ -0,0 +1,51 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import { Action } from './action';
+import {
+  DirEntry,
+  FileEntry,
+  FileVisitor,
+  MergeStrategy,
+  Tree,
+  UpdateRecorder,
+} from './interface';
+
+export class DelegateTree implements Tree {
+  constructor(protected _other: Tree) {}
+
+  branch(): Tree { return this._other.branch(); }
+  merge(other: Tree, strategy?: MergeStrategy): void { this._other.merge(other, strategy); }
+
+  get root(): DirEntry { return this._other.root; }
+
+  // Readonly.
+  read(path: string): Buffer | null { return this._other.read(path); }
+  exists(path: string): boolean { return this._other.exists(path); }
+  get(path: string): FileEntry | null { return this._other.get(path); }
+  getDir(path: string): DirEntry { return this._other.getDir(path); }
+  visit(visitor: FileVisitor): void { return this._other.visit(visitor); }
+
+  // Change content of host files.
+  overwrite(path: string, content: Buffer | string): void {
+    return this._other.overwrite(path, content);
+  }
+  beginUpdate(path: string): UpdateRecorder { return this._other.beginUpdate(path); }
+  commitUpdate(record: UpdateRecorder): void { return this._other.commitUpdate(record); }
+
+  // Structural methods.
+  create(path: string, content: Buffer | string): void {
+    return this._other.create(path, content);
+  }
+  delete(path: string): void { return this._other.delete(path); }
+  rename(from: string, to: string): void { return this._other.rename(from, to); }
+
+  apply(action: Action, strategy?: MergeStrategy): void {
+    return this._other.apply(action, strategy);
+  }
+  get actions(): Action[] { return this._other.actions; }
+}

packages/angular_devkit/schematics/src/tree/interface.ts

@@ -51,18 +51,22 @@ export interface FilePredicate<T> {
   (path: Path, entry?: Readonly<FileEntry> | null): T;
 }
 
+export const FileVisitorCancelToken = Symbol();
+export type FileVisitor = FilePredicate<void>;
+
 
 export interface Tree {
+  branch(): Tree;
+  merge(other: Tree, strategy?: MergeStrategy): void;
+
   readonly root: DirEntry;
 
   // Readonly.
-  readonly files: string[];
-  exists(path: string): boolean;
-
-  // Content access.
   read(path: string): Buffer | null;
+  exists(path: string): boolean;
   get(path: string): FileEntry | null;
   getDir(path: string): DirEntry;
+  visit(visitor: FileVisitor): void;
 
   // Change content of host files.
   overwrite(path: string, content: Buffer | string): void;

packages/angular_devkit/schematics/src/tree/null.ts

@@ -42,14 +42,19 @@ export class NullTreeDirEntry implements DirEntry {
 
 
 export class NullTree implements Tree {
+  branch(): Tree {
+    return new NullTree();
+  }
+  merge(_other: Tree, _strategy?: MergeStrategy): void {}
+
   readonly root: DirEntry = new NullTreeDirEntry(normalize('/'));
 
   // Simple readonly file system operations.
   exists(_path: string) { return false; }
   read(_path: string) { return null; }
   get(_path: string) { return null; }
   getDir(path: string) { return new NullTreeDirEntry(normalize('/' + path)); }
-  get files(): string[] { return []; }
+  visit() {}
 
   // Change content of host files.
   beginUpdate(path: string): never {

packages/angular_devkit/schematics/src/tree/virtual.ts

@@ -23,7 +23,15 @@ import {
 } from '../exception/exception';
 import { Action, ActionList, UnknownActionException } from './action';
 import { SimpleFileEntry } from './entry';
-import { DirEntry, FileEntry, MergeStrategy, Tree, UpdateRecorder } from './interface';
+import {
+  DirEntry,
+  FileEntry,
+  FileVisitor,
+  FileVisitorCancelToken,
+  MergeStrategy,
+  Tree,
+  UpdateRecorder,
+} from './interface';
 import { UpdateRecorderBase } from './recorder';
 
 
@@ -126,6 +134,16 @@ export class VirtualTree implements Tree {
     return dir;
   }
 
+  visit(visitor: FileVisitor) {
+    try {
+      this.files.forEach(path => visitor(path, this.get(path)));
+    } catch (e) {
+      if (e !== FileVisitorCancelToken) {
+        throw e;
+      }
+    }
+  }
+
   beginUpdate(path: string): UpdateRecorder {
     const entry = this.get(path);
     if (!entry) {
@@ -308,12 +326,12 @@ export class VirtualTree implements Tree {
   }
 
   static branch(tree: Tree) {
-    return (tree as VirtualTree).branch();
+    return tree.branch();
   }
 
   static merge(tree: Tree, other: Tree, strategy: MergeStrategy = MergeStrategy.Default): Tree {
-    const newTree = (tree as VirtualTree).branch() as VirtualTree;
-    newTree.merge((other as VirtualTree), strategy);
+    const newTree = tree.branch();
+    newTree.merge(other, strategy);
 
     return newTree;
   }

packages/angular_devkit/schematics/src/tree/virtual_spec.ts

@@ -9,12 +9,20 @@
 import { normalize } from '@angular-devkit/core';
 import { FileAlreadyExistException, FileDoesNotExistException } from '../exception/exception';
 import { FileSystemTree } from './filesystem';
-import { FileEntry, MergeStrategy } from './interface';
+import { FileEntry, MergeStrategy, Tree } from './interface';
 import { InMemoryFileSystemTreeHost } from './memory-host';
 import { merge, partition } from './static';
 import { VirtualTree } from './virtual';
 
 
+function files(tree: Tree) {
+  const treeFiles: string[] = [];
+  tree.visit(x => treeFiles.push(x));
+
+  return treeFiles;
+}
+
+
 describe('VirtualTree', () => {
   describe('exists()', () => {
     it('works', () => {
@@ -39,7 +47,7 @@ describe('VirtualTree', () => {
       tree.create('/some/other-file', 'some _content');
       tree.create('/some/other-file2', 'some _content');
 
-      expect(tree.files).toEqual([
+      expect(files(tree)).toEqual([
         '/some/file', '/some/other-file', '/some/other-file2',
       ].map(normalize));
     });
@@ -91,7 +99,7 @@ describe('VirtualTree', () => {
       expect(tree2.exists('file3')).toBe(true);
 
       const tree3 = merge(tree1, tree2, MergeStrategy.Error);
-      expect(tree3.files.sort()).toEqual(tree.files.sort());
+      expect(files(tree3).sort()).toEqual(files(tree).sort());
     });
   });
 
@@ -127,8 +135,8 @@ describe('VirtualTree', () => {
       tree.overwrite('/hello', 'world');
       tree.overwrite('/test', 'test 3');
 
-      const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test'];
-      expect(tree.files).toEqual(files.map(normalize));
+      const expected = ['/hello', '/sub/directory/file2', '/sub/file1', '/test'];
+      expect(files(tree)).toEqual(expected.map(normalize));
 
       const tree2 = tree.branch() as VirtualTree;
       expect(tree.actions.length).toBe(4);

packages/angular_devkit/schematics/testing/schematic-test-runner.ts

@@ -8,6 +8,7 @@
 import { Logger, schema } from '@angular-devkit/core';
 import {
   Collection,
+  DelegateTree,
   SchematicEngine,
   Tree,
   VirtualTree,
@@ -21,6 +22,16 @@ import { Observable } from 'rxjs/Observable';
 
 export interface SchematicSchemaT {}
 
+
+export class UnitTestTree extends DelegateTree {
+  get files() {
+    const result: string[] = [];
+    this.visit(path => result.push(path));
+
+    return result;
+  }
+}
+
 export class SchematicTestRunner {
   private _engineHost = new NodeModulesTestEngineHost();
   private _engine: SchematicEngine<{}, {}> = new SchematicEngine(this._engineHost);
@@ -57,21 +68,26 @@ export class SchematicTestRunner {
 
   get logger() { return this._logger; }
 
-  runSchematicAsync(schematicName: string, opts?: SchematicSchemaT, tree?: Tree): Observable<Tree> {
+  runSchematicAsync(
+    schematicName: string,
+    opts?: SchematicSchemaT,
+    tree?: Tree,
+  ): Observable<UnitTestTree> {
     const schematic = this._collection.createSchematic(schematicName);
     const host = Observable.of(tree || new VirtualTree);
 
-    return schematic.call(opts || {}, host, { logger: this._logger });
+    return schematic.call(opts || {}, host, { logger: this._logger })
+      .map(tree => new UnitTestTree(tree));
   }
 
-  runSchematic(schematicName: string, opts?: SchematicSchemaT, tree?: Tree): Tree {
+  runSchematic(schematicName: string, opts?: SchematicSchemaT, tree?: Tree): UnitTestTree {
     const schematic = this._collection.createSchematic(schematicName);
 
-    let result: Tree | null = null;
+    let result: UnitTestTree | null = null;
     const host = Observable.of(tree || new VirtualTree);
 
     schematic.call(opts || {}, host, { logger: this._logger })
-      .subscribe(t => result = t);
+      .subscribe(t => result = new UnitTestTree(t));
 
     if (result === null) {
       throw new Error('Schematic is async, please use runSchematicAsync');

packages/schematics/angular/application/index_spec.ts

@@ -5,8 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import { Tree } from '@angular-devkit/schematics';
-import { SchematicTestRunner } from '@angular-devkit/schematics/testing';
+import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing';
 import * as path from 'path';
 import { getFileContent } from '../utility/test';
 import { Schema as ApplicationOptions } from './schema';
@@ -72,7 +71,7 @@ describe('Application Schematic', () => {
   it('should handle a different sourceDir', () => {
     const options = { ...defaultOptions, sourceDir: 'some/custom/path' };
 
-    let tree: Tree | null = null;
+    let tree: UnitTestTree | null = null;
     expect(() => tree = schematicRunner.runSchematic('application', options))
       .not.toThrow();