Skip to content

Commit

Permalink
New: Add the fix-dry-run flag (fixes #9076) (#9073)
Browse files Browse the repository at this point in the history
  • Loading branch information
fatfisz authored and not-an-aardvark committed Oct 2, 2017
1 parent 8ebb034 commit 4567ab1
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 4 deletions.
15 changes: 15 additions & 0 deletions docs/user-guide/command-line-interface.md
Expand Up @@ -69,6 +69,7 @@ Output:
Miscellaneous:
--init Run config initialization wizard - default: false
--fix Automatically fix problems
--fix-dry-run Automatically fix problems without saving the changes to the file system
--debug Output debugging information
-h, --help Show help
-v, --version Output the version number
Expand Down Expand Up @@ -362,6 +363,20 @@ This option instructs ESLint to try to fix as many issues as possible. The fixes
1. This option throws an error when code is piped to ESLint.
1. This option has no effect on code that uses a processor, unless the processor opts into allowing autofixes.

If you want to fix code from `stdin` or otherwise want to get the fixes without actually writing them to the file, use the [`--fix-dry-run`](#--fix-dry-run) option.

#### `--fix-dry-run`

This option has the same effect as `--fix` with one difference: the fixes are not saved to the file system. This makes it possible to fix code from `stdin` (when used with the `--stdin` flag).

Because the default formatter does not output the fixed code, you'll have to use another one (e.g. `json`) to get the fixes. Here's an example of this pattern:

```
getSomeText | eslint --stdin --fix-dry-run --format=json
```

This flag can be useful for integrations (e.g. editor plugins) which need to autofix text from the command line without saving it to the filesystem.

#### `--debug`

This option outputs debugging information to the console. This information is useful when you're seeing a problem and having a hard time pinpointing it. The ESLint team may ask for this debugging information to help solve bugs.
Expand Down
10 changes: 7 additions & 3 deletions lib/cli.js
Expand Up @@ -63,7 +63,7 @@ function translateOptions(cliOptions) {
cache: cliOptions.cache,
cacheFile: cliOptions.cacheFile,
cacheLocation: cliOptions.cacheLocation,
fix: cliOptions.fix && (cliOptions.quiet ? quietFixPredicate : true),
fix: (cliOptions.fix || cliOptions.fixDryRun) && (cliOptions.quiet ? quietFixPredicate : true),
allowInlineConfig: cliOptions.inlineConfig,
reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives
};
Expand Down Expand Up @@ -171,9 +171,13 @@ const cli = {

debug(`Running on ${text ? "text" : "files"}`);

// disable --fix for piped-in code until we know how to do it correctly
if (currentOptions.fix && currentOptions.fixDryRun) {
log.error("The --fix option and the --fix-dry-run option cannot be used together.");
return 1;
}

if (text && currentOptions.fix) {
log.error("The --fix option is not available for piped-in code.");
log.error("The --fix option is not available for piped-in code; use --fix-dry-run instead.");
return 1;
}

Expand Down
6 changes: 6 additions & 0 deletions lib/options.js
Expand Up @@ -190,6 +190,12 @@ module.exports = optionator({
default: false,
description: "Automatically fix problems"
},
{
option: "fix-dry-run",
type: "Boolean",
default: false,
description: "Automatically fix problems without saving the changes to the file system"
},
{
option: "debug",
type: "Boolean",
Expand Down
35 changes: 35 additions & 0 deletions tests/bin/eslint.js
Expand Up @@ -71,6 +71,41 @@ describe("bin/eslint.js", () => {
return assertExitCode(child, 0);
});

it("has exit code 0 if no linting errors are reported", () => {
const child = runESLint([
"--stdin",
"--no-eslintrc",
"--rule",
"{'no-extra-semi': 2}",
"--fix-dry-run",
"--format",
"json"
]);

const expectedOutput = JSON.stringify([
{
filePath: "<text>",
messages: [],
errorCount: 0,
warningCount: 0,
fixableErrorCount: 0,
fixableWarningCount: 0,
output: "var foo = bar;\n"
}
]);

const exitCodePromise = assertExitCode(child, 0);
const stdoutPromise = getOutput(child).then(output => {
assert.strictEqual(output.stdout.trim(), expectedOutput);
assert.strictEqual(output.stderr, "");
});

child.stdin.write("var foo = bar;;\n");
child.stdin.end();

return Promise.all([exitCodePromise, stdoutPromise]);
});

it("has exit code 1 if a syntax error is thrown", () => {
const child = runESLint(["--stdin", "--no-eslintrc"]);

Expand Down
159 changes: 158 additions & 1 deletion tests/lib/cli.js
Expand Up @@ -755,7 +755,7 @@ describe("cli", () => {
results: []
});
sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done");
fakeCLIEngine.outputFixes = sandbox.stub();
fakeCLIEngine.outputFixes = sandbox.mock().once();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
Expand Down Expand Up @@ -858,6 +858,163 @@ describe("cli", () => {

});

describe("when passed --fix-dry-run", () => {
const sandbox = sinon.sandbox.create();
let localCLI;

afterEach(() => {
sandbox.verifyAndRestore();
});

it("should pass fix:true to CLIEngine when executing on files", () => {

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: true }));

fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype);
sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns({
errorCount: 0,
warningCount: 0,
results: []
});
sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done");
fakeCLIEngine.outputFixes = sandbox.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
"./logging": log
});

const exitCode = localCLI.execute("--fix-dry-run .");

assert.equal(exitCode, 0);

});

it("should not rewrite files when in fix-dry-run mode", () => {

const report = {
errorCount: 1,
warningCount: 0,
results: [{
filePath: "./foo.js",
output: "bar",
messages: [
{
severity: 2,
message: "Fake message"
}
]
}]
};

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: true }));

fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype);
sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report);
sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done");
fakeCLIEngine.outputFixes = sandbox.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
"./logging": log
});

