Skip to content
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

Add tslint rules for #3994 #4458

Merged
merged 6 commits into from Sep 18, 2015
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 21 additions & 2 deletions Jakefile.js
Expand Up @@ -770,17 +770,36 @@ task("update-sublime", ["local", serverFile], function() {
jake.cpR(serverFile + ".map", "../TypeScript-Sublime-Plugin/tsserver/");
});

var tslintRuleDir = "scripts/tslint";
var tslintRules = ([
"nextLineRule",
"noInferrableTypesRule",
"noNullRule",
"booleanTriviaRule"
]);
var tslintRulesFiles = tslintRules.map(function(p) {
return path.join(tslintRuleDir, p + ".ts");
});
var tslintRulesOutFiles = tslintRules.map(function(p) {
return path.join(builtLocalDirectory, "tslint", p + ".js");
});
desc("Compiles tslint rules to js");
task("build-rules", tslintRulesOutFiles);
tslintRulesFiles.forEach(function(ruleFile, i) {
compileFile(tslintRulesOutFiles[i], [ruleFile], [ruleFile], [], /*useBuiltCompiler*/ true, /*noOutFile*/ true, /*generateDeclarations*/ false, path.join(builtLocalDirectory, "tslint"));
});

// if the codebase were free of linter errors we could make jake runtests
// run this task automatically
desc("Runs tslint on the compiler sources");
task("lint", [], function() {
task("lint", ["build-rules"], function() {
function success(f) { return function() { console.log('SUCCESS: No linter errors in ' + f + '\n'); }};
function failure(f) { return function() { console.log('FAILURE: Please fix linting errors in ' + f + '\n') }};

var lintTargets = compilerSources.concat(harnessCoreSources);
for (var i in lintTargets) {
var f = lintTargets[i];
var cmd = 'tslint -c tslint.json ' + f;
var cmd = 'tslint --rules-dir built/local/tslint -c tslint.json ' + f;
exec(cmd, success(f), failure(f));
}
}, { async: true });
50 changes: 50 additions & 0 deletions scripts/tslint/booleanTriviaRule.ts
@@ -0,0 +1,50 @@
/// <reference path="../../node_modules/tslint/typings/typescriptServices.d.ts" />
/// <reference path="../../node_modules/tslint/lib/tslint.d.ts" />


export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING_FACTORY = (name: string, currently: string) => `Tag boolean argument as '${name}' (currently '${currently}')`;

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const program = ts.createProgram([sourceFile.fileName], Lint.createCompilerOptions());
const checker = program.getTypeChecker();
return this.applyWithWalker(new BooleanTriviaWalker(checker, program.getSourceFile(sourceFile.fileName), this.getOptions()));
}
}

class BooleanTriviaWalker extends Lint.RuleWalker {
constructor(private checker: ts.TypeChecker, file: ts.SourceFile, opts: Lint.IOptions) {
super(file, opts);
}

visitCallExpression(node: ts.CallExpression) {
super.visitCallExpression(node);
if (node.arguments) {
const targetCallSignature = this.checker.getResolvedSignature(node);
if (!!targetCallSignature) {
const targetParameters = targetCallSignature.getParameters();
const source = this.getSourceFile();
for (let index = 0; index < targetParameters.length; index++) {
const param = targetParameters[index];
const arg = node.arguments[index];
if (!(arg && param)) continue;

const argType = this.checker.getContextualType(arg);
if (argType && (argType.getFlags() & ts.TypeFlags.Boolean)) {
if (arg.kind !== ts.SyntaxKind.TrueKeyword && arg.kind !== ts.SyntaxKind.FalseKeyword) {
continue;
}
let triviaContent: string;
const ranges = ts.getLeadingCommentRanges(arg.getFullText(), 0);
if (ranges && ranges.length === 1 && ranges[0].kind === ts.SyntaxKind.MultiLineCommentTrivia) {
triviaContent = arg.getFullText().slice(ranges[0].pos + 2, ranges[0].end - 2); //+/-2 to remove /**/
}
if (triviaContent !== param.getName()) {
this.addFailure(this.createFailure(arg.getStart(source), arg.getWidth(source), Rule.FAILURE_STRING_FACTORY(param.getName(), triviaContent)));
}
}
}
}
}
}
}
63 changes: 63 additions & 0 deletions scripts/tslint/nextLineRule.ts
@@ -0,0 +1,63 @@
/// <reference path="../../node_modules/tslint/typings/typescriptServices.d.ts" />
/// <reference path="../../node_modules/tslint/lib/tslint.d.ts" />

const OPTION_CATCH = "check-catch";
const OPTION_ELSE = "check-else";

export class Rule extends Lint.Rules.AbstractRule {
public static CATCH_FAILURE_STRING = "'catch' should be on the line following the previous block's ending curly brace";
public static ELSE_FAILURE_STRING = "'else' should be on the line following the previous block's ending curly brace";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new NextLineWalker(sourceFile, this.getOptions()));
}
}

