Skip to content

Commit

Permalink
feat: Pass --max-warnings value to formatters (#16348)
Browse files Browse the repository at this point in the history 
* feat: Pass --max-warnings value to formatters

Fixes #14881

* Make formatter docs wording consistent

Originally suggested by @fasttime in
#16348 (comment).

* Update LoadedFormatter and FormatterFunction typedefs

Originally suggested by @fasttime in
#16348 (comment).

Internally, I define two types, `MaxWarningsExceeded` and `ResultsMeta`,
that the updated `LoadedFormatter` and `FormatterFunction` types can
use. I'm then able to reuse the `ResultsMeta` type instead of the
generic `Object` type in `ESLint` and `FlatESLint`'s `format` wrapper
functions.

Externally in the Node.js API docs, I describe the new second argument
to `LoadedFormatter`'s `format` method inline rather than separately
documenting the new types.

While working on this, I noticed that `FlatESLint` is using the
pre-PR #15727 `Formatter` type rather than `LoadedFormatter` and
`FormatterFunction`. I'll fix that in a follow-up PR to keep this PR
scoped to passing `maxWarningsExceeded` info.
  • Loading branch information
@btmills
btmills committed Oct 7, 2022
1 parent 8476a9b commit 173e820
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 19 deletions.
4 changes: 2 additions & 2 deletions docs/src/developer-guide/nodejs-api.md
View file
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
View file
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
View file
Expand Up @@ -37,6 +37,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 @@ -200,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 @@ -238,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 {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 @@ -251,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 @@ -406,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 @@ -427,7 +436,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
View file
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
View file
Expand Up @@ -57,6 +57,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 @@ -1070,14 +1071,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
View file
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
View file
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

0 comments on commit 173e820

Please sign in to comment.