Apply 'no-unnecessary-type-assertion' lint rule (#22005)

* Apply 'no-unnecessary-type-assertion' lint rule

* Fix type error

* Fix tsconfig.json

* Add --format back

Author: Andy

Gulpfile.ts

@@ -53,7 +53,6 @@ const cmdLineOptions = minimist(process.argv.slice(2), {
         "ru": "runners", "runner": "runners",
         "r": "reporter",
         "c": "colors", "color": "colors",
-        "f": "files", "file": "files",
         "w": "workers",
     },
     default: {
@@ -69,7 +68,6 @@ const cmdLineOptions = minimist(process.argv.slice(2), {
         light: process.env.light === undefined || process.env.light !== "false",
         reporter: process.env.reporter || process.env.r,
         lint: process.env.lint || true,
-        files: process.env.f || process.env.file || process.env.files || "",
         workers: process.env.workerCount || os.cpus().length,
     }
 });
@@ -1112,13 +1110,11 @@ function spawnLintWorker(files: {path: string}[], callback: (failures: number) =
 
 gulp.task("lint", "Runs tslint on the compiler sources. Optional arguments are: --f[iles]=regex", ["build-rules"], () => {
     if (fold.isTravis()) console.log(fold.start("lint"));
-    const fileMatcher = cmdLineOptions.files;
-    const files = fileMatcher
-        ? `src/**/${fileMatcher}`
-        : `Gulpfile.ts "scripts/generateLocalizedDiagnosticMessages.ts" "scripts/tslint/**/*.ts" "src/**/*.ts" --exclude "src/lib/*.d.ts"`;
-    const cmd = `node node_modules/tslint/bin/tslint ${files} --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish`;
-    console.log("Linting: " + cmd);
-    child_process.execSync(cmd, { stdio: [0, 1, 2] });
+    for (const project of ["scripts/tslint/tsconfig.json", "src/tsconfig-base.json"]) {
+        const cmd = `node node_modules/tslint/bin/tslint --project ${project} --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish`;
+        console.log("Linting: " + cmd);
+        child_process.execSync(cmd, { stdio: [0, 1, 2] });
+    }
     if (fold.isTravis()) console.log(fold.end("lint"));
 });
 

Jakefile.js

@@ -1302,15 +1302,13 @@ function spawnLintWorker(files, callback) {
 desc("Runs tslint on the compiler sources. Optional arguments are: f[iles]=regex");
 task("lint", ["build-rules"], () => {
     if (fold.isTravis()) console.log(fold.start("lint"));
-    const fileMatcher = process.env.f || process.env.file || process.env.files;
-    
-    const files = fileMatcher
-        ? `src/**/${fileMatcher}`
-        : `Gulpfile.ts scripts/generateLocalizedDiagnosticMessages.ts "scripts/tslint/**/*.ts" "src/**/*.ts" --exclude "src/lib/*.d.ts"`;
-    const cmd = `node node_modules/tslint/bin/tslint ${files} --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish`;
-    console.log("Linting: " + cmd);
-    jake.exec([cmd], { interactive: true, windowsVerbatimArguments: true }, () => {
+   function lint(project, cb) {
+        const cmd = `node node_modules/tslint/bin/tslint --project ${project} --formatters-dir ./built/local/tslint/formatters --format autolinkableStylish`;
+        console.log("Linting: " + cmd);
+        jake.exec([cmd], { interactive: true, windowsVerbatimArguments: true }, cb);
+   }
+   lint("scripts/tslint/tsconfig.json", () => lint("src/tsconfig-base.json", () => {
         if (fold.isTravis()) console.log(fold.end("lint"));
         complete();
-    });
+   }));
 });

scripts/tslint/rules/booleanTriviaRule.ts

@@ -27,7 +27,7 @@ function walk(ctx: Lint.WalkContext<void>): void {
     /** Skip certain function/method names whose parameter names are not informative. */
     function shouldIgnoreCalledExpression(expression: ts.Expression): boolean {
         if (expression.kind === ts.SyntaxKind.PropertyAccessExpression) {
-            const methodName = (expression as ts.PropertyAccessExpression).name.text as string;
+            const methodName = (expression as ts.PropertyAccessExpression).name.text;
             if (methodName.indexOf("set") === 0) {
                 return true;
             }
@@ -45,7 +45,7 @@ function walk(ctx: Lint.WalkContext<void>): void {
             }
         }
         else if (expression.kind === ts.SyntaxKind.Identifier) {
-            const functionName = (expression as ts.Identifier).text as string;
+            const functionName = (expression as ts.Identifier).text;
             if (functionName.indexOf("set") === 0) {
                 return true;
             }

scripts/tslint/rules/noUnnecessaryTypeAssertion2Rule.ts

@@ -0,0 +1,98 @@
+/**
+ * @license
+ * Copyright 2016 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as ts from "typescript";
+import * as Lint from "tslint";
+
+export class Rule extends Lint.Rules.TypedRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "no-unnecessary-type-assertion",
+        description: "Warns if a type assertion does not change the type of an expression.",
+        options: {
+            type: "list",
+            listType: {
+                type: "array",
+                items: { type: "string" },
+            },
+        },
+        optionsDescription: "A list of whitelisted assertion types to ignore",
+        type: "typescript",
+        hasFix: true,
+        typescriptOnly: true,
+        requiresTypeInfo: true,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING = "This assertion is unnecessary since it does not change the type of the expression.";
+
+    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+        return this.applyWithWalker(new Walker(sourceFile, this.ruleName, this.ruleArguments, program.getTypeChecker()));
+    }
+}
+
+class Walker extends Lint.AbstractWalker<string[]> {
+    constructor(sourceFile: ts.SourceFile, ruleName: string, options: string[], private readonly checker: ts.TypeChecker) {
+        super(sourceFile, ruleName, options);
+    }
+
+    public walk(sourceFile: ts.SourceFile) {
+        const cb = (node: ts.Node): void => {
+            switch (node.kind) {
+                case ts.SyntaxKind.TypeAssertionExpression:
+                case ts.SyntaxKind.AsExpression:
+                    this.verifyCast(node as ts.TypeAssertion | ts.AsExpression);
+            }
+
+            return ts.forEachChild(node, cb);
+        };
+
+        return ts.forEachChild(sourceFile, cb);
+    }
+
+    private verifyCast(node: ts.TypeAssertion | ts.NonNullExpression | ts.AsExpression) {
+        if (ts.isAssertionExpression(node) && this.options.indexOf(node.type.getText(this.sourceFile)) !== -1) {
+            return;
+        }
+        const castType = this.checker.getTypeAtLocation(node);
+        if (castType === undefined) {
+            return;
+        }
+
+        if (node.kind !== ts.SyntaxKind.NonNullExpression &&
+            (castType.flags & ts.TypeFlags.Literal ||
+                castType.flags & ts.TypeFlags.Object &&
+                (castType as ts.ObjectType).objectFlags & ts.ObjectFlags.Tuple) ||
+            // Sometimes tuple types don't have ObjectFlags.Tuple set, like when
+            // they're being matched against an inferred type. So, in addition,
+            // check if any properties are numbers, which implies that this is
+            // likely a tuple type.
+            (castType.getProperties().some((symbol) => !isNaN(Number(symbol.name))))) {
+
+            // It's not always safe to remove a cast to a literal type or tuple
+            // type, as those types are sometimes widened without the cast.
+            return;
+        }
+
+        const uncastType = this.checker.getTypeAtLocation(node.expression);
+        if (uncastType === castType) {
+            this.addFailureAtNode(node, Rule.FAILURE_STRING, node.kind === ts.SyntaxKind.TypeAssertionExpression
+                ? Lint.Replacement.deleteFromTo(node.getStart(), node.expression.getStart())
+                : Lint.Replacement.deleteFromTo(node.expression.getEnd(), node.getEnd()));
+        }
+    }
+}

scripts/tslint/tsconfig.json

@@ -1,5 +1,6 @@
 {
 	"compilerOptions": {
+		"lib": ["es6"],
 		"noImplicitAny": true,
 		"noImplicitReturns": true,
 		"noImplicitThis": true,

src/compiler/binder.ts

@@ -264,7 +264,7 @@ namespace ts {
                     return (isGlobalScopeAugmentation(<ModuleDeclaration>node) ? "__global" : `"${moduleName}"`) as __String;
                 }
                 if (name.kind === SyntaxKind.ComputedPropertyName) {
-                    const nameExpression = (<ComputedPropertyName>name).expression;
+                    const nameExpression = name.expression;
                     // treat computed property names where expression is string/numeric literal as just string/numeric literal
                     if (isStringOrNumericLiteral(nameExpression)) {
                         return escapeLeadingUnderscores(nameExpression.text);
@@ -459,10 +459,7 @@ namespace ts {
                 //       and this case is specially handled. Module augmentations should only be merged with original module definition
                 //       and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
                 if (node.kind === SyntaxKind.JSDocTypedefTag) Debug.assert(isInJavaScriptFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
-                const isJSDocTypedefInJSDocNamespace = node.kind === SyntaxKind.JSDocTypedefTag &&
-                    (node as JSDocTypedefTag).name &&
-                    (node as JSDocTypedefTag).name.kind === SyntaxKind.Identifier &&
-                    ((node as JSDocTypedefTag).name as Identifier).isInJSDocNamespace;
+                const isJSDocTypedefInJSDocNamespace = isJSDocTypedefTag(node) && node.name && node.name.kind === SyntaxKind.Identifier && node.name.isInJSDocNamespace;
                 if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefInJSDocNamespace) {
                     const exportKind = symbolFlags & SymbolFlags.Value ? SymbolFlags.ExportValue : 0;
                     const local = declareSymbol(container.locals, /*parent*/ undefined, node, exportKind, symbolExcludes);
@@ -527,7 +524,7 @@ namespace ts {
                 if (!isIIFE) {
                     currentFlow = { flags: FlowFlags.Start };
                     if (containerFlags & (ContainerFlags.IsFunctionExpression | ContainerFlags.IsObjectLiteralOrClassExpressionMethod)) {
-                        (<FlowStart>currentFlow).container = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
+                        currentFlow.container = <FunctionExpression | ArrowFunction | MethodDeclaration>node;
                     }
                 }
                 // We create a return control flow graph for IIFEs and constructors. For constructors
@@ -997,7 +994,7 @@ namespace ts {
             addAntecedent(postLoopLabel, currentFlow);
             bind(node.initializer);
             if (node.initializer.kind !== SyntaxKind.VariableDeclarationList) {
-                bindAssignmentTargetFlow(<Expression>node.initializer);
+                bindAssignmentTargetFlow(node.initializer);
             }
             bindIterativeStatement(node.statement, postLoopLabel, preLoopLabel);
             addAntecedent(preLoopLabel, currentFlow);
@@ -1170,7 +1167,7 @@ namespace ts {
                     i++;
                 }
                 const preCaseLabel = createBranchLabel();
-                addAntecedent(preCaseLabel, createFlowSwitchClause(preSwitchCaseFlow, <SwitchStatement>node.parent, clauseStart, i + 1));
+                addAntecedent(preCaseLabel, createFlowSwitchClause(preSwitchCaseFlow, node.parent, clauseStart, i + 1));
                 addAntecedent(preCaseLabel, fallthroughFlow);
                 currentFlow = finishFlowLabel(preCaseLabel);
                 const clause = clauses[i];
@@ -1251,13 +1248,13 @@ namespace ts {
             else if (node.kind === SyntaxKind.ObjectLiteralExpression) {
                 for (const p of (<ObjectLiteralExpression>node).properties) {
                     if (p.kind === SyntaxKind.PropertyAssignment) {
-                        bindDestructuringTargetFlow((<PropertyAssignment>p).initializer);
+                        bindDestructuringTargetFlow(p.initializer);
                     }
                     else if (p.kind === SyntaxKind.ShorthandPropertyAssignment) {
-                        bindAssignmentTargetFlow((<ShorthandPropertyAssignment>p).name);
+                        bindAssignmentTargetFlow(p.name);
                     }
                     else if (p.kind === SyntaxKind.SpreadAssignment) {
-                        bindAssignmentTargetFlow((<SpreadAssignment>p).expression);
+                        bindAssignmentTargetFlow(p.expression);
                     }
                 }
             }
@@ -1572,7 +1569,7 @@ namespace ts {
         }
 
         function hasExportDeclarations(node: ModuleDeclaration | SourceFile): boolean {
-            const body = node.kind === SyntaxKind.SourceFile ? node : (<ModuleDeclaration>node).body;
+            const body = node.kind === SyntaxKind.SourceFile ? node : node.body;
             if (body && (body.kind === SyntaxKind.SourceFile || body.kind === SyntaxKind.ModuleBlock)) {
                 for (const stat of (<BlockLike>body).statements) {
                     if (stat.kind === SyntaxKind.ExportDeclaration || stat.kind === SyntaxKind.ExportAssignment) {
@@ -2210,7 +2207,7 @@ namespace ts {
         function checkTypePredicate(node: TypePredicateNode) {
             const { parameterName, type } = node;
             if (parameterName && parameterName.kind === SyntaxKind.Identifier) {
-                checkStrictModeIdentifier(parameterName as Identifier);
+                checkStrictModeIdentifier(parameterName);
             }
             if (parameterName && parameterName.kind === SyntaxKind.ThisType) {
                 seenThisKeyword = true;
@@ -2555,13 +2552,13 @@ namespace ts {
                 }
             }
 
-            checkStrictModeFunctionName(<FunctionDeclaration>node);
+            checkStrictModeFunctionName(node);
             if (inStrictMode) {
                 checkStrictModeFunctionDeclaration(node);
                 bindBlockScopedDeclaration(node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
             }
             else {
-                declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
+                declareSymbolAndAddToSymbolTable(node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
             }
         }
 

src/compiler/builder.ts

@@ -228,7 +228,7 @@ namespace ts {
             host = oldProgramOrHost as CompilerHost;
         }
         else {
-            newProgram = newProgramOrRootNames as Program;
+            newProgram = newProgramOrRootNames;
             host = hostOrOptions as BuilderProgramHost;
             oldProgram = oldProgramOrHost as BuilderProgram;
         }