class NextLineWalker extends Lint.RuleWalker {
public visitIfStatement(node: ts.IfStatement) {
const sourceFile = node.getSourceFile();
const thenStatement = node.thenStatement;

const elseStatement = node.elseStatement;
if (!!elseStatement) {
// find the else keyword
const elseKeyword = getFirstChildOfKind(node, ts.SyntaxKind.ElseKeyword);
if (this.hasOption(OPTION_ELSE) && !!elseKeyword) {
const thenStatementEndLoc = sourceFile.getLineAndCharacterOfPosition(thenStatement.getEnd());
const elseKeywordLoc = sourceFile.getLineAndCharacterOfPosition(elseKeyword.getStart());
if (thenStatementEndLoc.line !== (elseKeywordLoc.line - 1)) {
const failure = this.createFailure(elseKeyword.getStart(), elseKeyword.getWidth(), Rule.ELSE_FAILURE_STRING);
this.addFailure(failure);
}
}
}

super.visitIfStatement(node);
}

public visitTryStatement(node: ts.TryStatement) {
const sourceFile = node.getSourceFile();
const catchClause = node.catchClause;

// "visit" try block
const tryKeyword = node.getChildAt(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a getFirstToken()

const tryBlock = node.tryBlock;
const tryOpeningBrace = tryBlock.getChildAt(0);

if (this.hasOption(OPTION_CATCH) && !!catchClause) {
const tryClosingBrace = node.tryBlock.getChildAt(node.tryBlock.getChildCount() - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getLastToken()

const catchKeyword = catchClause.getChildAt(0);
const tryClosingBraceLoc = sourceFile.getLineAndCharacterOfPosition(tryClosingBrace.getEnd());
const catchKeywordLoc = sourceFile.getLineAndCharacterOfPosition(catchKeyword.getStart());
if (tryClosingBraceLoc.line !== (catchKeywordLoc.line - 1)) {
const failure = this.createFailure(catchKeyword.getStart(), catchKeyword.getWidth(), Rule.CATCH_FAILURE_STRING);
this.addFailure(failure);
}
}
super.visitTryStatement(node);
}
}

function getFirstChildOfKind(node: ts.Node, kind: ts.SyntaxKind) {
return node.getChildren().filter((child) => child.kind === kind)[0];
}
43 changes: 43 additions & 0 deletions scripts/tslint/noInferrableTypesRule.ts
@@ -0,0 +1,43 @@
/// <reference path="../../node_modules/tslint/typings/typescriptServices.d.ts" />
/// <reference path="../../node_modules/tslint/lib/tslint.d.ts" />


export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING_FACTORY = (type: string) => `LHS type (${type}) inferred by RHS expression, remove type annotation`;

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new InferrableTypeWalker(sourceFile, this.getOptions()));
}
}

class InferrableTypeWalker extends Lint.RuleWalker {
visitVariableStatement(node: ts.VariableStatement) {
node.declarationList.declarations.forEach(e => {
if ((!!e.type) && (!!e.initializer)) {
let failure: string;
switch (e.type.kind) {
case ts.SyntaxKind.BooleanKeyword:
if (e.initializer.kind === ts.SyntaxKind.TrueKeyword || e.initializer.kind === ts.SyntaxKind.FalseKeyword) {
failure = 'boolean';
}
break;
case ts.SyntaxKind.NumberKeyword:
if (e.initializer.kind === ts.SyntaxKind.NumericLiteral) {
failure = 'number';
}
break;
case ts.SyntaxKind.StringKeyword:
if (e.initializer.kind === ts.SyntaxKind.StringLiteral || e.initializer.kind === ts.SyntaxKind.NoSubstitutionTemplateLiteral) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider TemplateExpression as well

failure = 'string';
}
break;
}
if (failure) {
this.addFailure(this.createFailure(e.type.getStart(), e.type.getWidth(), Rule.FAILURE_STRING_FACTORY(failure)));
}
}
});

super.visitVariableStatement(node);
}
}
20 changes: 20 additions & 0 deletions scripts/tslint/noNullRule.ts
@@ -0,0 +1,20 @@
/// <reference path="../../node_modules/tslint/typings/typescriptServices.d.ts" />
/// <reference path="../../node_modules/tslint/lib/tslint.d.ts" />


export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING = "Don't use the 'null' keyword - use 'undefined' for missing values instead";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new NullWalker(sourceFile, this.getOptions()));
}
}

class NullWalker extends Lint.RuleWalker {
visitNode(node: ts.Node) {
super.visitNode(node);
if (node.kind === ts.SyntaxKind.NullKeyword) {
this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
}
}
}
7 changes: 7 additions & 0 deletions scripts/tslint/tsconfig.json
@@ -0,0 +1,7 @@
{
"compilerOptions": {
"noImplicitAny": true,
"module": "commonjs",
"outDir": "../../built/local/tslint"
}
}
17 changes: 14 additions & 3 deletions tslint.json
Expand Up @@ -8,7 +8,8 @@
"spaces"
],
"one-line": [true,
"check-open-brace"
"check-open-brace",
"check-whitespace"
],
"no-unreachable": true,
"no-use-before-declare": true,
Expand All @@ -21,14 +22,24 @@
"check-branch",
"check-operator",
"check-separator",
"check-type"
"check-type",
"check-module"
],
"typedef-whitespace": [true, {
"call-signature": "nospace",
"index-signature": "nospace",
"parameter": "nospace",
"property-declaration": "nospace",
"variable-declaration": "nospace"
}]
}],
"next-line": [true,
"check-catch",
"check-else"
],
"no-internal-module": true,
"no-trailing-whitespace": true,
"no-inferrable-types": true,
"no-null": true,
"boolean-trivia": true
}
}