Merge pull request #4458 from weswigham/tslint-rules

Add tslint rules for #3994
microsoft · Sep 18, 2015 · e813963 · e813963
2 parents e200dad + 2793bc2
commit e813963
Show file tree

Hide file tree

Showing 7 changed files with 222 additions and 5 deletions.
diff --git a/Jakefile.js b/Jakefile.js
@@ -780,17 +780,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 });
diff --git a/scripts/tslint/booleanTriviaRule.ts b/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)));
+                        }
+                    }
+                }
+            }
+		}			
+    }
+}
diff --git a/scripts/tslint/nextLineRule.ts b/scripts/tslint/nextLineRule.ts
@@ -0,0 +1,61 @@
+/// <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(sourceFile));
+                if (thenStatementEndLoc.line !== (elseKeywordLoc.line - 1)) {
+                    const failure = this.createFailure(elseKeyword.getStart(sourceFile), elseKeyword.getWidth(sourceFile), 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 tryBlock = node.tryBlock;
+
+        if (this.hasOption(OPTION_CATCH) && !!catchClause) {
+            const tryClosingBrace = tryBlock.getLastToken(sourceFile);
+            const catchKeyword = catchClause.getFirstToken(sourceFile);
+            const tryClosingBraceLoc = sourceFile.getLineAndCharacterOfPosition(tryClosingBrace.getEnd());
+            const catchKeywordLoc = sourceFile.getLineAndCharacterOfPosition(catchKeyword.getStart(sourceFile));
+            if (tryClosingBraceLoc.line !== (catchKeywordLoc.line - 1)) {
+                const failure = this.createFailure(catchKeyword.getStart(sourceFile), catchKeyword.getWidth(sourceFile), 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];
+}
diff --git a/scripts/tslint/noInferrableTypesRule.ts b/scripts/tslint/noInferrableTypesRule.ts
@@ -0,0 +1,49 @@
+/// <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:
+                        switch (e.initializer.kind) {
+                            case ts.SyntaxKind.StringLiteral:
+                            case ts.SyntaxKind.NoSubstitutionTemplateLiteral:
+                            case ts.SyntaxKind.TemplateExpression:
+                                failure = 'string';
+                                break;
+                            default:
+                                break;
+                        }
+                    break;
+                }
+                if (failure) {
+                    this.addFailure(this.createFailure(e.type.getStart(), e.type.getWidth(), Rule.FAILURE_STRING_FACTORY(failure)));
+                }
+            }
+        });
+
+        super.visitVariableStatement(node);
+    }
+}
diff --git a/scripts/tslint/noNullRule.ts b/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));
+        }
+    }
+}
diff --git a/scripts/tslint/tsconfig.json b/scripts/tslint/tsconfig.json
@@ -0,0 +1,7 @@
+{
+	"compilerOptions": {
+		"noImplicitAny": true,
+		"module": "commonjs",
+		"outDir": "../../built/local/tslint"
+	}
+}
diff --git a/tslint.json b/tslint.json
@@ -8,7 +8,8 @@
             "spaces"
         ],
         "one-line": [true,
-            "check-open-brace"
+            "check-open-brace",
+            "check-whitespace"
         ],
         "no-unreachable": true,
         "no-use-before-declare": true,
@@ -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
   }
 }