From 1bfbefdaaf19ef32df42b89a3f5d32cff1e5b831 Mon Sep 17 00:00:00 2001 From: Antonios Katopodis <23344572+A-Katopodis@users.noreply.github.com> Date: Sat, 31 Jul 2021 00:14:39 +0200 Subject: [PATCH] New: Exit on fatal error (fixes #13711) (#14730) * Docs: Updated docs for exit-on-fatal-error * New: Added exit-on-fatal-error feature * Addressed PR feedback * Made description consistent * Updated dev docs for nodejs-api * Updated cmd docs * Added test case for 1 exit code * Fixed typos Co-authored-by: A-Katopodis --- docs/developer-guide/nodejs-api.md | 4 ++- docs/user-guide/command-line-interface.md | 5 +++ lib/cli-engine/cli-engine.js | 6 ++++ lib/cli.js | 13 ++++++-- lib/options.js | 6 ++++ tests/bin/eslint.js | 9 ++++++ .../exit-on-fatal-error/fatal-error.js | 1 + .../no-fatal-error-rule-violation.js | 3 ++ .../exit-on-fatal-error/no-fatal-error.js | 1 + tests/lib/cli-engine/cli-engine.js | 29 ++++++++++++++--- tests/lib/cli.js | 32 +++++++++++++++++++ tests/lib/eslint/eslint.js | 22 ++++++++++--- 12 files changed, 120 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/exit-on-fatal-error/fatal-error.js create mode 100644 tests/fixtures/exit-on-fatal-error/no-fatal-error-rule-violation.js create mode 100644 tests/fixtures/exit-on-fatal-error/no-fatal-error.js diff --git a/docs/developer-guide/nodejs-api.md b/docs/developer-guide/nodejs-api.md index ec53551c538..6aab5b566de 100644 --- a/docs/developer-guide/nodejs-api.md +++ b/docs/developer-guide/nodejs-api.md @@ -355,7 +355,9 @@ The `LintResult` value is the information of the linting result of each file. Th * `fixableWarningCount` (`number`)
The number of warnings that can be fixed automatically by the `fix` constructor option. * `errorCount` (`number`)
- The number of errors. This includes fixable errors. + The number of errors. This includes fixable errors and fatal errors. +* `fatalErrorCount` (`number`)
+ The number of fatal errors. * `warningCount` (`number`)
The number of warnings. This includes fixable warnings. * `output` (`string | undefined`)
diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index 99fd61eb950..4500e6ff613 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -81,6 +81,7 @@ Miscellaneous: --init Run config initialization wizard - default: false --env-info Output execution environment information - default: false --no-error-on-unmatched-pattern Prevent errors when pattern is unmatched - default: false + --exit-on-fatal-error Exit with exit code 2 in case of fatal error - default: false --debug Output debugging information -h, --help Show help -v, --version Output the version number @@ -467,6 +468,10 @@ This option outputs information about the execution environment, including the v This option prevents errors when a quoted glob pattern or `--ext` is unmatched. This will not prevent errors when your shell can't match a glob. +#### `--exit-on-fatal-error` + +This option causes ESLint to exit with exit code 2 if one or more fatal parsing errors occur. Without this option, fatal parsing errors are reported as rule violations. + #### `--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. diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js index ca298f9c356..24f6a10d14b 100644 --- a/lib/cli-engine/cli-engine.js +++ b/lib/cli-engine/cli-engine.js @@ -156,6 +156,9 @@ function calculateStatsPerFile(messages) { return messages.reduce((stat, message) => { if (message.fatal || message.severity === 2) { stat.errorCount++; + if (message.fatal) { + stat.fatalErrorCount++; + } if (message.fix) { stat.fixableErrorCount++; } @@ -168,6 +171,7 @@ function calculateStatsPerFile(messages) { return stat; }, { errorCount: 0, + fatalErrorCount: 0, warningCount: 0, fixableErrorCount: 0, fixableWarningCount: 0 @@ -183,12 +187,14 @@ function calculateStatsPerFile(messages) { function calculateStatsPerRun(results) { return results.reduce((stat, result) => { stat.errorCount += result.errorCount; + stat.fatalErrorCount += result.fatalErrorCount; stat.warningCount += result.warningCount; stat.fixableErrorCount += result.fixableErrorCount; stat.fixableWarningCount += result.fixableWarningCount; return stat; }, { errorCount: 0, + fatalErrorCount: 0, warningCount: 0, fixableErrorCount: 0, fixableWarningCount: 0 diff --git a/lib/cli.js b/lib/cli.js index f766764546c..477310da585 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -131,14 +131,16 @@ function translateOptions({ */ function countErrors(results) { let errorCount = 0; + let fatalErrorCount = 0; let warningCount = 0; for (const result of results) { errorCount += result.errorCount; + fatalErrorCount += result.fatalErrorCount; warningCount += result.warningCount; } - return { errorCount, warningCount }; + return { errorCount, fatalErrorCount, warningCount }; } /** @@ -314,9 +316,12 @@ const cli = { if (await printResults(engine, resultsToPrint, options.format, options.outputFile)) { // Errors and warnings from the original unfiltered results should determine the exit code - const { errorCount, warningCount } = countErrors(results); + const { errorCount, fatalErrorCount, warningCount } = countErrors(results); + const tooManyWarnings = options.maxWarnings >= 0 && warningCount > options.maxWarnings; + const shouldExitForFatalErrors = + options.exitOnFatalError && fatalErrorCount > 0; if (!errorCount && tooManyWarnings) { log.error( @@ -325,6 +330,10 @@ const cli = { ); } + if (shouldExitForFatalErrors) { + return 2; + } + return (errorCount || tooManyWarnings) ? 1 : 0; } diff --git a/lib/options.js b/lib/options.js index 92c140bd3c8..e3661ec81d5 100644 --- a/lib/options.js +++ b/lib/options.js @@ -290,6 +290,12 @@ module.exports = optionator({ default: "true", description: "Prevent errors when pattern is unmatched" }, + { + option: "exit-on-fatal-error", + type: "Boolean", + default: "false", + description: "Exit with exit code 2 in case of fatal error" + }, { option: "debug", type: "Boolean", diff --git a/tests/bin/eslint.js b/tests/bin/eslint.js index 7500d6611ce..4fed66aedd7 100644 --- a/tests/bin/eslint.js +++ b/tests/bin/eslint.js @@ -89,6 +89,7 @@ describe("bin/eslint.js", () => { filePath: "", messages: [], errorCount: 0, + fatalErrorCount: 0, warningCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, @@ -117,6 +118,14 @@ describe("bin/eslint.js", () => { return assertExitCode(child, 1); }); + it("has exit code 2 if a syntax error is thrown when exit-on-fatal-error is true", () => { + const child = runESLint(["--stdin", "--no-eslintrc", "--exit-on-fatal-error"]); + + child.stdin.write("This is not valid JS syntax.\n"); + child.stdin.end(); + return assertExitCode(child, 2); + }); + it("has exit code 1 if a linting error occurs", () => { const child = runESLint(["--stdin", "--no-eslintrc", "--rule", "semi:2"]); diff --git a/tests/fixtures/exit-on-fatal-error/fatal-error.js b/tests/fixtures/exit-on-fatal-error/fatal-error.js new file mode 100644 index 00000000000..53a47a5b79e --- /dev/null +++ b/tests/fixtures/exit-on-fatal-error/fatal-error.js @@ -0,0 +1 @@ +var foo 1 \ No newline at end of file diff --git a/tests/fixtures/exit-on-fatal-error/no-fatal-error-rule-violation.js b/tests/fixtures/exit-on-fatal-error/no-fatal-error-rule-violation.js new file mode 100644 index 00000000000..a23ad4a7965 --- /dev/null +++ b/tests/fixtures/exit-on-fatal-error/no-fatal-error-rule-violation.js @@ -0,0 +1,3 @@ +/*eslint no-unused-vars: "error"*/ + +var foo = 1; \ No newline at end of file diff --git a/tests/fixtures/exit-on-fatal-error/no-fatal-error.js b/tests/fixtures/exit-on-fatal-error/no-fatal-error.js new file mode 100644 index 00000000000..d5814b856aa --- /dev/null +++ b/tests/fixtures/exit-on-fatal-error/no-fatal-error.js @@ -0,0 +1 @@ +var foo = 1; \ No newline at end of file diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index 59243b0b7dc..680317b29a3 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -144,6 +144,7 @@ describe("CLIEngine", () => { assert.strictEqual(report.results.length, 1); assert.strictEqual(report.errorCount, 5); assert.strictEqual(report.warningCount, 0); + assert.strictEqual(report.fatalErrorCount, 0); assert.strictEqual(report.fixableErrorCount, 3); assert.strictEqual(report.fixableWarningCount, 0); assert.strictEqual(report.results[0].messages.length, 5); @@ -310,6 +311,7 @@ describe("CLIEngine", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var bar = foo;" @@ -317,6 +319,7 @@ describe("CLIEngine", () => { ], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -519,6 +522,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar = foo" @@ -526,6 +530,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -562,6 +567,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, output: "var bar = foothis is a syntax error." @@ -569,6 +575,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -604,6 +611,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar =" @@ -611,6 +619,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -692,6 +701,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar = foothis is a syntax error.\n return bar;" @@ -699,6 +709,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -1684,6 +1695,7 @@ describe("CLIEngine", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "true ? \"yes\" : \"no\";\n" @@ -1693,6 +1705,7 @@ describe("CLIEngine", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0 }, @@ -1713,6 +1726,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var msg = \"hi\";\nif (msg == \"hi\") {\n\n}\n" @@ -1734,6 +1748,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var msg = \"hi\" + foo;\n" @@ -5077,6 +5092,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, source: "/* eslint-disable */" @@ -5084,6 +5100,7 @@ describe("CLIEngine", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -6155,7 +6172,8 @@ describe("CLIEngine", () => { } ], source: "a == b", - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6177,7 +6195,8 @@ describe("CLIEngine", () => { fixableErrorCount: 0, fixableWarningCount: 0, messages: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6223,7 +6242,8 @@ describe("CLIEngine", () => { fixableErrorCount: 0, fixableWarningCount: 0, messages: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6258,7 +6278,8 @@ describe("CLIEngine", () => { } ], source: "a == b", - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); diff --git a/tests/lib/cli.js b/tests/lib/cli.js index d1fea182db1..dbfe03c26ea 100644 --- a/tests/lib/cli.js +++ b/tests/lib/cli.js @@ -803,6 +803,38 @@ describe("cli", () => { }); }); + describe("when given the exit-on-fatal-error flag", () => { + it("should not change exit code if no fatal errors are reported", async () => { + const filePath = getFixturePath("exit-on-fatal-error", "no-fatal-error.js"); + const exitCode = await cli.execute(`--no-ignore --exit-on-fatal-error ${filePath}`); + + assert.strictEqual(exitCode, 0); + }); + + it("should exit with exit code 1 if no fatal errors are found, but rule violations are found", async () => { + const filePath = getFixturePath("exit-on-fatal-error", "no-fatal-error-rule-violation.js"); + const exitCode = await cli.execute(`--no-ignore --exit-on-fatal-error ${filePath}`); + + assert.strictEqual(exitCode, 1); + }); + + it("should exit with exit code 2 if fatal error is found", async () => { + const filePath = getFixturePath("exit-on-fatal-error", "fatal-error.js"); + const exitCode = await cli.execute(`--no-ignore --exit-on-fatal-error ${filePath}`); + + assert.strictEqual(exitCode, 2); + }); + + it("should exit with exit code 2 if fatal error is found in any file", async () => { + const filePath = getFixturePath("exit-on-fatal-error"); + const exitCode = await cli.execute(`--no-ignore --exit-on-fatal-error ${filePath}`); + + assert.strictEqual(exitCode, 2); + }); + + + }); + describe("when passed --no-inline-config", () => { let localCLI; diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index c5a1b7360c0..aa35b842eef 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -397,6 +397,7 @@ describe("ESLint", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var bar = foo;", @@ -597,6 +598,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar = foo", @@ -636,6 +638,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, output: "var bar = foothis is a syntax error.", @@ -674,6 +677,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar =", @@ -761,6 +765,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 1, fixableErrorCount: 0, fixableWarningCount: 0, source: "var bar = foothis is a syntax error.\n return bar;", @@ -1658,6 +1663,7 @@ describe("ESLint", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "true ? \"yes\" : \"no\";\n", @@ -1668,6 +1674,7 @@ describe("ESLint", () => { messages: [], errorCount: 0, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, usedDeprecatedRules: [] @@ -1689,6 +1696,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var msg = \"hi\";\nif (msg == \"hi\") {\n\n}\n", @@ -1711,6 +1719,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, output: "var msg = \"hi\" + foo;\n", @@ -5006,6 +5015,7 @@ describe("ESLint", () => { ], errorCount: 1, warningCount: 0, + fatalErrorCount: 0, fixableErrorCount: 0, fixableWarningCount: 0, source: "/* eslint-disable */", @@ -6060,7 +6070,8 @@ describe("ESLint", () => { ], source: "a == b", usedDeprecatedRules: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6083,7 +6094,8 @@ describe("ESLint", () => { fixableWarningCount: 0, messages: [], usedDeprecatedRules: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6130,7 +6142,8 @@ describe("ESLint", () => { fixableWarningCount: 0, messages: [], usedDeprecatedRules: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); }); @@ -6166,7 +6179,8 @@ describe("ESLint", () => { ], source: "a == b", usedDeprecatedRules: [], - warningCount: 0 + warningCount: 0, + fatalErrorCount: 0 } ]); });