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

feat: Pass --max-warnings value to formatters #16348

Merged
merged 3 commits into from Oct 7, 2022
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
Expand Up @@ -132,13 +132,18 @@ Each `message` object contains information about the ESLint rule that was trigge
The formatter function receives an object as the second argument. The object has two properties:
Copy link
Member

Choose a reason for hiding this comment

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

- The object has two properties:
+ The object has two or three properties:

or maybe just "the following properties"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching this! Fixed in c772b6c.


* `cwd` ... The current working directory. This value comes from the `cwd` constructor option of the [ESLint](nodejs-api#-new-eslintoptions) class.
* `maxWarningsExceeded` (optional): If `--max-warnings` was set and the number of warnings exceeded the limit, this property's value will be an object containing two properties: `maxWarnings`, the value of the `--max-warnings` option, and `foundWarnings`, the number of lint warnings.
* `rulesMeta` ... The `meta` property values of rules. See the [Working with Rules](working-with-rules) page for more information about rules.

For example, here's what the object would look like if one rule, `no-extra-semi`, had been run:

```js
{
cwd: "/path/to/cwd",
maxWarningsExceeded: {
maxWarnings: 5,
foundWarnings: 6
},
rulesMeta: {
"no-extra-semi": {
type: "suggestion",
Expand All @@ -153,7 +158,7 @@ For example, here's what the object would look like if one rule, `no-extra-semi`
unexpected: "Unnecessary semicolon."
}
}
}
},
}
```

Expand Down
30 changes: 19 additions & 11 deletions lib/cli.js
Expand Up @@ -201,7 +201,7 @@ async function translateOptions({
/**
* Count error messages.
* @param {LintResult[]} results The lint results.
* @returns {{errorCount:number;warningCount:number}} The number of error messages.
* @returns {{errorCount:number;fatalErrorCount:number,warningCount:number}} The number of error messages.
*/
function countErrors(results) {
let errorCount = 0;
Expand Down Expand Up @@ -239,10 +239,11 @@ async function isDirectory(filePath) {
* @param {LintResult[]} results The results to print.
* @param {string} format The name of the formatter to use or the path to the formatter.
* @param {string} outputFile The path for the output file.
* @param {Object} resultsMeta Warning count and max threshold.
* @returns {Promise<boolean>} True if the printing succeeds, false if not.
* @private
*/
async function printResults(engine, results, format, outputFile) {
async function printResults(engine, results, format, outputFile, resultsMeta) {
let formatter;

try {
Expand All @@ -252,7 +253,7 @@ async function printResults(engine, results, format, outputFile) {
return false;
}

const output = await formatter.format(results);
const output = await formatter.format(results, resultsMeta);

if (output) {
if (outputFile) {
Expand Down Expand Up @@ -407,17 +408,24 @@ const cli = {
resultsToPrint = ActiveESLint.getErrorResults(resultsToPrint);
}

if (await printResults(engine, resultsToPrint, options.format, options.outputFile)) {
const resultCounts = countErrors(results);
const tooManyWarnings = options.maxWarnings >= 0 && resultCounts.warningCount > options.maxWarnings;
const resultsMeta = tooManyWarnings
? {
maxWarningsExceeded: {
maxWarnings: options.maxWarnings,
foundWarnings: resultCounts.warningCount
}
}
: {};

// Errors and warnings from the original unfiltered results should determine the exit code
const { errorCount, fatalErrorCount, warningCount } = countErrors(results);
if (await printResults(engine, resultsToPrint, options.format, options.outputFile, resultsMeta)) {

const tooManyWarnings =
options.maxWarnings >= 0 && warningCount > options.maxWarnings;
// Errors and warnings from the original unfiltered results should determine the exit code
const shouldExitForFatalErrors =
options.exitOnFatalError && fatalErrorCount > 0;
options.exitOnFatalError && resultCounts.fatalErrorCount > 0;

if (!errorCount && tooManyWarnings) {
if (!resultCounts.errorCount && tooManyWarnings) {
log.error(
"ESLint found too many warnings (maximum: %s).",
options.maxWarnings
Expand All @@ -428,7 +436,7 @@ const cli = {
return 2;
}

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

return 2;
Expand Down
4 changes: 3 additions & 1 deletion lib/eslint/eslint.js
Expand Up @@ -625,14 +625,16 @@ class ESLint {
/**
* The main formatter method.
* @param {LintResult[]} results The lint results to format.
* @param {Object} resultsMeta Warning count and max threshold.
* @returns {string | Promise<string>} The formatted lint results.
*/
format(results) {
format(results, resultsMeta) {
let rulesMeta = null;

results.sort(compareResultsByFilePath);

return formatter(results, {
...resultsMeta,
get cwd() {
return options.cwd;
},
Expand Down
4 changes: 3 additions & 1 deletion lib/eslint/flat-eslint.js
Expand Up @@ -1114,14 +1114,16 @@ class FlatESLint {
/**
* The main formatter method.
* @param {LintResults[]} results The lint results to format.
* @param {Object} resultsMeta Warning count and max threshold.
* @returns {string} The formatted lint results.
*/
format(results) {
format(results, resultsMeta) {
let rulesMeta = null;

results.sort(compareResultsByFilePath);

return formatter(results, {
...resultsMeta,
cwd,
get rulesMeta() {
if (!rulesMeta) {
Expand Down
39 changes: 39 additions & 0 deletions tests/lib/cli.js
Expand Up @@ -244,6 +244,45 @@ describe("cli", () => {
});
});

describe("when the --max-warnings option is passed", () => {
const flag = useFlatConfig ? "--no-config-lookup" : "--no-eslintrc";

describe("and there are too many warnings", () => {
it(`should provide \`maxWarningsExceeded\` metadata to the formatter with configType:${configType}`, async () => {
const exit = await cli.execute(
`--no-ignore -f json-with-metadata --max-warnings 1 --rule 'quotes: warn' ${flag}`,
"'hello' + 'world';",
useFlatConfig
);

assert.strictEqual(exit, 1);

const { metadata } = JSON.parse(log.info.args[0][0]);

assert.deepStrictEqual(
metadata.maxWarningsExceeded,
{ maxWarnings: 1, foundWarnings: 2 }
);
});
});

describe("and warnings do not exceed the limit", () => {
it(`should omit \`maxWarningsExceeded\` metadata from the formatter with configType:${configType}`, async () => {
const exit = await cli.execute(
`--no-ignore -f json-with-metadata --max-warnings 1 --rule 'quotes: warn' ${flag}`,
"'hello world';",
useFlatConfig
);

assert.strictEqual(exit, 0);

const { metadata } = JSON.parse(log.info.args[0][0]);

assert.notProperty(metadata, "maxWarningsExceeded");
});
});
});

describe("when given an invalid built-in formatter name", () => {
it(`should execute with error: with configType:${configType}`, async () => {
const filePath = getFixturePath("passing.js");
Expand Down