Add text replacement fixes for rules

src/language/rule/rule.ts

@@ -89,6 +89,63 @@ export interface IRule {
     applyWithWalker(walker: RuleWalker): RuleFailure[];
 }
 
+export class Replacement {
+    public static applyAll(content: string, replacements: Replacement[]) {
+        // sort in reverse so that diffs are properly applied
+        replacements.sort((a, b) => b.end - a.end);
+        return replacements.reduce((text, r) => r.apply(text), content);
+    }
+
+    constructor(private innerStart: number, private innerLength: number, private innerText: string) {
+    }
+
+    get start() {
+        return this.innerStart;
+    }
+
+    get length() {
+        return this.innerLength;
+    }
+
+    get end() {
+        return this.innerStart + this.innerLength;
+    }
+
+    get text() {
+        return this.innerText;
+    }
+
+    public apply(content: string) {
+        return content.substring(0, this.start) + this.text + content.substring(this.start + this.length);
+    }
+}
+
+export class Fix {
+    public static applyAll(content: string, fixes: Fix[]) {
+        // accumulate replacements
+        let replacements: Replacement[] = [];
+        for (const fix of fixes) {
+            replacements = replacements.concat(fix.replacements);
+        }
+        return Replacement.applyAll(content, replacements);
+    }
+
+    constructor(private innerRuleName: string, private innerReplacements: Replacement[]) {
+    }
+
+    get ruleName() {
+        return this.innerRuleName;
+    }
+
+    get replacements() {
+        return this.innerReplacements;
+    }
+
+    public apply(content: string) {
+        return Replacement.applyAll(content, this.innerReplacements);
+    }
+}
+
 export class RuleFailurePosition {
     constructor(private position: number, private lineAndCharacter: ts.LineAndCharacter) {
     }
@@ -128,7 +185,8 @@ export class RuleFailure {
                 start: number,
                 end: number,
                 private failure: string,
-                private ruleName: string) {
+                private ruleName: string,
+                private fix?: Fix) {
 
         this.fileName = sourceFile.fileName;
         this.startPosition = this.createFailurePosition(start);
@@ -155,10 +213,19 @@ export class RuleFailure {
         return this.failure;
     }
 
+    public hasFix() {
+        return this.fix !== undefined;
+    }
+
+    public getFix() {
+        return this.fix;
+    }
+
     public toJson(): any {
         return {
             endPosition: this.endPosition.toJson(),
             failure: this.failure,
+            fix: this.fix,
             name: this.fileName,
             ruleName: this.ruleName,
             startPosition: this.startPosition.toJson(),

src/language/walker/ruleWalker.ts

@@ -18,7 +18,7 @@
 import * as ts from "typescript";
 
 import {IOptions} from "../../lint";
-import {IDisabledInterval, RuleFailure} from "../rule/rule";
+import {Fix, IDisabledInterval, Replacement, RuleFailure} from "../rule/rule";
 import {doesIntersect} from "../utils";
 import {SyntaxWalker} from "./syntaxWalker";
 
@@ -69,10 +69,10 @@ export class RuleWalker extends SyntaxWalker {
         this.position += node.getFullWidth();
     }
 
-    public createFailure(start: number, width: number, failure: string): RuleFailure {
+    public createFailure(start: number, width: number, failure: string, fix?: Fix): RuleFailure {
         const from = (start > this.limit) ? this.limit : start;
         const to = ((start + width) > this.limit) ? this.limit : (start + width);
-        return new RuleFailure(this.sourceFile, from, to, failure, this.ruleName);
+        return new RuleFailure(this.sourceFile, from, to, failure, this.ruleName, fix);
     }
 
     public addFailure(failure: RuleFailure) {
@@ -82,6 +82,18 @@ export class RuleWalker extends SyntaxWalker {
         }
     }
 
+    public createReplacement(start: number, length: number, text: string): Replacement {
+        return new Replacement(start, length, text);
+    }
+
+    public appendText(start: number, text: string): Replacement {
+        return this.createReplacement(start, 0, text);
+    }
+
+    public deleteText(start: number, length: number): Replacement {
+        return this.createReplacement(start, length, "");
+    }
+
     private existsFailure(failure: RuleFailure) {
         return this.failures.some((f) => f.equals(failure));
     }

src/rules/semicolonRule.ts

@@ -149,7 +149,11 @@ class SemicolonWalker extends Lint.RuleWalker {
         const always = this.hasOption(OPTION_ALWAYS) || (this.getOptions() && this.getOptions().length === 0);
 
         if (always && !hasSemicolon) {
-            this.addFailure(this.createFailure(Math.min(position, this.getLimit()), 0, Rule.FAILURE_STRING_MISSING));
+            const failureStart = Math.min(position, this.getLimit());
+            const fix = new Lint.Fix(Rule.metadata.ruleName, [
+                this.appendText(failureStart, ";"),
+            ]);
+            this.addFailure(this.createFailure(failureStart, 0, Rule.FAILURE_STRING_MISSING, fix));
         } else if (this.hasOption(OPTION_NEVER) && hasSemicolon) {
             const scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, sourceFile.text);
             scanner.setTextPos(position);
@@ -161,7 +165,11 @@ class SemicolonWalker extends Lint.RuleWalker {
 
             if (tokenKind !== ts.SyntaxKind.OpenParenToken && tokenKind !== ts.SyntaxKind.OpenBracketToken
                     && tokenKind !== ts.SyntaxKind.PlusToken && tokenKind !== ts.SyntaxKind.MinusToken) {
-                this.addFailure(this.createFailure(Math.min(position - 1, this.getLimit()), 1, Rule.FAILURE_STRING_UNNECESSARY));
+                const failureStart = Math.min(position - 1, this.getLimit());
+                const fix = new Lint.Fix(Rule.metadata.ruleName, [
+                    this.deleteText(failureStart, 1),
+                ]);
+                this.addFailure(this.createFailure(failureStart, 1, Rule.FAILURE_STRING_UNNECESSARY, fix));
             }
         }
     }

src/test.ts

@@ -22,32 +22,36 @@ import * as glob from "glob";
 import * as path from "path";
 import * as ts from "typescript";
 
+import {Fix} from "./language/rule/rule";
 import {createCompilerOptions} from "./language/utils";
 import {LintError} from "./test/lintError";
 import * as parse from "./test/parse";
 import * as Linter from "./tslint";
 
-const FILE_EXTENSION = ".lint";
+const MARKUP_FILE_EXTENSION = ".lint";
+const FIXES_FILE_EXTENSION = ".fix";
 
 export interface TestResult {
     directory: string;
     results: {
         [fileName: string]: {
-            errorsFromMarkup: LintError[];
             errorsFromLinter: LintError[];
+            errorsFromMarkup: LintError[];
+            fixesFromLinter: string;
+            fixesFromMarkup: string;
             markupFromLinter: string;
             markupFromMarkup: string;
         }
     };
 }
 
 export function runTest(testDirectory: string, rulesDirectory?: string | string[]): TestResult {
-    const filesToLint = glob.sync(path.join(testDirectory, `**/*${FILE_EXTENSION}`));
+    const filesToLint = glob.sync(path.join(testDirectory, `**/*${MARKUP_FILE_EXTENSION}`));
     const tslintConfig = Linter.findConfiguration(path.join(testDirectory, "tslint.json"), null);
     const results: TestResult = { directory: testDirectory, results: {} };
 
     for (const fileToLint of filesToLint) {
-        const fileBasename = path.basename(fileToLint, FILE_EXTENSION);
+        const fileBasename = path.basename(fileToLint, MARKUP_FILE_EXTENSION);
         const fileCompileName = fileBasename.replace(/\.lint$/, "");
         const fileText = fs.readFileSync(fileToLint, "utf8");
         const fileTextWithoutMarkup = parse.removeErrorMarkup(fileText);
@@ -87,7 +91,8 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
             rulesDirectory,
         };
         const linter = new Linter(fileBasename, fileTextWithoutMarkup, lintOptions, program);
-        const errorsFromLinter: LintError[] = linter.lint().failures.map((failure) => {
+        const failures = linter.lint().failures;
+        const errorsFromLinter: LintError[] = failures.map((failure) => {
             const startLineAndCharacter = failure.getStartPosition().getLineAndCharacter();
             const endLineAndCharacter = failure.getEndPosition().getLineAndCharacter();
 
@@ -104,9 +109,27 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
             };
         });
 
+        // test against fixed files
+        let fixedFileText: string;
+        let newFileText: string;
+        try {
+            const fixedFile = fileToLint.replace(/\.lint$/, FIXES_FILE_EXTENSION);
+            const stat = fs.statSync(fixedFile);
+            if (stat.isFile()) {
+                fixedFileText = fs.readFileSync(fixedFile, "utf8");
+                const fixes = failures.filter(f => f.hasFix()).map(f => f.getFix());
+                newFileText = Fix.applyAll(fileTextWithoutMarkup, fixes);
+            }
+        } catch (e) {
+            fixedFileText = "";
+            newFileText = "";
+        }
+
         results.results[fileToLint] = {
-            errorsFromMarkup,
             errorsFromLinter,
+            errorsFromMarkup,
+            fixesFromLinter: newFileText,
+            fixesFromMarkup: fixedFileText,
             markupFromLinter: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromMarkup),
             markupFromMarkup: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromLinter),
         };
@@ -122,31 +145,43 @@ export function consoleTestResultHandler(testResult: TestResult): boolean {
         const results = testResult.results[fileName];
         process.stdout.write(`${fileName}:`);
 
-        const diffResults = diff.diffLines(results.markupFromMarkup, results.markupFromLinter);
-        const didTestPass = !diffResults.some((diff) => diff.added || diff.removed);
+        const markupDiffResults = diff.diffLines(results.markupFromMarkup, results.markupFromLinter);
+        const fixesDiffResults = diff.diffLines(results.fixesFromMarkup, results.fixesFromLinter);
+        const didMarkupTestPass = !markupDiffResults.some((diff) => diff.added || diff.removed);
+        const didFixesTestPass = !fixesDiffResults.some((diff) => diff.added || diff.removed);
 
         /* tslint:disable:no-console */
-        if (didTestPass) {
+        if (didMarkupTestPass && didFixesTestPass) {
             console.log(colors.green(" Passed"));
         } else {
             console.log(colors.red(" Failed!"));
-            console.log(colors.green(`Expected (from ${FILE_EXTENSION} file)`));
-            console.log(colors.red("Actual (from TSLint)"));
-
             didAllTestsPass = false;
-
-            for (const diffResult of diffResults) {
-                let color = colors.grey;
-                if (diffResult.added) {
-                    color = colors.green;
-                } else if (diffResult.removed) {
-                    color = colors.red;
-                }
-                process.stdout.write(color(diffResult.value));
+            if (!didMarkupTestPass) {
+                displayDiffResults(markupDiffResults, MARKUP_FILE_EXTENSION);
+            }
+            if (!didFixesTestPass) {
+                displayDiffResults(fixesDiffResults, FIXES_FILE_EXTENSION);
             }
         }
         /* tslint:enable:no-console */
     }
 
     return didAllTestsPass;
 }
+
+function displayDiffResults(diffResults: diff.IDiffResult[], extension: string) {
+    /* tslint:disable:no-console */
+    console.log(colors.green(`Expected (from ${extension} file)`));
+    console.log(colors.red("Actual (from TSLint)"));
+
+    for (const diffResult of diffResults) {
+        let color = colors.grey;
+        if (diffResult.added) {
+            color = colors.green;
+        } else if (diffResult.removed) {
+            color = colors.red;
+        }
+        process.stdout.write(color(diffResult.value));
+    }
+    /* tslint:enable:no-console */
+}

test/check-bin.sh

@@ -108,6 +108,13 @@ expectOut $? 0 "tslint --test did not exit correctly for a passing test with cus
 ./bin/tslint -r test/files/custom-rules-2 --test test/files/custom-rule-cli-rule-test
 expectOut $? 0 "tslint --test did not exit correctly for a passing test with custom rules from the CLI"
 
+# ensure --test command works correctly with fixes
+./bin/tslint --test test/files/fixes-test
+expectOut $? 0 "tslint --test did not exit correctly for a passing test with fixes"
+
+./bin/tslint --test test/files/incorrect-fixes-test
+expectOut $? 1 "tslint --test did not exit correctly for a failing test with fixes"
+
 # make sure tslint exits correctly when tsconfig is specified but no files are given
 ./bin/tslint -c test/files/tsconfig-test/tslint.json --project test/files/tsconfig-test/tsconfig.json
 expectOut $? 0 "tslint with tsconfig did not exit correctly"

test/files/fixes-test/test.ts.fix

@@ -0,0 +1,9 @@
+var testVariable = "eval";
+
+function a() {
+    function b() {
+        function c() {
+            console.log("hi");
+        }
+    }
+}

test/files/fixes-test/test.ts.lint

@@ -0,0 +1,11 @@
+var testVariable = "eval"
+                         ~nil [Missing semicolon]
+
+function a() {
+    function b() {
+        function c() {
+            console.log("hi")
+                             ~nil [Missing semicolon]
+        }
+    }
+}

test/files/fixes-test/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "semicolon": [true, "always"]
+  }
+}

test/files/incorrect-fixes-test/test.ts.fix

@@ -0,0 +1,9 @@
+var testVariable = "eval";
+
+function a() {
+    function b() {
+        function c() {
+            console.log("hi")
+        }
+    }
+}

test/files/incorrect-fixes-test/test.ts.lint

@@ -0,0 +1,11 @@
+var testVariable = "eval"
+                         ~nil [Missing semicolon]
+
+function a() {
+    function b() {
+        function c() {
+            console.log("hi")
+                             ~nil [Missing semicolon]
+        }
+    }
+}

test/files/incorrect-fixes-test/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "semicolon": [true, "always"]
+  }
+}