refactor: add a linting rule to disallow "!" non-null op

hansl · hansl · commit e8b29a96cfe9 · 2018-06-05T17:55:14.000-07:00
And fix all files that used it.
diff --git a/lib/packages.ts b/lib/packages.ts
@@ -57,13 +57,14 @@ function _getHashOf(pkg: PackageInfo): string {
     hashCache[pkg.name] = (md5Stream.read() as Buffer).toString('hex');
   }
 
-  if (!hashCache[pkg.name]) {
+  const value = hashCache[pkg.name];
+  if (!value) {
     // Protect against circular dependency.
     throw new Error('Circular dependency detected between the following packages: '
       + Object.keys(hashCache).filter(key => hashCache[key] == null).join(', '));
   }
 
-  return hashCache[pkg.name] !;
+  return value;
 }
 
 
diff --git a/packages/angular_devkit/build_optimizer/src/helpers/transform-javascript.ts b/packages/angular_devkit/build_optimizer/src/helpers/transform-javascript.ts
@@ -83,15 +83,29 @@ export function transformJavascript(
     k, ts.createSourceFile(k, v, ts.ScriptTarget.ES2015)));
 
   const host: ts.CompilerHost = {
-    getSourceFile: (fileName) => sourcesMap.get(fileName)!,
+    getSourceFile: (fileName) => {
+      const sourceFile = sourcesMap.get(fileName);
+      if (!sourceFile) {
+        throw new Error(`File ${fileName} does not have a sourceFile.`);
+      }
+
+      return sourceFile;
+    },
     getDefaultLibFileName: () => defaultLibFileName,
     getCurrentDirectory: () => '',
     getDirectories: () => [],
     getCanonicalFileName: (fileName) => fileName,
     useCaseSensitiveFileNames: () => true,
     getNewLine: () => '\n',
     fileExists: (fileName) => fileMap.has(fileName),
-    readFile: (fileName) => fileMap.has(fileName) ? fileMap.get(fileName)! : '',
+    readFile: (fileName) => {
+      const content = fileMap.get(fileName);
+      if (!content) {
+        throw new Error(`File ${fileName} does not exist.`);
+      }
+
+      return content;
+    },
     writeFile: (fileName, text) => outputs.set(fileName, text),
   };
 
