Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking: use an exit code of 2 for fatal config problems (fixes #9384) #10009

Merged
merged 2 commits into from Mar 22, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/eslint.js
Expand Up @@ -53,7 +53,7 @@ process.once("uncaughtException", err => {
console.error(err.stack);
}

process.exitCode = 1;
process.exitCode = 2;
});

if (useStdIn) {
Expand Down
8 changes: 8 additions & 0 deletions docs/user-guide/command-line-interface.md
Expand Up @@ -450,3 +450,11 @@ ESLint supports `.eslintignore` files to exclude files from the linting process
**/vendor/*.js

A more detailed breakdown of supported patterns and directories ESLint ignores by default can be found in [Configuring ESLint](configuring.md#ignoring-files-and-directories).

## Exit codes

ESLint will exit with one of the following exit codes:

* `0`: Linting was successful and there are no linting errors. If the `--max-warnings` flag is set to `n`, the number of linting warnings is at most `n`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is valuable but I'm throwing it out anyway: Should we explicitly note that --help or --version returns exit code of 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is also the case for other flags like --init and --print-config. Maybe I could add a qualifier that these exit codes only apply when ESLint is used to lint files.

* `1`: Linting was successful and there is at least one linting error, or there are more linting warnings than allowed by the `--max-warnings` option.
* `2`: Linting was unsuccessful due to a configuration problem or an internal error.
12 changes: 6 additions & 6 deletions lib/cli.js
Expand Up @@ -139,7 +139,7 @@ const cli = {
currentOptions = options.parse(args);
} catch (error) {
log.error(error.message);
return 1;
return 2;
}

const files = currentOptions._;
Expand All @@ -153,11 +153,11 @@ const cli = {
} else if (currentOptions.printConfig) {
if (files.length) {
log.error("The --print-config option must be used with exactly one file name.");
return 1;
return 2;
}
if (useStdin) {
log.error("The --print-config option is not available for piped-in code.");
return 1;
return 2;
}

const engine = new CLIEngine(translateOptions(currentOptions));
Expand All @@ -176,12 +176,12 @@ const cli = {

if (currentOptions.fix && currentOptions.fixDryRun) {
log.error("The --fix option and the --fix-dry-run option cannot be used together.");
return 1;
return 2;
}

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

const engine = new CLIEngine(translateOptions(currentOptions));
Expand All @@ -207,7 +207,7 @@ const cli = {

return (report.errorCount || tooManyWarnings) ? 1 : 0;
}
return 1;
return 2;


}
Expand Down
6 changes: 3 additions & 3 deletions tests/bin/eslint.js
Expand Up @@ -138,7 +138,7 @@ describe("bin/eslint.js", () => {
["--stdin"], { cwd: "/", env: { HOME: "/" } }
);

const exitCodePromise = assertExitCode(child, 1);
const exitCodePromise = assertExitCode(child, 2);
const stderrPromise = getOutput(child).then(output => {
assert.match(
output.stderr,
Expand Down Expand Up @@ -316,7 +316,7 @@ describe("bin/eslint.js", () => {
describe("handling crashes", () => {
it("prints the error message to stderr in the event of a crash", () => {
const child = runESLint(["--rule=no-restricted-syntax:[error, 'Invalid Selector [[[']", "Makefile.js"]);
const exitCodeAssertion = assertExitCode(child, 1);
const exitCodeAssertion = assertExitCode(child, 2);
const outputAssertion = getOutput(child).then(output => {
const expectedSubstring = "Syntax error in selector";

Expand All @@ -330,7 +330,7 @@ describe("bin/eslint.js", () => {
it("prints the error message pointing to line of code", () => {
const invalidConfig = `${__dirname}/../fixtures/bin/.eslintrc.yml`;
const child = runESLint(["--no-ignore", invalidConfig]);
const exitCodeAssertion = assertExitCode(child, 1);
const exitCodeAssertion = assertExitCode(child, 2);
const outputAssertion = getOutput(child).then(output => {
const expectedSubstring = "Error: bad indentation of a mapping entry at line";

Expand Down
28 changes: 14 additions & 14 deletions tests/lib/cli.js
Expand Up @@ -121,7 +121,7 @@ describe("cli", () => {
const filePath = getFixturePath("files");
const result = cli.execute(`--blah --another ${filePath}`);

assert.strictEqual(result, 1);
assert.strictEqual(result, 2);
});

});
Expand Down Expand Up @@ -245,7 +245,7 @@ describe("cli", () => {
const filePath = getFixturePath("passing.js");
const exit = cli.execute(`-f fakeformatter ${filePath}`);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
});
});

Expand All @@ -265,14 +265,14 @@ describe("cli", () => {
const filePath = getFixturePath("passing.js");
const exit = cli.execute(`-f ${formatterPath} ${filePath}`);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
});
});

describe("when executing a file with a lint error", () => {
it("should exit with error", () => {
const filePath = getFixturePath("undef.js");
const code = `--no-ignore --config --rule no-undef:2 ${filePath}`;
const code = `--no-ignore --rule no-undef:2 ${filePath}`;

const exit = cli.execute(code);

Expand Down Expand Up @@ -308,15 +308,15 @@ describe("cli", () => {

describe("when executing with version flag", () => {
it("should print out current version", () => {
cli.execute("-v");
assert.strictEqual(cli.execute("-v"), 0);

assert.strictEqual(log.info.callCount, 1);
});
});

describe("when executing with help flag", () => {
it("should print out help", () => {
cli.execute("-h");
assert.strictEqual(cli.execute("-h"), 0);

assert.strictEqual(log.info.callCount, 1);
});
Expand Down Expand Up @@ -403,7 +403,7 @@ describe("cli", () => {
assert.throws(() => {
const exit = cli.execute(code);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
}, /Error while loading rule 'custom-rule': Cannot read property/);
});

Expand Down Expand Up @@ -559,7 +559,7 @@ describe("cli", () => {

const exit = cli.execute(code);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
assert.isTrue(log.info.notCalled);
assert.isTrue(log.error.calledOnce);
});
Expand All @@ -572,7 +572,7 @@ describe("cli", () => {

const exit = cli.execute(code);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
assert.isTrue(log.info.notCalled);
assert.isTrue(log.error.calledOnce);
});
Expand Down Expand Up @@ -611,7 +611,7 @@ describe("cli", () => {
const filePath = getFixturePath("passing.js");
const exit = cli.execute(`--no-ignore --parser-options test111 ${filePath}`);

assert.strictEqual(exit, 1);
assert.strictEqual(exit, 2);
});

it("should exit with no error if parser is valid", () => {
Expand Down Expand Up @@ -862,7 +862,7 @@ describe("cli", () => {

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

assert.strictEqual(exitCode, 1);
assert.strictEqual(exitCode, 2);
});

});
Expand Down Expand Up @@ -1020,7 +1020,7 @@ describe("cli", () => {

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

assert.strictEqual(exitCode, 1);
assert.strictEqual(exitCode, 2);
});
});

Expand All @@ -1042,15 +1042,15 @@ describe("cli", () => {

assert.isTrue(log.info.notCalled);
assert.isTrue(log.error.calledOnce);
assert.strictEqual(exitCode, 1);
assert.strictEqual(exitCode, 2);
});

it("should error out when executing on text", () => {
const exitCode = cli.execute("--print-config=myFile.js", "foo = bar;");

assert.isTrue(log.info.notCalled);
assert.isTrue(log.error.calledOnce);
assert.strictEqual(exitCode, 1);
assert.strictEqual(exitCode, 2);
});
});

Expand Down