const exitCode = localCLI.execute("--fix-dry-run .");

assert.equal(exitCode, 1);

});

it("should provide fix predicate when in fix-dry-run mode and quiet mode", () => {

const report = {
errorCount: 0,
warningCount: 1,
results: [{
filePath: "./foo.js",
output: "bar",
messages: [
{
severity: 1,
message: "Fake message"
}
]
}]
};

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: sinon.match.func }));

fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype);
sandbox.stub(fakeCLIEngine.prototype, "executeOnFiles").returns(report);
sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done");
fakeCLIEngine.getErrorResults = sandbox.stub().returns([]);
fakeCLIEngine.outputFixes = sandbox.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
"./logging": log
});

const exitCode = localCLI.execute("--fix-dry-run --quiet .");

assert.equal(exitCode, 0);

});

it("should allow executing on text", () => {

const report = {
errorCount: 1,
warningCount: 0,
results: [{
filePath: "./foo.js",
output: "bar",
messages: [
{
severity: 2,
message: "Fake message"
}
]
}]
};

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().withExactArgs(sinon.match({ fix: true }));

fakeCLIEngine.prototype = leche.fake(CLIEngine.prototype);
sandbox.stub(fakeCLIEngine.prototype, "executeOnText").returns(report);
sandbox.stub(fakeCLIEngine.prototype, "getFormatter").returns(() => "done");
fakeCLIEngine.outputFixes = sandbox.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
"./logging": log
});

const exitCode = localCLI.execute("--fix-dry-run .", "foo = bar;");

assert.equal(exitCode, 1);
});

it("should not call CLIEngine and return 1 when used with --fix", () => {

// create a fake CLIEngine to test with
const fakeCLIEngine = sandbox.mock().never();

localCLI = proxyquire("../../lib/cli", {
"./cli-engine": fakeCLIEngine,
"./logging": log
});

const exitCode = localCLI.execute("--fix --fix-dry-run .", "foo = bar;");

assert.equal(exitCode, 1);
});
});

describe("when passing --print-config", () => {
it("should print out the configuration", () => {
const filePath = getFixturePath("files");
Expand Down

2 comments on commit 4567ab1

@graingert
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs aren't very clear on where these fixes go

@platinumazure
Copy link
Member

@platinumazure platinumazure commented on 4567ab1 Oct 10, 2017

Choose a reason for hiding this comment

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

@graingert The fixes show up in the output for certain formatters (e.g., json). They are not applied to the filesystem at all. The idea is that editors or scripts could use the output and apply the fixes to buffers that are not files.

If you have any thoughts on how the documentation can be improved in light of this information, please feel free to create an issue or PR. Thanks!

Please sign in to comment.