Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
5 commits merged into from
Feb 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions Gulpfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
}
});
Expand Down Expand Up @@ -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"));
});

Expand Down
16 changes: 7 additions & 9 deletions Jakefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
}));
});
4 changes: 2 additions & 2 deletions scripts/tslint/rules/booleanTriviaRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
98 changes: 98 additions & 0 deletions scripts/tslint/rules/noUnnecessaryTypeAssertion2Rule.ts
Original file line number Diff line number Diff line change
@@ -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()));
}
}
}
1 change: 1 addition & 0 deletions scripts/tslint/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"lib": ["es6"],
"noImplicitAny": true,
"noImplicitReturns": true,
"noImplicitThis": true,
Expand Down
27 changes: 12 additions & 15 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading