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 all commits
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
4 changes: 2 additions & 2 deletions docs/src/developer-guide/nodejs-api.md
Expand Up @@ -410,8 +410,8 @@ This edit information means replacing the range of the `range` property by the `

The `LoadedFormatter` value is the object to convert the [LintResult] objects to text. The [eslint.loadFormatter()][eslint-loadformatter] method returns it. It has the following method:

* `format` (`(results: LintResult[]) => string | Promise<string>`)<br>
The method to convert the [LintResult] objects to text.
* `format` (`(results: LintResult[], resultsMeta: ResultsMeta) => string | Promise<string>`)<br>
The method to convert the [LintResult] objects to text. `resultsMeta` is an object that will contain a `maxWarningsExceeded` object if `--max-warnings` was set and the number of warnings exceeded the limit. The `maxWarningsExceeded` object will contain two properties: `maxWarnings`, the value of the `--max-warnings` option, and `foundWarnings`, the number of lint warnings.

---

Expand Down
9 changes: 7 additions & 2 deletions docs/src/developer-guide/working-with-custom-formatters.md
Expand Up @@ -129,16 +129,21 @@ Each `message` object contains information about the ESLint rule that was trigge

## The `context` Argument

The formatter function receives an object as the second argument. The object has two properties:
The formatter function receives an object as the second argument. The object has the following properties:

* `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
31 changes: 20 additions & 11 deletions lib/cli.js
Expand Up @@ -38,6 +38,7 @@ const debug = require("debug")("eslint:cli");
/** @typedef {import("./eslint/eslint").LintMessage} LintMessage */
/** @typedef {import("./eslint/eslint").LintResult} LintResult */
/** @typedef {import("./options").ParsedCLIOptions} ParsedCLIOptions */
/** @typedef {import("./shared/types").ResultsMeta} ResultsMeta */

//------------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -201,7 +202,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 +240,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 {ResultsMeta} 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 +254,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 +409,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 +437,7 @@ const cli = {
return 2;
}

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

return 2;
Expand Down
7 changes: 5 additions & 2 deletions lib/eslint/eslint.js
Expand Up @@ -36,11 +36,12 @@ const { version } = require("../../package.json");
/** @typedef {import("../shared/types").Plugin} Plugin */
/** @typedef {import("../shared/types").Rule} Rule */
/** @typedef {import("../shared/types").LintResult} LintResult */
/** @typedef {import("../shared/types").ResultsMeta} ResultsMeta */

/**
* The main formatter object.
* @typedef LoadedFormatter
* @property {function(LintResult[]): string | Promise<string>} format format function.
* @property {(results: LintResult[], resultsMeta: ResultsMeta) => string | Promise<string>} format format function.
*/

/**
Expand Down Expand Up @@ -625,14 +626,16 @@ class ESLint {
/**
* The main formatter method.
* @param {LintResult[]} results The lint results to format.
* @param {ResultsMeta} 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
5 changes: 4 additions & 1 deletion lib/eslint/flat-eslint.js
Expand Up @@ -59,6 +59,7 @@ const LintResultCache = require("../cli-engine/lint-result-cache");
/** @typedef {import("../shared/types").LintMessage} LintMessage */
/** @typedef {import("../shared/types").ParserOptions} ParserOptions */
/** @typedef {import("../shared/types").Plugin} Plugin */
/** @typedef {import("../shared/types").ResultsMeta} ResultsMeta */
/** @typedef {import("../shared/types").RuleConf} RuleConf */
/** @typedef {import("../shared/types").Rule} Rule */
/** @typedef {ReturnType<ConfigArray.extractConfig>} ExtractedConfig */
Expand Down Expand Up @@ -1114,14 +1115,16 @@ class FlatESLint {
/**
* The main formatter method.
* @param {LintResults[]} results The lint results to format.
* @param {ResultsMeta} 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
15 changes: 14 additions & 1 deletion lib/shared/types.js
Expand Up @@ -190,10 +190,23 @@ module.exports = {};
* @property {DeprecatedRuleInfo[]} usedDeprecatedRules The list of used deprecated rules.
*/

/**
* Information provided when the maximum warning threshold is exceeded.
* @typedef {Object} MaxWarningsExceeded
* @property {number} maxWarnings Number of warnings to trigger nonzero exit code.
* @property {number} foundWarnings Number of warnings found while linting.
*/

/**
* Metadata about results for formatters.
* @typedef {Object} ResultsMeta
* @property {MaxWarningsExceeded} [maxWarningsExceeded] Present if the maxWarnings threshold was exceeded.
*/

/**
* A formatter function.
* @callback FormatterFunction
* @param {LintResult[]} results The list of linting results.
* @param {{cwd: string, rulesMeta: Record<string, RuleMeta>}} [context] A context object.
* @param {{cwd: string, maxWarningsExceeded?: MaxWarningsExceeded, rulesMeta: Record<string, RuleMeta>}} [context] A context object.
* @returns {string | Promise<string>} Formatted text.
*/
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