refactor: remove null from typings for descriptions

angular · Sep 15, 2017 · 4441735 · 4441735
1 parent 452bde1
commit 4441735
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 83 deletions.
diff --git a/packages/angular_devkit/schematics/BUILD b/packages/angular_devkit/schematics/BUILD
@@ -14,6 +14,7 @@ ts_library(
         exclude = ["src/**/*_spec.ts", "src/**/*_benchmark.ts"],
     ),
     deps = [
+      "//packages/angular_devkit/core",
         # @deps: rxjs
     ],
     tsconfig = "//:tsconfig.json",

diff --git a/packages/angular_devkit/schematics/src/engine/engine.ts b/packages/angular_devkit/schematics/src/engine/engine.ts
@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import { BaseException } from '@angular-devkit/core';
+import { CollectionDescription } from '@angular-devkit/schematics';
 import 'rxjs/add/operator/map';
 import { Url } from 'url';
 import { MergeStrategy } from '../tree/interface';
@@ -30,8 +31,8 @@ export class UnknownCollectionException extends BaseException {
   constructor(name: string) { super(`Unknown collection "${name}".`); }
 }
 export class UnknownSchematicException extends BaseException {
-  constructor(name: string, collection: Collection<{}, {}>) {
-    super(`Schematic "${name}" not found in collection "${collection.description.name}".`);
+  constructor(name: string, collection: CollectionDescription<{}>) {
+    super(`Schematic "${name}" not found in collection "${collection.name}".`);
   }
 }
 
@@ -83,7 +84,7 @@ export class SchematicEngine<CollectionT extends object, SchematicT extends obje
 
     const description = this._host.createSchematicDescription(name, collection.description);
     if (!description) {
-      throw new UnknownSchematicException(name, collection);
+      throw new UnknownSchematicException(name, collection.description);
     }
 
     const factory = this._host.getSchematicRuleFactory(description, collection.description);

diff --git a/packages/angular_devkit/schematics/src/engine/interface.ts b/packages/angular_devkit/schematics/src/engine/interface.ts
@@ -37,11 +37,11 @@ export type SchematicDescription<CollectionMetadataT extends object,
  * parameters contain additional metadata that you want to store while remaining type-safe.
  */
 export interface EngineHost<CollectionMetadataT extends object, SchematicMetadataT extends object> {
-  createCollectionDescription(name: string): CollectionDescription<CollectionMetadataT> | null;
+  createCollectionDescription(name: string): CollectionDescription<CollectionMetadataT>;
   createSchematicDescription(
       name: string,
       collection: CollectionDescription<CollectionMetadataT>):
-        SchematicDescription<CollectionMetadataT, SchematicMetadataT> | null;
+        SchematicDescription<CollectionMetadataT, SchematicMetadataT>;
   getSchematicRuleFactory<OptionT extends object>(
       schematic: SchematicDescription<CollectionMetadataT, SchematicMetadataT>,
       collection: CollectionDescription<CollectionMetadataT>): RuleFactory<OptionT>;

diff --git a/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts b/packages/angular_devkit/schematics/tools/file-system-engine-host-base.ts
@@ -5,12 +5,13 @@
  * 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 { JsonObject } from '@angular-devkit/core';
+import { BaseException, JsonObject } from '@angular-devkit/core';
 import {
   EngineHost,
   FileSystemTree,
   RuleFactory,
   Source,
+  UnknownSchematicException,
 } from '@angular-devkit/schematics';
 import { dirname, isAbsolute, join, resolve } from 'path';
 import { Url } from 'url';
@@ -30,27 +31,61 @@ export declare type OptionTransform<T extends object, R extends object>
     = (schematic: FileSystemSchematicDescription, options: T) => R;
 
 
+export class CollectionCannotBeResolvedException extends BaseException {
+  constructor(name: string) {
+    super(`Collection ${JSON.stringify(name)} cannot be resolved.`);
+  }
+}
+export class InvalidCollectionJsonException extends BaseException {
+  constructor(_name: string, path: string) {
+    super(`Collection JSON at path ${JSON.stringify(path)} is invalid.`);
+  }
+}
+export class SchematicMissingFactoryException extends BaseException {
+  constructor(name: string) {
+    super(`Schematic ${JSON.stringify(name)} is missing a factory.`);
+  }
+}
+export class FactoryCannotBeResolvedException extends BaseException {
+  constructor(name: string) {
+    super(`Schematic ${JSON.stringify(name)} cannot resolve the factory.`);
+  }
+}
+export class CollectionMissingSchematicsMapException extends BaseException {
+  constructor(name: string) { super(`Collection "${name}" does not have a schematics map.`); }
+}
+export class CollectionMissingFieldsException extends BaseException {
+  constructor(name: string) { super(`Collection "${name}" is missing fields.`); }
+}
+export class SchematicMissingFieldsException extends BaseException {
+  constructor(name: string) { super(`Schematic "${name}" is missing fields.`); }
+}
+export class SchematicMissingDescriptionException extends BaseException {
+  constructor(name: string) { super(`Schematics "${name}" does not have a description.`); }
+}
+
+
 /**
  * A EngineHost base class that uses the file system to resolve collections. This is the base of
  * all other EngineHost provided by the tooling part of the Schematics library.
  */
 export abstract class FileSystemEngineHostBase implements
     EngineHost<FileSystemCollectionDescription, FileSystemSchematicDescription> {
-  protected abstract _resolveCollectionPath(name: string): string | null;
+  protected abstract _resolveCollectionPath(name: string): string;
   protected abstract _resolveReferenceString(
       name: string, parentPath: string): { ref: RuleFactory<{}>, path: string } | null;
   protected abstract _transformCollectionDescription(
-      name: string, desc: Partial<FileSystemCollectionDesc>): FileSystemCollectionDesc | null;
+      name: string, desc: Partial<FileSystemCollectionDesc>): FileSystemCollectionDesc;
   protected abstract _transformSchematicDescription(
       name: string,
       collection: FileSystemCollectionDesc,
-      desc: Partial<FileSystemSchematicDesc>): FileSystemSchematicDesc | null;
+      desc: Partial<FileSystemSchematicDesc>): FileSystemSchematicDesc;
 
   private _transforms: OptionTransform<object, object>[] = [];
 
   listSchematics(collection: FileSystemCollection) {
     const schematics: string[] = [];
-    for (const key in collection.description.schematics) {
+    for (const key of Object.keys(collection.description.schematics)) {
       const schematic = collection.description.schematics[key];
 
       // If extends is present without a factory it is an alias, do not return it
@@ -74,42 +109,36 @@ export abstract class FileSystemEngineHostBase implements
    * @param name
    * @return {{path: string}}
    */
-  createCollectionDescription(name: string): FileSystemCollectionDesc | null {
-    try {
-      const path = this._resolveCollectionPath(name);
-      if (!path) {
-        return null;
-      }
-
-      const partialDesc: Partial<FileSystemCollectionDesc> | null = readJsonFile(path);
-      if (!partialDesc) {
-        return null;
-      }
-
-      const description = this._transformCollectionDescription(name, {
-        ...partialDesc,
-        path,
-      });
-      if (!description || !description.name) {
-        return null;
-      }
+  createCollectionDescription(name: string): FileSystemCollectionDesc {
+    const path = this._resolveCollectionPath(name);
+    const jsonValue = readJsonFile(path);
+    if (!jsonValue || typeof jsonValue != 'object') {
+      throw new InvalidCollectionJsonException(name, path);
+    }
 
-      return description;
-    } catch (e) {
-      return null;
+    const description = this._transformCollectionDescription(name, {
+      ...jsonValue,
+      path,
+    });
+    if (!description || !description.name) {
+      throw new InvalidCollectionJsonException(name, path);
     }
+
+    return description;
   }
 
   createSchematicDescription(
-      name: string, collection: FileSystemCollectionDesc): FileSystemSchematicDesc | null {
+    name: string,
+    collection: FileSystemCollectionDesc,
+  ): FileSystemSchematicDesc {
     if (!(name in collection.schematics)) {
-      return null;
+      throw new UnknownSchematicException(name, collection);
     }
 
     const collectionPath = dirname(collection.path);
     let partialDesc: Partial<FileSystemSchematicDesc> | null = collection.schematics[name];
     if (!partialDesc) {
-      return null;
+      throw new UnknownSchematicException(name, collection);
     }
 
     if (partialDesc.extends) {
@@ -121,26 +150,23 @@ export abstract class FileSystemEngineHostBase implements
       if (collectionName !== null) {
         // const extendCollection = engine.createCollection(collectionName);
         const extendCollection = this.createCollectionDescription(collectionName);
-        if (!extendCollection) {
-          return null;
-        }
         partialDesc = this.createSchematicDescription(schematicName, extendCollection);
       } else {
         partialDesc = this.createSchematicDescription(schematicName, collection);
       }
     }
     if (!partialDesc) {
-      return null;
+      throw new UnknownSchematicException(name, collection);
     }
 
     // Use any on this ref as we don't have the OptionT here, but we don't need it (we only need
     // the path).
     if (!partialDesc.factory) {
-      return null;
+      throw new SchematicMissingFactoryException(name);
     }
     const resolvedRef = this._resolveReferenceString(partialDesc.factory, collectionPath);
     if (!resolvedRef) {
-      return null;
+      throw new FactoryCannotBeResolvedException(name);
     }
 
     const { path } = resolvedRef;
@@ -153,7 +179,7 @@ export abstract class FileSystemEngineHostBase implements
       schemaJson = readJsonFile(schema) as JsonObject;
     }
 
-    const description = this._transformSchematicDescription(name, collection, {
+    return this._transformSchematicDescription(name, collection, {
       ...partialDesc,
       schema,
       schemaJson,
@@ -162,12 +188,6 @@ export abstract class FileSystemEngineHostBase implements
       factoryFn: resolvedRef.ref,
       collection,
     });
-
-    if (!description) {
-      return null;
-    }
-
-    return description;
   }
 
   createSourceFromUrl(url: Url): Source | null {

diff --git a/packages/angular_devkit/schematics/tools/file-system-engine-host.ts b/packages/angular_devkit/schematics/tools/file-system-engine-host.ts
@@ -6,6 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import { RuleFactory } from '@angular-devkit/schematics';
+import {
+  CollectionCannotBeResolvedException,
+  CollectionMissingFieldsException,
+  CollectionMissingSchematicsMapException,
+  SchematicMissingFieldsException,
+} from '@angular-devkit/schematics/tools';
 import { existsSync } from 'fs';
 import { join } from 'path';
 import { FileSystemCollectionDesc, FileSystemSchematicDesc } from './description';
@@ -20,7 +26,7 @@ import { FileSystemEngineHostBase } from './file-system-engine-host-base';
 export class FileSystemEngineHost extends FileSystemEngineHostBase {
   constructor(protected _root: string) { super(); }
 
-  protected _resolveCollectionPath(name: string): string | null {
+  protected _resolveCollectionPath(name: string): string {
     // Allow `${_root}/${name}.json` as a collection.
     if (existsSync(join(this._root, name + '.json'))) {
       return join(this._root, name + '.json');
@@ -31,7 +37,7 @@ export class FileSystemEngineHost extends FileSystemEngineHostBase {
       return join(this._root, name, 'collection.json');
     }
 
-    return null;
+    throw new CollectionCannotBeResolvedException(name);
   }
 
   protected _resolveReferenceString(refString: string, parentPath: string) {
@@ -45,26 +51,26 @@ export class FileSystemEngineHost extends FileSystemEngineHostBase {
   }
 
   protected _transformCollectionDescription(
-    _name: string,
+    name: string,
     desc: Partial<FileSystemCollectionDesc>,
-  ): FileSystemCollectionDesc | null {
-    if (!desc.name || !desc.path || !desc.schematics || !desc.version) {
-      return null;
+  ): FileSystemCollectionDesc {
+    if (!desc.name || !desc.path || !desc.version) {
+      throw new CollectionMissingFieldsException(name);
     }
-    if (typeof desc.schematics != 'object') {
-      return null;
+    if (!desc.schematics || typeof desc.schematics != 'object') {
+      throw new CollectionMissingSchematicsMapException(name);
     }
 
     return desc as FileSystemCollectionDesc;
   }
 
   protected _transformSchematicDescription(
-    _name: string,
+    name: string,
     _collection: FileSystemCollectionDesc,
     desc: Partial<FileSystemSchematicDesc>,
-  ): FileSystemSchematicDesc | null {
+  ): FileSystemSchematicDesc {
     if (!desc.factoryFn || !desc.path || !desc.description) {
-      return null;
+      throw new SchematicMissingFieldsException(name);
     }
 
     return desc as FileSystemSchematicDesc;

diff --git a/packages/angular_devkit/schematics/tools/node-module-engine-host.ts b/packages/angular_devkit/schematics/tools/node-module-engine-host.ts
@@ -5,8 +5,12 @@
  * 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 { BaseException } from '@angular-devkit/core';
 import { RuleFactory } from '@angular-devkit/schematics';
+import {
+  CollectionCannotBeResolvedException,
+  CollectionMissingFieldsException,
+  CollectionMissingSchematicsMapException, SchematicMissingFieldsException,
+} from '@angular-devkit/schematics/tools';
 import { join } from 'path';
 import {
   FileSystemCollectionDesc,
@@ -16,22 +20,14 @@ import { ExportStringRef } from './export-ref';
 import { FileSystemEngineHostBase } from './file-system-engine-host-base';
 
 
-export class CollectionMissingSchematicsMapException extends BaseException {
-  constructor(name: string) { super(`Collection "${name}" does not have a schematics map.`); }
-}
-export class SchematicMissingDescriptionException extends BaseException {
-  constructor(name: string) { super(`Schematics "${name}" does not have a description.`); }
-}
-
-
 /**
  * A simple EngineHost that uses NodeModules to resolve collections.
  */
 export class NodeModulesEngineHost extends FileSystemEngineHostBase {
-  protected _resolveCollectionPath(name: string): string | null {
+  protected _resolveCollectionPath(name: string): string {
     const pkgJsonSchematics = require(join(name, 'package.json'))['schematics'];
     if (!pkgJsonSchematics) {
-      return null;
+      throw new CollectionCannotBeResolvedException(name);
     }
 
     return require.resolve(join(name, pkgJsonSchematics));
@@ -49,7 +45,10 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
   protected _transformCollectionDescription(
     name: string,
     desc: Partial<FileSystemCollectionDesc>,
-  ): FileSystemCollectionDesc | null {
+  ): FileSystemCollectionDesc {
+    if (!desc.name || !desc.path || !desc.version) {
+      throw new CollectionMissingFieldsException(name);
+    }
     if (!desc.schematics || typeof desc.schematics != 'object') {
       throw new CollectionMissingSchematicsMapException(name);
     }
@@ -63,12 +62,12 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
   }
 
   protected _transformSchematicDescription(
-    _name: string,
+    name: string,
     _collection: FileSystemCollectionDesc,
     desc: Partial<FileSystemSchematicDesc>,
-  ): FileSystemSchematicDesc | null {
+  ): FileSystemSchematicDesc {
     if (!desc.factoryFn || !desc.path || !desc.description) {
-      return null;
+      throw new SchematicMissingFieldsException(name);
     }
 
     return desc as FileSystemSchematicDesc;