diff --git a/packages/angular_devkit/schematics/bin/schematics.ts b/packages/angular_devkit/schematics/bin/schematics.ts
@@ -68,11 +68,11 @@ function usage(exitCode = 0): never {
 function parseSchematicName(str: string | null): { collection: string, schematic: string } {
   let collection = '@schematics/angular';
 
-  if (!str) {
+  if (!str || str === null) {
     usage(1);
   }
 
-  let schematic: string = str !;
+  let schematic: string = str as string;
   if (schematic.indexOf(':') != -1) {
     [collection, schematic] = schematic.split(':', 2);
 
@@ -112,8 +112,8 @@ const engine = new SchematicEngine(engineHost);
 
 // Add support for schemaJson.
 engineHost.registerOptionsTransform((schematic: FileSystemSchematicDesc, options: {}) => {
-  if (schematic.schema) {
-    const SchemaMetaClass = SchemaClassFactory<{}>(schematic.schemaJson !);
+  if (schematic.schema && schematic.schemaJson) {
+    const SchemaMetaClass = SchemaClassFactory<{}>(schematic.schemaJson);
     const schemaClass = new SchemaMetaClass(options);
 
     return schemaClass.$$root();
diff --git a/packages/angular_devkit/schematics/src/engine/schematic_spec.ts b/packages/angular_devkit/schematics/src/engine/schematic_spec.ts
@@ -5,6 +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
  */
+// tslint:disable:non-null-operator
 import { Observable } from 'rxjs/Observable';
 import 'rxjs/add/operator/toArray';
 import 'rxjs/add/operator/toPromise';
diff --git a/packages/angular_devkit/schematics/src/rules/base_spec.ts b/packages/angular_devkit/schematics/src/rules/base_spec.ts
@@ -5,6 +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
  */
+// tslint:disable:non-null-operator
 import { Observable } from 'rxjs/Observable';
 import 'rxjs/add/operator/toPromise';
 import { Rule, SchematicContext, Source } from '../engine/interface';
diff --git a/packages/angular_devkit/schematics/src/rules/move_spec.ts b/packages/angular_devkit/schematics/src/rules/move_spec.ts
@@ -5,6 +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
  */
+// tslint:disable:non-null-operator
 import { Observable } from 'rxjs/Observable';
 import 'rxjs/add/operator/toPromise';
 import { SchematicContext } from '../engine/interface';
diff --git a/packages/angular_devkit/schematics/src/tree/virtual.ts b/packages/angular_devkit/schematics/src/tree/virtual.ts
@@ -154,7 +154,8 @@ export class VirtualTree implements Tree {
     this.set(new SimpleFileEntry(path, content as Buffer));
   }
   protected _rename(path: SchematicPath, to: SchematicPath, action?: Action, force = false) {
-    if (!this._cacheMap.has(path)) {
+    const entry = this.get(path);
+    if (!entry) {
       throw new FileDoesNotExistException(path);
     }
     if (this._cacheMap.has(to) && !force) {
@@ -167,7 +168,6 @@ export class VirtualTree implements Tree {
       this._actions.rename(path, to);
     }
 
-    const entry = this.get(path) !;
     this.set(new SimpleFileEntry(to, entry.content));
     this._cacheMap.delete(path);
   }
diff --git a/packages/angular_devkit/schematics/src/tree/virtual_spec.ts b/packages/angular_devkit/schematics/src/tree/virtual_spec.ts
@@ -5,6 +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
  */
+// tslint:disable:non-null-operator
 import { FileAlreadyExistException, FileDoesNotExistException } from '../exception/exception';
 import { FileSystemTree } from './filesystem';
 import { FileEntry, MergeStrategy } from './interface';
diff --git a/packages/angular_devkit/schematics/src/utility/update-buffer.ts b/packages/angular_devkit/schematics/src/utility/update-buffer.ts
@@ -210,10 +210,10 @@ export class UpdateBuffer {
     }
 
     if (start == h.end && h.next !== null) {
-      return [h !, h.next !];
+      return [h, h.next];
     }
 
-    return [h !, h.slice(start) !];
+    return [h, h.slice(start)];
   }
 
   get length(): number {
diff --git a/packages/schematics/angular/application/index.ts b/packages/schematics/angular/application/index.ts
@@ -98,7 +98,7 @@ export default function (options: ApplicationOptions): Rule {
             'dot': '.',
             ...options as object,
           }),
-        move(options.directory),
+          move(options.directory),
         ])),
       schematic('module', {
         name: 'app',
@@ -115,7 +115,7 @@ export default function (options: ApplicationOptions): Rule {
         flat: true,
         ...componentOptions,
       }),
-    addBootstrapToNgModule(options.directory),
+      addBootstrapToNgModule(options.directory),
       mergeWith(
         apply(url('./other-files'), [
           componentOptions.inlineTemplate ? filter(path => !path.endsWith('.html')) : noop(),
diff --git a/packages/schematics/angular/class/index.ts b/packages/schematics/angular/class/index.ts
@@ -7,6 +7,7 @@
 */
 import {
   Rule,
+  SchematicsError,
   apply,
   branchAndMerge,
   chain,
@@ -25,14 +26,18 @@ import { Schema as ClassOptions } from './schema';
 export default function (options: ClassOptions): Rule {
   options.type = !!options.type ? `.${options.type}` : '';
   options.path = options.path ? normalizePath(options.path) : options.path;
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
 
   const templateSource = apply(url('./files'), [
     options.spec ? noop() : filter(path => !path.endsWith('.spec.ts')),
     template({
       ...stringUtils,
       ...options as object,
     }),
-    move(options.sourceDir !),
+    move(sourceDir),
   ]);
 
   return chain([
diff --git a/packages/schematics/angular/component/index.ts b/packages/schematics/angular/component/index.ts
@@ -8,6 +8,7 @@
 import {
   Rule,
   SchematicContext,
+  SchematicsError,
   Tree,
   apply,
   branchAndMerge,
@@ -36,8 +37,12 @@ function addDeclarationToNgModule(options: ComponentOptions): Rule {
     }
 
     const modulePath = options.module;
-    let sourceText = host.read(modulePath) !.toString('utf-8');
-    let source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
+    const text = host.read(modulePath);
+    if (text === null) {
+      throw new SchematicsError(`File ${modulePath} does not exist.`);
+    }
+    const sourceText = text.toString('utf-8');
+    const source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
 
     const componentPath = `/${options.sourceDir}/${options.path}/`
                           + (options.flat ? '' : stringUtils.dasherize(options.name) + '/')
@@ -59,8 +64,13 @@ function addDeclarationToNgModule(options: ComponentOptions): Rule {
     host.commitUpdate(declarationRecorder);
 
     if (options.export) {
-      sourceText = host.read(modulePath) !.toString('utf-8');
-      source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
+      // Need to refresh the AST because we overwrote the file in the host.
+      const text = host.read(modulePath);
+      if (text === null) {
+        throw new SchematicsError(`File ${modulePath} does not exist.`);
+      }
+      const sourceText = text.toString('utf-8');
+      const source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
 
       const exportRecorder = host.beginUpdate(modulePath);
       const exportChanges = addExportToModule(source, modulePath,
@@ -92,6 +102,11 @@ function buildSelector(options: ComponentOptions) {
 
 
 export default function(options: ComponentOptions): Rule {
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
+
   return (host: Tree, context: SchematicContext) => {
     options.selector = options.selector || buildSelector(options);
     options.path = options.path ? normalizePath(options.path) : options.path;
@@ -106,7 +121,7 @@ export default function(options: ComponentOptions): Rule {
         'if-flat': (s: string) => options.flat ? '' : s,
         ...options as object,
       }),
-      move(options.sourceDir !),
+      move(sourceDir),
     ]);
 
     return chain([
diff --git a/packages/schematics/angular/directive/index.ts b/packages/schematics/angular/directive/index.ts
@@ -8,6 +8,7 @@
 import {
   Rule,
   SchematicContext,
+  SchematicsError,
   Tree,
   apply,
   branchAndMerge,
@@ -36,8 +37,12 @@ function addDeclarationToNgModule(options: DirectiveOptions): Rule {
     }
 
     const modulePath = options.module;
-    let sourceText = host.read(modulePath) !.toString('utf-8');
-    let source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
+    const text = host.read(modulePath);
+    if (text === null) {
+      throw new SchematicsError(`File ${modulePath} does not exist.`);
+    }
+    const sourceText = text.toString('utf-8');
+    const source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
 
     const directivePath = `/${options.sourceDir}/${options.path}/`
                           + (options.flat ? '' : stringUtils.dasherize(options.name) + '/')
@@ -58,8 +63,13 @@ function addDeclarationToNgModule(options: DirectiveOptions): Rule {
     host.commitUpdate(declarationRecorder);
 
     if (options.export) {
-      sourceText = host.read(modulePath) !.toString('utf-8');
-      source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
+      // Need to refresh the AST because we overwrote the file in the host.
+      const text = host.read(modulePath);
+      if (text === null) {
+        throw new SchematicsError(`File ${modulePath} does not exist.`);
+      }
+      const sourceText = text.toString('utf-8');
+      const source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
 
       const exportRecorder = host.beginUpdate(modulePath);
       const exportChanges = addExportToModule(source, modulePath,
@@ -91,6 +101,10 @@ function buildSelector(options: DirectiveOptions) {
 export default function (options: DirectiveOptions): Rule {
   options.selector = options.selector || buildSelector(options);
   options.path = options.path ? normalizePath(options.path) : options.path;
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
 
   return (host: Tree, context: SchematicContext) => {
     options.module = findModuleFromOptions(host, options);
@@ -101,7 +115,7 @@ export default function (options: DirectiveOptions): Rule {
         'if-flat': (s: string) => options.flat ? '' : s,
         ...options as object,
       }),
-      move(options.sourceDir !),
+      move(sourceDir),
     ]);
 
     return chain([
diff --git a/packages/schematics/angular/enum/index.ts b/packages/schematics/angular/enum/index.ts
@@ -7,6 +7,7 @@
 */
 import {
   Rule,
+  SchematicsError,
   apply,
   branchAndMerge,
   chain,
@@ -22,13 +23,17 @@ import { Schema as EnumOptions } from './schema';
 
 export default function (options: EnumOptions): Rule {
   options.path = options.path ? normalizePath(options.path) : options.path;
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
 
   const templateSource = apply(url('./files'), [
     template({
       ...stringUtils,
       ...options as object,
     }),
-    move(options.sourceDir !),
+    move(sourceDir),
   ]);
 
   return chain([
diff --git a/packages/schematics/angular/guard/index.ts b/packages/schematics/angular/guard/index.ts
@@ -8,6 +8,7 @@
 import {
   Rule,
   SchematicContext,
+  SchematicsError,
   Tree,
   apply,
   branchAndMerge,
@@ -36,7 +37,12 @@ function addDeclarationToNgModule(options: GuardOptions): Rule {
     }
 
     const modulePath = options.module;
-    const sourceText = host.read(modulePath) !.toString('utf-8');
+    const text = host.read(modulePath);
+    if (text === null) {
+      throw new SchematicsError(`File ${modulePath} does not exist.`);
+    }
+    const sourceText = text.toString('utf-8');
+
     const source = ts.createSourceFile(modulePath, sourceText, ts.ScriptTarget.Latest, true);
 
     const guardPath = `/${options.sourceDir}/${options.path}/`
@@ -61,6 +67,10 @@ function addDeclarationToNgModule(options: GuardOptions): Rule {
 
 export default function (options: GuardOptions): Rule {
   options.path = options.path ? normalizePath(options.path) : options.path;
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
 
   return (host: Tree, context: SchematicContext) => {
     if (options.module) {
@@ -73,7 +83,7 @@ export default function (options: GuardOptions): Rule {
         ...stringUtils,
         ...options as object,
       }),
-      move(options.sourceDir !),
+      move(sourceDir),
     ]);
 
     return chain([
diff --git a/packages/schematics/angular/interface/index.ts b/packages/schematics/angular/interface/index.ts
@@ -7,6 +7,7 @@
  */
 import {
   Rule,
+  SchematicsError,
   apply,
   branchAndMerge,
   chain,
@@ -24,13 +25,17 @@ export default function (options: InterfaceOptions): Rule {
   options.prefix = options.prefix ? options.prefix : '';
   options.type = !!options.type ? `.${options.type}` : '';
   options.path = options.path ? normalizePath(options.path) : options.path;
+  const sourceDir = options.sourceDir;
+  if (!sourceDir) {
+    throw new SchematicsError(`sourceDir option is required.`);
+  }
 
   const templateSource = apply(url('./files'), [
     template({
       ...stringUtils,
       ...options as object,
     }),
-    move(options.sourceDir !),
+    move(sourceDir),
   ]);
 
   return chain([
diff --git a/packages/schematics/angular/module/index.ts b/packages/schematics/angular/module/index.ts
diff --git a/packages/schematics/angular/pipe/index.ts b/packages/schematics/angular/pipe/index.ts
diff --git a/packages/schematics/angular/service/index.ts b/packages/schematics/angular/service/index.ts
diff --git a/packages/schematics/angular/utility/ast-utils.ts b/packages/schematics/angular/utility/ast-utils.ts
diff --git a/rules/nonNullOperatorRule.ts b/rules/nonNullOperatorRule.ts
diff --git a/scripts/test.ts b/scripts/test.ts
diff --git a/scripts/validate-commits.ts b/scripts/validate-commits.ts
diff --git a/tslint.json b/tslint.json

Original file line number	Diff line number	Diff line change
`@@ -57,13 +57,14 @@ function _getHashOf(pkg: PackageInfo): string {`
`57`	`57`	`hashCache[pkg.name] = (md5Stream.read() as Buffer).toString('hex');`
`58`	`58`	`}`
`59`	`59`
`60`		`- if (!hashCache[pkg.name]) {`
	`60`	`+ const value = hashCache[pkg.name];`
	`61`	`+ if (!value) {`
`61`	`62`	`// Protect against circular dependency.`
`62`	`63`	`throw new Error('Circular dependency detected between the following packages: '`
`63`	`64`	`+ Object.keys(hashCache).filter(key => hashCache[key] == null).join(', '));`
`64`	`65`	`}`
`65`	`66`
`66`		`- return hashCache[pkg.name] !;`
	`67`	`+ return value;`
`67`	`68`	`}`
`68`	`69`
`69`	`70`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,8 @@ export class VirtualTree implements Tree {`
`154`	`154`	`this.set(new SimpleFileEntry(path, content as Buffer));`
`155`	`155`	`}`
`156`	`156`	`protected _rename(path: SchematicPath, to: SchematicPath, action?: Action, force = false) {`
`157`		`- if (!this._cacheMap.has(path)) {`
	`157`	`+ const entry = this.get(path);`
	`158`	`+ if (!entry) {`
`158`	`159`	`throw new FileDoesNotExistException(path);`
`159`	`160`	`}`
`160`	`161`	`if (this._cacheMap.has(to) && !force) {`
`@@ -167,7 +168,6 @@ export class VirtualTree implements Tree {`
`167`	`168`	`this._actions.rename(path, to);`
`168`	`169`	`}`
`169`	`170`
`170`		`- const entry = this.get(path) !;`
`171`	`171`	`this.set(new SimpleFileEntry(to, entry.content));`
`172`	`172`	`this._cacheMap.delete(path);`
`173`	`173`	`}`
Original file line number	Diff line number	Diff line change
`@@ -210,10 +210,10 @@ export class UpdateBuffer {`
`210`	`210`	`}`
`211`	`211`
`212`	`212`	`if (start == h.end && h.next !== null) {`
`213`		`- return [h !, h.next !];`
	`213`	`+ return [h, h.next];`
`214`	`214`	`}`
`215`	`215`
`216`		`- return [h !, h.slice(start) !];`
	`216`	`+ return [h, h.slice(start)];`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	`get length(): number {`