Use --fix instead of -t applyFixes (#1697)

docs/_data/formatters.json

@@ -1,11 +1,4 @@
 [
-  {
-    "formatterName": "applyFixes",
-    "description": "Automatically fixes lint failures.",
-    "descriptionDetails": "\nModifies source files and applies fixes for lint failures where possible. Changes\nshould be tested as not all fixes preserve semantics.",
-    "sample": "\nAll done. Remember to test the changes, as not all fixes preserve semantics.",
-    "consumer": "machine"
-  },
   {
     "formatterName": "checkstyle",
     "description": "Formats errors as through they were Checkstyle output.",

src/formatters/proseFormatter.ts

@@ -29,12 +29,30 @@ export class Formatter extends AbstractFormatter {
     };
     /* tslint:enable:object-literal-sort-keys */
 
-    public format(failures: RuleFailure[]): string {
-        if (failures.length === 0) {
+    public format(failures: RuleFailure[], fixes?: RuleFailure[]): string {
+        if (failures.length === 0 && (!fixes || fixes.length === 0)) {
             return "";
         }
 
-        const outputLines = failures.map((failure: RuleFailure) => {
+        let fixLines: string[] = [];
+        if (fixes) {
+            let perFileFixes: { [fileName: string]: number } = {};
+            for (const fix of fixes) {
+                if (perFileFixes[fix.getFileName()] == null) {
+                    perFileFixes[fix.getFileName()] = 1;
+                } else {
+                    perFileFixes[fix.getFileName()]++;
+                }
+            }
+
+            Object.keys(perFileFixes).forEach((fixedFile: string) => {
+                const fixCount = perFileFixes[fixedFile];
+                fixLines.push(`Fixed ${fixCount} error(s) in ${fixedFile}`);
+            });
+            fixLines.push("");   // add a blank line between fixes and failures
+        }
+
+        let errorLines = failures.map((failure: RuleFailure) => {
             const fileName = failure.getFileName();
             const failureString = failure.getFailure();
 
@@ -44,6 +62,6 @@ export class Formatter extends AbstractFormatter {
             return `${fileName}${positionTuple}: ${failureString}`;
         });
 
-        return outputLines.join("\n") + "\n";
+        return fixLines.concat(errorLines).join("\n") + "\n";
     }
 }

src/language/formatter/formatter.ts

@@ -47,5 +47,10 @@ export interface IFormatterMetadata {
 export type ConsumerType = "human" | "machine";
 
 export interface IFormatter {
-    format(failures: RuleFailure[]): string;
+    /**
+     * Formats linter results
+     * @param {RuleFailure[]} failures Linter errors that were not fixed
+     * @param {RuleFailure[]} fixes Fixed linter errors. Available when the `--fix` argument is used on the command line
+     */
+    format(failures: RuleFailure[], fixes?: RuleFailure[]): string;
 }

src/lint.ts

@@ -42,6 +42,7 @@ export var Utils = utils;
 export interface LintResult {
     failureCount: number;
     failures: RuleFailure[];
+    fixes?: RuleFailure[];
     format: string | Function;
     output: string;
 }
@@ -55,11 +56,13 @@ export interface ILinterOptionsRaw {
 
 export interface ILinterOptions extends ILinterOptionsRaw {
     configuration: any;
+    fix: boolean;
     formatter: string | Function;
     rulesDirectory: string | string[];
 }
 
 export interface IMultiLinterOptions {
+    fix: boolean;
     formatter?: string | Function;
     formattersDirectory?: string;
     rulesDirectory?: string | string[];

src/tslint-cli.ts

@@ -50,6 +50,9 @@ let processed = optimist
             alias: "exclude",
             describe: "exclude globs from path expansion",
         },
+        fix: {
+            describe: "Fixes linting errors for select rules. This may overwrite linted files",
+        },
         force: {
             describe: "return status code 0 even if there are lint errors",
             type: "boolean",
@@ -152,6 +155,9 @@ tslint accepts the following commandline options:
         This option can be supplied multiple times if you need multiple
         globs to indicate which files to exclude.
 
+    --fix:
+        Fixes linting errors for select rules. This may overwrite linted files.
+
     --force:
         Return status code 0 even if there are any lint errors.
         Useful while running as npm script.
@@ -220,6 +226,7 @@ const possibleConfigAbsolutePath = argv.c != null ? path.resolve(argv.c) : null;
 const processFiles = (files: string[], program?: ts.Program) => {
 
     const linter = new Linter({
+        fix: argv.fix,
         formatter: argv.t,
         formattersDirectory: argv.s || "",
         rulesDirectory: argv.r || "",

src/tslint.ts

@@ -75,6 +75,7 @@ class Linter {
 
         return {
             configuration: configuration || DEFAULT_CONFIG,
+            fix: false,
             formatter: formatter || "prose",
             formattersDirectory,
             rulesDirectory: arrayify(rulesDirectory).concat(arrayify(configuration.rulesDirectory)),

src/tslintMulti.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { existsSync } from "fs";
+import * as fs from "fs";
 import * as ts from "typescript";
 
 import {
@@ -30,10 +30,10 @@ import {
 import { EnableDisableRulesWalker } from "./enableDisableRules";
 import { findFormatter } from "./formatterLoader";
 import { IFormatter } from "./language/formatter/formatter";
-import { RuleFailure } from "./language/rule/rule";
+import {Fix, IRule, RuleFailure} from "./language/rule/rule";
 import { TypedRule } from "./language/rule/typedRule";
-import { getSourceFile } from "./language/utils";
-import { IMultiLinterOptions, IRule, LintResult } from "./lint";
+import * as utils from "./language/utils";
+import { IMultiLinterOptions, LintResult } from "./lint";
 import { loadRules } from "./ruleLoader";
 import { arrayify } from "./utils";
 
@@ -49,6 +49,7 @@ class MultiLinter {
     public static loadConfigurationFromPath = loadConfigurationFromPath;
 
     private failures: RuleFailure[] = [];
+    private fixes: RuleFailure[] = [];
 
     /**
      * Creates a TypeScript program object from a tsconfig.json file path and optional project directory.
@@ -65,7 +66,7 @@ class MultiLinter {
 
         const { config } = ts.readConfigFile(configFile, ts.sys.readFile);
         const parseConfigHost = {
-            fileExists: existsSync,
+            fileExists: fs.existsSync,
             readDirectory: ts.sys.readDirectory,
             useCaseSensitiveFileNames: true,
         };
@@ -89,54 +90,35 @@ class MultiLinter {
     }
 
     public lint(fileName: string, source?: string, configuration: IConfigurationFile = DEFAULT_CONFIG): void {
-        let sourceFile: ts.SourceFile;
-        if (this.program) {
-            sourceFile = this.program.getSourceFile(fileName);
-            // check if the program has been type checked
-            if (sourceFile && !("resolvedModules" in sourceFile)) {
-                throw new Error("Program must be type checked before linting");
+        const enabledRules = this.getEnabledRules(fileName, source, configuration);
+        let sourceFile = this.getSourceFile(fileName, source);
+        let hasLinterRun = false;
+
+        if (this.options.fix) {
+            this.fixes = [];
+            for (let rule of enabledRules) {
+                let fileFailures = this.applyRule(rule, sourceFile);
+                const fixes = fileFailures.map(f => f.getFix()).filter(f => !!f);
+                source = fs.readFileSync(fileName, { encoding: "utf-8" });
+                if (fixes.length > 0) {
+                    this.fixes = this.fixes.concat(fileFailures);
+                    source = Fix.applyAll(source, fixes);
+                    fs.writeFileSync(fileName, source, { encoding: "utf-8" });
+
+                    // reload AST if file is modified
+                    sourceFile = this.getSourceFile(fileName, source);
+                }
+                this.failures = this.failures.concat(fileFailures);
             }
-        } else {
-            sourceFile = getSourceFile(fileName, source);
+            hasLinterRun = true;
         }
 
-        if (sourceFile === undefined) {
-            throw new Error(`Invalid source file: ${fileName}. Ensure that the files supplied to lint have a .ts, .tsx, or .js extension.`);
-        }
-
-        // walk the code first to find all the intervals where rules are disabled
-        const rulesWalker = new EnableDisableRulesWalker(sourceFile, {
-            disabledIntervals: [],
-            ruleName: "",
-        });
-        rulesWalker.walk(sourceFile);
-        const enableDisableRuleMap = rulesWalker.enableDisableRuleMap;
-
-        const rulesDirectories = arrayify(this.options.rulesDirectory)
-            .concat(arrayify(configuration.rulesDirectory));
-        const configurationRules = configuration.rules;
-        const jsConfiguration = configuration.jsRules;
-        const isJs = fileName.substr(-3) === ".js";
-        let configuredRules: IRule[];
-
-        if (isJs) {
-            configuredRules = loadRules(jsConfiguration, enableDisableRuleMap, rulesDirectories, true);
-        } else {
-            configuredRules = loadRules(configurationRules, enableDisableRuleMap, rulesDirectories, false);
-        }
-
-        const enabledRules = configuredRules.filter((r) => r.isEnabled());
-        for (let rule of enabledRules) {
-            let ruleFailures: RuleFailure[] = [];
-            if (this.program && rule instanceof TypedRule) {
-                ruleFailures = rule.applyWithProgram(sourceFile, this.program);
-            } else {
-                ruleFailures = rule.apply(sourceFile);
-            }
-            for (let ruleFailure of ruleFailures) {
-                if (!this.containsRule(this.failures, ruleFailure)) {
-                    this.failures.push(ruleFailure);
-                }
+        // make a 1st pass or make a 2nd pass if there were any fixes because the positions may be off        
+        if (!hasLinterRun || this.fixes.length > 0) {
+            this.failures = [];
+            for (let rule of enabledRules) {
+                const fileFailures = this.applyRule(rule, sourceFile);
+                this.failures = this.failures.concat(fileFailures);
             }
         }
     }
@@ -153,16 +135,71 @@ class MultiLinter {
             throw new Error(`formatter '${formatterName}' not found`);
         }
 
-        const output = formatter.format(this.failures);
+        const output = formatter.format(this.failures, this.fixes);
 
         return {
             failureCount: this.failures.length,
             failures: this.failures,
+            fixes: this.fixes,
             format: formatterName,
             output,
         };
     }
 
+    private applyRule(rule: IRule, sourceFile: ts.SourceFile) {
+        let ruleFailures: RuleFailure[] = [];
+        if (this.program && rule instanceof TypedRule) {
+            ruleFailures = rule.applyWithProgram(sourceFile, this.program);
+        } else {
+            ruleFailures = rule.apply(sourceFile);
+        }
+        let fileFailures: RuleFailure[] = [];
+        for (let ruleFailure of ruleFailures) {
+            if (!this.containsRule(this.failures, ruleFailure)) {
+                fileFailures.push(ruleFailure);
+            }
+        }
+        return fileFailures;
+    }
+
+    private getEnabledRules(fileName: string, source?: string, configuration: IConfigurationFile = DEFAULT_CONFIG): IRule[] {
+        const sourceFile = this.getSourceFile(fileName, source);
+
+        // walk the code first to find all the intervals where rules are disabled
+        const rulesWalker = new EnableDisableRulesWalker(sourceFile, {
+            disabledIntervals: [],
+            ruleName: "",
+        });
+        rulesWalker.walk(sourceFile);
+        const enableDisableRuleMap = rulesWalker.enableDisableRuleMap;
+
+        const rulesDirectories = arrayify(this.options.rulesDirectory)
+            .concat(arrayify(configuration.rulesDirectory));
+        const isJs = fileName.substr(-3) === ".js";
+        const configurationRules = isJs ? configuration.jsRules : configuration.rules;
+        let configuredRules = loadRules(configurationRules, enableDisableRuleMap, rulesDirectories, isJs);
+
+        return configuredRules.filter((r) => r.isEnabled());
+    }
+
+    private getSourceFile(fileName: string, source?: string) {
+        let sourceFile: ts.SourceFile;
+        if (this.program) {
+            sourceFile = this.program.getSourceFile(fileName);
+            // check if the program has been type checked
+            if (sourceFile && !("resolvedModules" in sourceFile)) {
+                throw new Error("Program must be type checked before linting");
+            }
+        } else {
+            sourceFile = utils.getSourceFile(fileName, source);
+        }
+
+        if (sourceFile === undefined) {
+            throw new Error(`Invalid source file: ${fileName}. Ensure that the files supplied to lint have a .ts, .tsx, or .js extension.`);
+        }
+        return sourceFile;
+    }
+
     private containsRule(rules: RuleFailure[], rule: RuleFailure) {
         return rules.some((r) => r.equals(rule));
     }