Skip to content

Commit

Permalink
feat: improve flat config errors for invalid rule options and severit…
Browse files Browse the repository at this point in the history
…ies (#17140)

* feat: improve flat config errors for invalid rule options and severities

* Apply suggestions from code review

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* Updated error messages and tests per review

* Account for undefined value in printing

* More general truthiness check

---------

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
  • Loading branch information
JoshuaKGoldberg and nzakas committed May 4, 2023
1 parent f5574dc commit 5db7808
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 37 deletions.
92 changes: 57 additions & 35 deletions lib/config/flat-config-schema.js
Expand Up @@ -126,32 +126,65 @@ function normalizeRuleOptions(ruleOptions) {
// Assertions
//-----------------------------------------------------------------------------

/**
* The error type when a rule's options are configured with an invalid type.
*/
class InvalidRuleOptionsError extends Error {

/**
* @param {string} ruleId Rule name being configured.
* @param {any} value The invalid value.
*/
constructor(ruleId, value) {
super(`Key "${ruleId}": Expected severity of "off", 0, "warn", 1, "error", or 2.`);
this.messageTemplate = "invalid-rule-options";
this.messageData = { ruleId, value };
}
}

/**
* Validates that a value is a valid rule options entry.
* @param {string} ruleId Rule name being configured.
* @param {any} value The value to check.
* @returns {void}
* @throws {TypeError} If the value isn't a valid rule options.
* @throws {InvalidRuleOptionsError} If the value isn't a valid rule options.
*/
function assertIsRuleOptions(value) {

function assertIsRuleOptions(ruleId, value) {
if (typeof value !== "string" && typeof value !== "number" && !Array.isArray(value)) {
throw new TypeError("Expected a string, number, or array.");
throw new InvalidRuleOptionsError(ruleId, value);
}
}

/**
* The error type when a rule's severity is invalid.
*/
class InvalidRuleSeverityError extends Error {

/**
* @param {string} ruleId Rule name being configured.
* @param {any} value The invalid value.
*/
constructor(ruleId, value) {
super(`Key "${ruleId}": Expected severity of "off", 0, "warn", 1, "error", or 2.`);
this.messageTemplate = "invalid-rule-severity";
this.messageData = { ruleId, value };
}
}

/**
* Validates that a value is valid rule severity.
* @param {string} ruleId Rule name being configured.
* @param {any} value The value to check.
* @returns {void}
* @throws {TypeError} If the value isn't a valid rule severity.
* @throws {InvalidRuleSeverityError} If the value isn't a valid rule severity.
*/
function assertIsRuleSeverity(value) {
function assertIsRuleSeverity(ruleId, value) {
const severity = typeof value === "string"
? ruleSeverities.get(value.toLowerCase())
: ruleSeverities.get(value);

if (typeof severity === "undefined") {
throw new TypeError("Expected severity of \"off\", 0, \"warn\", 1, \"error\", or 2.");
throw new InvalidRuleSeverityError(ruleId, value);
}
}

Expand Down Expand Up @@ -357,39 +390,28 @@ const rulesSchema = {
validate(value) {
assertIsObject(value);

let lastRuleId;

// Performance: One try-catch has less overhead than one per loop iteration
try {

/*
* We are not checking the rule schema here because there is no
* guarantee that the rule definition is present at this point. Instead
* we wait and check the rule schema during the finalization step
* of calculating a config.
*/
for (const ruleId of Object.keys(value)) {

// avoid hairy edge case
if (ruleId === "__proto__") {
continue;
}
/*
* We are not checking the rule schema here because there is no
* guarantee that the rule definition is present at this point. Instead
* we wait and check the rule schema during the finalization step
* of calculating a config.
*/
for (const ruleId of Object.keys(value)) {

lastRuleId = ruleId;
// avoid hairy edge case
if (ruleId === "__proto__") {
continue;
}

const ruleOptions = value[ruleId];
const ruleOptions = value[ruleId];

assertIsRuleOptions(ruleOptions);
assertIsRuleOptions(ruleId, ruleOptions);

if (Array.isArray(ruleOptions)) {
assertIsRuleSeverity(ruleOptions[0]);
} else {
assertIsRuleSeverity(ruleOptions);
}
if (Array.isArray(ruleOptions)) {
assertIsRuleSeverity(ruleId, ruleOptions[0]);
} else {
assertIsRuleSeverity(ruleId, ruleOptions);
}
} catch (error) {
error.message = `Key "${lastRuleId}": ${error.message}`;
throw error;
}
}
};
Expand Down
17 changes: 17 additions & 0 deletions messages/invalid-rule-options.js
@@ -0,0 +1,17 @@
"use strict";

const { stringifyValueForError } = require("./shared");

module.exports = function({ ruleId, value }) {
return `
Configuration for rule "${ruleId}" is invalid. Each rule must have a severity ("off", 0, "warn", 1, "error", or 2) and may be followed by additional options for the rule.
You passed '${stringifyValueForError(value, 4)}', which doesn't contain a valid severity.
If you're attempting to configure rule options, perhaps you meant:
"${ruleId}": ["error", ${stringifyValueForError(value, 8)}]
See https://eslint.org/docs/latest/use/configure/rules#using-configuration-files for configuring rules.
`.trimStart();
};
13 changes: 13 additions & 0 deletions messages/invalid-rule-severity.js
@@ -0,0 +1,13 @@
"use strict";

const { stringifyValueForError } = require("./shared");

module.exports = function({ ruleId, value }) {
return `
Configuration for rule "${ruleId}" is invalid. Expected severity of "off", 0, "warn", 1, "error", or 2.
You passed '${stringifyValueForError(value, 4)}'.
See https://eslint.org/docs/latest/use/configure/rules#using-configuration-files for configuring rules.
`.trimStart();
};
18 changes: 18 additions & 0 deletions messages/shared.js
@@ -0,0 +1,18 @@
/**
* @fileoverview Shared utilities for error messages.
* @author Josh Goldberg
*/

"use strict";

/**
* Converts a value to a string that may be printed in errors.
* @param {any} value The invalid value.
* @param {number} indentation How many spaces to indent
* @returns {string} The value, stringified.
*/
function stringifyValueForError(value, indentation) {
return value ? JSON.stringify(value, null, 4).replace(/\n/gu, `\n${" ".repeat(indentation)}`) : `${value}`;
}

module.exports = { stringifyValueForError };
2 changes: 1 addition & 1 deletion tests/lib/config/flat-config-array.js
Expand Up @@ -1712,7 +1712,7 @@ describe("FlatConfigArray", () => {
foo: true
}
}
], "Key \"rules\": Key \"foo\": Expected a string, number, or array.");
], "Key \"rules\": Key \"foo\": Expected severity of \"off\", 0, \"warn\", 1, \"error\", or 2.");
});

it("should error when an invalid rule severity of the right type is set", async () => {
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/linter/linter.js
Expand Up @@ -8815,7 +8815,7 @@ describe("Linter with FlatConfigArray", () => {

assert.throws(() => {
linter.verify(code, config, filename, true);
}, /Key "rules": Key "semi"/u);
}, /Key "rules": Key "semi": Expected severity/u);
});

it("should process empty config", () => {
Expand Down

0 comments on commit 5db7808

Please sign in to comment.