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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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
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.`);
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -8811,7 +8811,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