Add dry-run to Linter API fix option

[jmlopez-rod]

The current implementation in tslint 4.0 for the lint method has the following:

Linter.prototype.lint = function (fileName, source, configuration) {
        if (configuration === void 0) { configuration = configuration_1.DEFAULT_CONFIG; }
        var enabledRules = this.getEnabledRules(fileName, source, configuration);
        var sourceFile = this.getSourceFile(fileName, source);
        var hasLinterRun = false;
        var fileFailures = [];
        if (this.options.fix) {
            this.fixes = [];
            for (var _i = 0, enabledRules_1 = enabledRules; _i < enabledRules_1.length; _i++) {
                var rule = enabledRules_1[_i];
                var ruleFailures = this.applyRule(rule, sourceFile);
                var fixes = ruleFailures.map(function (f) { return f.getFix(); }).filter(function (f) { return !!f; });
                source = fs.readFileSync(fileName, { encoding: "utf-8" });
                if (fixes.length > 0) {
                    this.fixes = this.fixes.concat(ruleFailures);
                    source = rule_1.Fix.applyAll(source, fixes);
                    fs.writeFileSync(fileName, source, { encoding: "utf-8" });
                    // reload AST if file is modified
                    sourceFile = this.getSourceFile(fileName, source);
                }
                fileFailures = fileFailures.concat(ruleFailures);
            }
            hasLinterRun = true;
        }

If in the configuration I set fix to true then source = fs.readFileSync(fileName, { encoding: "utf-8" }); gets called. This is great for the CLI but when writing my custom code I usually don't pass the fileName of something that exists. I simply pass the source code. Now, in this case I wanted to try to see if I could see the results of the fixer but it failed at the line I just mentioned.

Could we get pass in an extra option in order to prevent the lint method from reading and writing the file? A dry run option would be great, for instance:

export interface ILinterOptions {
    fix: boolean;
    dryRun?: boolean;
    formatter?: string | Function;
    formattersDirectory?: string;
    rulesDirectory?: string | string[];
}

By default we can assume that dryRun is false so that the same behaviour stays the same.

The main goal I want to achieve is to get all the failures and the output of the fixer.

[donaldpipowitch]

I'd like to get the fixed result without actually writing it to the file as well. We run TSLint, ESLint and Prettier in a pipe and it would be nice, if I could just write the newest versions of a file at the end on my own.

[ajafff]

My thoughts on this:

• Linter#lint could return this fixed file contents instead of writing them to disk directly
  • That's a bit tricky because linting one file can result in fixes for another file - fix(Linter): apply fix to the correct file #2421
  • After fixing one rule's failures, the linter runs again. When using --project we need to write the new content in order to update the ts.Program. Otherwise we would need to create our own ts.CompilerHost that returns the in memory file contents instead of reading the file.
• Let the consumer handle all of the above:
  • don't enable the fix option
  • get the list of failures from linter.lint
  • apply the fixes like tslint does
  • (create a new ts.Program with the changed file contents)
  • repeat
  • now you have your fixed file contents without writing them to disk

[ajafff]

What do you think about adding a similar concept as TypeScript's CompilerHost to add an abstraction for file system operations? Let's just call it LinterHost for now.
TSLint would then call LinterHost#writeFile with the fixed source code. The default would just delegate to fs.writeFileSync.
If you implement a custom LinterHost you can store the new file contents in memory instead of writing them to disk.

[azz]

My use case is the same as ESLint fix's API.

ESLint has two methods:

• executeOnText(), similar to Linter#lint, but does not write to disk.
• outputFixes(), writes the fixes to disk.

This is great for building tools where you are combining multiple tools together, such as prettier-tslint, which I'm working on now.

I always saw the CompilerHost API as a little bit sloppy. Great for unit tests, but ugly to use in practice.

[adidahiya]

This is great for building tools where you are combining multiple tools together, such as prettier-tslint, which I'm working on now.

@azz This is tangential to the issue thread, but have you checked out https://github.com/ikatyang/tslint-plugin-prettier? Curious to hear your thoughts on the tradeoffs between the two.

[azz]

prettier-tslint is to tslint-plugin-prettier as prettier-eslint is to eslint-plugin-prettier. If you want E/TSLint to validate that your style matches 100% with what Prettier says, use (e|t)slint-plugin-prettier. If you want to perform E/TSLint fixes that affect code style, they the plugin is incompatible and you use prettier-(e|t)slint. My main use case for having prettier-tslint is the editor integration, you can have style changes (Prettier) and code fixes (TSLint) happen on-save.

e.g. I can type:

if(!foo) throw "bah!"

save, then it will transform to:

if (!foo) {
  throw new Error("bah!");
}

[adidahiya]

My main use case for having prettier-tslint is the editor integration, you can have style changes (Prettier) and code fixes (TSLint) happen on-save.

FYI, this is also how I currently use tslint-plugin-prettier (in conjunction with tslint-config-prettier)...
I get both prettier formatting + TSLint auto-fixes on save in VSCode.

It sounds like you are just allowing some TSLint rules to override prettier's choices then? That makes sense, I guess, if you want to diverge from prettier's opinions. Thanks for explaining.

[ajafff]

@azz you can already achieve the same as executeOnText.
After calling linter.lint(...) you use linter.getResult() to get the fixes. Be aware that the fixes property of the result object are not the real fixes, it's an array of all failures that have fixes.
Collect all fixes and use Replacement.applyFixes to apply them. Note that it doesn't handle overlapping fixes. You would need a custom algorithm to handle that.

ESLint works very similar: it gathers all fixes, sorts them in descending order and applies them to the text. If there are overlapping fixes, only the first is applied and the second is ignored.
The only difference is that TSLint failures can contain more than one replacement.

[azz]

Seems like a lot of work that should live inside TSLint itself.

ESLint's executeOnText() provides results.output which is the fixed code. Not sure what the objection is for TSLint to do the same, when it is trivial to add?

[JoshuaKGoldberg]

By now, this is out of scope. 😿 See #4534.

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