refactor: add tslint-sonarts and a lot of tslint rules

This should clean up the code a bit.

Note: at first I added the no-useless-cast rule, but after getting frustrated
with it (as it has many false positive), I decided to remove the rule but some
useless casts were removed so I let those in the PR.

Author: hansl

package-lock.json

@@ -130,6 +130,7 @@
     "tree-kill": "^1.2.0",
     "ts-node": "^5.0.0",
     "tslint": "^5.9.1",
+    "tslint-sonarts": "^1.7.0",
     "tsutils": "~2.22.2",
     "typescript": "~2.7.2",
     "uglifyjs-webpack-plugin": "^1.2.5",

package.json

@@ -22,8 +22,6 @@ export default class VersionCommand extends Command {
   public readonly options: Option[] = [];
 
   public run() {
-    let angularCoreVersion = '';
-    const angularSameAsCore: string[] = [];
     const pkg = require(path.resolve(__dirname, '..', 'package.json'));
     let projPkg;
     try {
@@ -95,6 +93,8 @@ export default class VersionCommand extends Command {
 
       ngCliVersion = `local (v${pkg.version}, branch: ${gitBranch})`;
     }
+    let angularCoreVersion = '';
+    const angularSameAsCore: string[] = [];
 
     if (projPkg) {
       // Filter all angular versions that are the same as core.
@@ -107,6 +107,9 @@ export default class VersionCommand extends Command {
             delete versions[angularPackage];
           }
         }
+
+        // Make sure we list them in alphabetical order.
+        angularSameAsCore.sort();
       }
     }
 
@@ -128,7 +131,7 @@ export default class VersionCommand extends Command {
       Node: ${process.versions.node}
       OS: ${process.platform} ${process.arch}
       Angular: ${angularCoreVersion}
-      ... ${angularSameAsCore.sort().reduce<string[]>((acc, name) => {
+      ... ${angularSameAsCore.reduce<string[]>((acc, name) => {
         // Perform a simple word wrap around 60.
         if (acc.length == 0) {
           return [name];

packages/angular/cli/commands/version.ts

@@ -15,7 +15,7 @@ export type NpmInstall = (packageName: string,
                           logger: logging.Logger,
                           packageManager: string,
                           projectRoot: string,
-                          save?: boolean) => void;
+                          save?: boolean) => Promise<void>;
 
 export default async function (packageName: string,
                                logger: logging.Logger,

packages/angular/cli/tasks/npm-install.ts

@@ -119,7 +119,7 @@ export function getWorkspaceRaw(
     throw new Error('Invalid JSON');
   }
 
-  return [ast as JsonAstObject, configPath];
+  return [ast, configPath];
 }
 
 export function validateWorkspace(json: JsonObject) {

packages/angular/cli/utilities/config.ts

@@ -34,17 +34,15 @@ export class TestProjectHost extends NodeJsSyncHost {
       throw new Error('TestProjectHost must be initialized before being used.');
     }
 
-    // tslint:disable-next-line:non-null-operator
-    return this._currentRoot!;
+    return this._currentRoot;
   }
 
   scopedSync(): virtualFs.SyncDelegateHost<Stats> {
     if (this._currentRoot === null || this._scopedSyncHost === null) {
       throw new Error('TestProjectHost must be initialized before being used.');
     }
 
-    // tslint:disable-next-line:non-null-operator
-    return this._scopedSyncHost!;
+    return this._scopedSyncHost;
   }
 
   initialize(): Observable<void> {
@@ -100,7 +98,7 @@ export class TestProjectHost extends NodeJsSyncHost {
   }
 
   writeMultipleFiles(files: { [path: string]: string | ArrayBufferLike | Buffer }): void {
-    Object.keys(files).map(fileName => {
+    Object.keys(files).forEach(fileName => {
       let content = files[fileName];
       if (typeof content == 'string') {
         content = virtualFs.stringToFileBuffer(content);

packages/angular_devkit/architect/testing/test-project-host.ts

@@ -290,13 +290,13 @@ export class DevServerBuilder implements Builder<DevServerBuilderOptions> {
     let sslKey: string | undefined = undefined;
     let sslCert: string | undefined = undefined;
     if (options.sslKey) {
-      const keyPath = path.resolve(root, options.sslKey as string);
+      const keyPath = path.resolve(root, options.sslKey);
       if (existsSync(keyPath)) {
         sslKey = readFileSync(keyPath, 'utf-8');
       }
     }
     if (options.sslCert) {
-      const certPath = path.resolve(root, options.sslCert as string);
+      const certPath = path.resolve(root, options.sslCert);
       if (existsSync(certPath)) {
         sslCert = readFileSync(certPath, 'utf-8');
       }

packages/angular_devkit/build_angular/src/dev-server/index.ts

@@ -83,7 +83,7 @@ export class KarmaBuilder implements Builder<KarmaBuilderSchema> {
 
         // TODO: inside the configs, always use the project root and not the workspace root.
         // Until then we pretend the app root is relative (``) but the same as `projectRoot`.
-        (karmaOptions.buildWebpack.options as any).root = ''; // tslint:disable-line:no-any
+        karmaOptions.buildWebpack.options.root = '';
 
         // Assign additional karmaConfig options to the local ngapp config
         karmaOptions.configFile = karmaConfig;
@@ -112,7 +112,7 @@ export class KarmaBuilder implements Builder<KarmaBuilderSchema> {
   ) {
     let wco: WebpackConfigOptions;
 
-    const tsConfigPath = getSystemPath(resolve(root, normalize(options.tsConfig as string)));
+    const tsConfigPath = getSystemPath(resolve(root, normalize(options.tsConfig)));
     const tsConfig = readTsconfig(tsConfigPath);
 
     const projectTs = requireProjectModule(getSystemPath(projectRoot), 'typescript') as typeof ts;

packages/angular_devkit/build_angular/src/karma/index.ts

@@ -106,8 +106,8 @@ export class ServerBuilder implements Builder<BuildWebpackServerSchema> {
 
     if (wco.buildOptions.main || wco.buildOptions.polyfills) {
       const typescriptConfigPartial = wco.buildOptions.aot
-        ? getAotConfig(wco, host as virtualFs.Host<Stats>)
-        : getNonAotConfig(wco, host as virtualFs.Host<Stats>);
+        ? getAotConfig(wco, host)
+        : getNonAotConfig(wco, host);
       webpackConfigs.push(typescriptConfigPartial);
     }
 

packages/angular_devkit/build_angular/src/server/index.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:no-big-function
 
 import { DefaultTimeout, TestLogger, runTargetSpec } from '@angular-devkit/architect/testing';
 import { join, normalize, virtualFs } from '@angular-devkit/core';

packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.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:no-big-function
 
 import { DefaultTimeout, runTargetSpec } from '@angular-devkit/architect/testing';
 import { normalize, tags, virtualFs } from '@angular-devkit/core';

packages/angular_devkit/build_angular/test/browser/styles_spec_large.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:no-big-function
 // tslint:disable-next-line:no-implicit-dependencies
 import { tags } from '@angular-devkit/core';
 import { RawSourceMap } from 'source-map';

packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer_spec.ts

@@ -22,20 +22,22 @@ export function collectDeepNodes<T extends ts.Node>(node: ts.Node, kind: ts.Synt
 }
 
 export function drilldownNodes<T extends ts.Node>(
-  startingNode: ts.Node,
-  path: { prop: string | null, kind: ts.SyntaxKind }[],
+  startingNode: T,
+  path: { prop: keyof T, kind: ts.SyntaxKind }[],
 ): T | null {
-  let currentNode: T | ts.Node | undefined = startingNode;
+  let currentNode: T = startingNode;
   for (const segment of path) {
     if (segment.prop) {
       // ts.Node has no index signature, so we need to cast it as any.
+      const tempNode = currentNode[segment.prop];
+      if (!tempNode || typeof tempNode != 'object' || currentNode.kind !== segment.kind) {
+        return null;
+      }
+
       // tslint:disable-next-line:no-any
-      currentNode = (currentNode as any)[segment.prop];
-    }
-    if (!currentNode || currentNode.kind !== segment.kind) {
-      return null;
+      currentNode = tempNode as any as T;
     }
   }
 
-  return currentNode as T;
+  return currentNode;
 }

packages/angular_devkit/build_optimizer/src/helpers/ast-utils.ts

@@ -38,7 +38,7 @@ export function getFoldFileTransformer(program: ts.Program): ts.TransformerFacto
         // Check if node is a class to add statements to.
         const clazz = classes.find((cl) => cl.classFunction === node);
         if (clazz) {
-          const functionExpression = node as ts.FunctionExpression;
+          const functionExpression: ts.FunctionExpression = node as ts.FunctionExpression;
 
           const newExpressions = clazz.statements.map((st) =>
             ts.createStatement(st.expressionStatement.expression));

packages/angular_devkit/build_optimizer/src/transforms/class-fold.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:no-big-function
 // tslint:disable-next-line:no-implicit-dependencies
 import { tags } from '@angular-devkit/core';
 import { transformJavascript } from '../helpers/transform-javascript';

packages/angular_devkit/build_optimizer/src/transforms/prefix-classes_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:no-big-function
 // tslint:disable-next-line:no-implicit-dependencies
 import { tags } from '@angular-devkit/core';
 import { transformJavascript } from '../helpers/transform-javascript';

packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts

@@ -542,13 +542,13 @@ function _readBlanks(context: JsonParserContext): (JsonAstComment | JsonAstMulti
   if ((context.mode & JsonParseMode.CommentsAllowed) != 0) {
     const comments: (JsonAstComment | JsonAstMultilineComment)[] = [];
     while (true) {
-      let char = context.original[context.position.offset];
+      const char = context.original[context.position.offset];
       if (char == '/' && context.original[context.position.offset + 1] == '*') {
         const start = context.position;
         // Multi line comment.
         _next(context);
         _next(context);
-        char = context.original[context.position.offset];
+
         while (context.original[context.position.offset] != '*'
             || context.original[context.position.offset + 1] != '/') {
           _next(context);
@@ -572,7 +572,7 @@ function _readBlanks(context: JsonParserContext): (JsonAstComment | JsonAstMulti
         // Multi line comment.
         _next(context);
         _next(context);
-        char = context.original[context.position.offset];
+
         while (context.original[context.position.offset] != '\n') {
           _next(context);
           if (context.position.offset >= context.original.length) {

packages/angular_devkit/core/src/json/parser.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:no-big-function
 import { JsonParseMode, parseJson, parseJsonAst } from './parser';
 
 

packages/angular_devkit/core/src/json/parser_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
  */
-// tslint:disable:no-any
-// tslint:disable:non-null-operator
+// tslint:disable:no-any non-null-operator no-big-function
 import { of as observableOf } from 'rxjs';
 import { map, mergeMap } from 'rxjs/operators';
 import { CoreSchemaRegistry } from './registry';

packages/angular_devkit/core/src/json/schema/registry_spec.ts

@@ -138,7 +138,7 @@ export function templateParser(sourceText: string, fileName: string): TemplateAs
   // Optimization that uses the fact that the end of a node is always the beginning of the next
   // node, so we keep the positioning of the nodes in memory.
   let start = _positionFor(sourceText, offset);
-  let end = null as Position | null;
+  let end: Position | null;
 
   const increment = reExpressions.length + 1;
   for (let i = 0; i < parsed.length; i += increment) {

packages/angular_devkit/core/src/utils/template.ts

@@ -179,7 +179,7 @@ export class SimpleMemoryHost implements Host<{}> {
   protected _delete(path: Path): void {
     path = this._toAbsolute(path);
     if (this._isDirectory(path)) {
-      for (const [cachePath, _] of this._cache.entries()) {
+      for (const [cachePath] of this._cache.entries()) {
         if (path.startsWith(cachePath + NormalizedSep)) {
           this._cache.delete(cachePath);
         }

packages/angular_devkit/core/src/virtual-fs/host/memory.ts

@@ -5,7 +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:no-any
+// tslint:disable:no-any no-big-function
 import { path } from '../path';
 import { fileBuffer } from './buffer';
 import { CordHost } from './record';

packages/angular_devkit/core/src/virtual-fs/host/record_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:no-big-function
 
 import { tap } from 'rxjs/operators';
 import { schema } from '..';

packages/angular_devkit/core/src/workspace/workspace_spec.ts

@@ -94,7 +94,7 @@ export class DryRunSink extends HostSink {
 
     this._filesToDelete.forEach(path => {
       // Check if this is a renaming.
-      for (const [from, _] of this._filesToRename) {
+      for (const [from] of this._filesToRename) {
         if (from == path) {
           // The event is sent later on.
           return;
@@ -108,7 +108,7 @@ export class DryRunSink extends HostSink {
     });
     this._filesToCreate.forEach((content, path) => {
       // Check if this is a renaming.
-      for (const [_, to] of this._filesToRename) {
+      for (const [, to] of this._filesToRename) {
         if (to == path) {
           // The event is sent later on.
           return;

packages/angular_devkit/schematics/src/sink/dryrun.ts

@@ -33,7 +33,8 @@ describe('DryRunSink', () => {
     const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test'];
     const treeFiles: Path[] = [];
     tree.visit(path => treeFiles.push(path));
-    expect(treeFiles.sort()).toEqual(files.map(normalize));
+    treeFiles.sort();
+    expect(treeFiles).toEqual(files.map(normalize));
 
     const sink = new DryRunSink(new virtualFs.SimpleMemoryHost());
     sink.reporter.pipe(toArray())
@@ -62,7 +63,8 @@ describe('DryRunSink', () => {
     const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test'];
     const treeFiles: Path[] = [];
     tree.visit(path => treeFiles.push(path));
-    expect(treeFiles.sort()).toEqual(files.map(normalize));
+    treeFiles.sort();
+    expect(treeFiles).toEqual(files.map(normalize));
 
     // Need to create this file on the filesystem, otherwise the commit phase will fail.
     const outputHost = new virtualFs.SimpleMemoryHost();

packages/angular_devkit/schematics/src/sink/dryrun_spec.ts

@@ -30,7 +30,8 @@ describe('FileSystemSink', () => {
     const files = ['/hello', '/sub/directory/file2', '/sub/file1', '/test'];
     const treeFiles: string[] = [];
     tree.visit(path => treeFiles.push(path));
-    expect(treeFiles.sort()).toEqual(files);
+    treeFiles.sort();
+    expect(treeFiles).toEqual(files);
 
     const outputHost = new virtualFs.test.TestHost();
     const sink = new HostSink(outputHost);

packages/angular_devkit/schematics/src/sink/host_spec.ts

@@ -47,7 +47,8 @@ describe('FilterHostTree', () => {
     const filtered = new FilterHostTree(tree, p => p != '/file2');
     const filteredFiles: string[] = [];
     filtered.visit(path => filteredFiles.push(path));
-    expect(filteredFiles.sort()).toEqual(['/file1', '/file3'].map(normalize));
+    filteredFiles.sort();
+    expect(filteredFiles).toEqual(['/file1', '/file3'].map(normalize));
     expect(filtered.actions.length).toEqual(2);
   });
 
@@ -61,7 +62,8 @@ describe('FilterHostTree', () => {
     const filtered2 = new FilterHostTree(filtered, p => p != '/file3');
     const filteredFiles: string[] = [];
     filtered2.visit(path => filteredFiles.push(path));
-    expect(filteredFiles.sort()).toEqual(['/file1'].map(normalize));
+    filteredFiles.sort();
+    expect(filteredFiles).toEqual(['/file1'].map(normalize));
     expect(filtered2.actions.map(a => a.kind)).toEqual(['c']);
   });
 
@@ -77,7 +79,8 @@ describe('FilterHostTree', () => {
     const filtered2 = new FilterHostTree(filtered, p => p != '/file3');
     const filteredFiles: string[] = [];
     filtered2.visit(path => filteredFiles.push(path));
-    expect(filteredFiles.sort()).toEqual(['/file1'].map(normalize));
+    filteredFiles.sort();
+    expect(filteredFiles).toEqual(['/file1'].map(normalize));
     expect(filtered2.actions.map(a => a.kind)).toEqual([]);
   });
 
@@ -91,7 +94,8 @@ describe('FilterHostTree', () => {
     const filtered2 = new FilterHostTree(filtered, p => p != '/file3');
     const filteredFiles: string[] = [];
     filtered2.visit(path => filteredFiles.push(path));
-    expect(filteredFiles.sort()).toEqual(['/dir1/file1'].map(normalize));
+    filteredFiles.sort();
+    expect(filteredFiles).toEqual(['/dir1/file1'].map(normalize));
     expect(filtered2.actions.map(a => a.kind)).toEqual(['c']);
   });
 
@@ -107,7 +111,8 @@ describe('FilterHostTree', () => {
     const filtered2 = new FilterHostTree(filtered, p => p != '/file3');
     const filteredFiles: string[] = [];
     filtered2.visit(path => filteredFiles.push(path));
-    expect(filteredFiles.sort()).toEqual(['/dir1/file1'].map(normalize));
+    filteredFiles.sort();
+    expect(filteredFiles).toEqual(['/dir1/file1'].map(normalize));
     expect(filtered2.actions.map(a => a.kind)).toEqual([]);
   });
 });