Skip to content

Commit

Permalink
fix: validate options when comment with just severity enables rule (#…
Browse files Browse the repository at this point in the history 
…18133)
  • Loading branch information
@mdjermanovic
mdjermanovic committed Feb 23, 2024
1 parent 7db5bb2 commit 0cb4914
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 122 deletions.
215 changes: 105 additions & 110 deletions lib/linter/linter.js
View file
Expand Up @@ -43,7 +43,7 @@ const
const { getRuleFromConfig } = require("../config/flat-config-helpers");
const { FlatConfigArray } = require("../config/flat-config-array");
const { RuleValidator } = require("../config/rule-validator");
const { assertIsRuleOptions, assertIsRuleSeverity } = require("../config/flat-config-schema");
const { assertIsRuleSeverity } = require("../config/flat-config-schema");
const { normalizeSeverityToString } = require("../shared/severity");
const debug = require("debug")("eslint:linter");
const MAX_AUTOFIX_PASSES = 10;
Expand Down Expand Up @@ -326,10 +326,11 @@ function createDisableDirectives(options) {
* @param {SourceCode} sourceCode The SourceCode object to get comments from.
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
* @param {string|null} warnInlineConfig If a string then it should warn directive comments as disabled. The string value is the config name what the setting came from.
* @param {ConfigData} config Provided config.
* @returns {{configuredRules: Object, enabledGlobals: {value:string,comment:Token}[], exportedVariables: Object, problems: LintMessage[], disableDirectives: DisableDirective[]}}
* A collection of the directive comments that were found, along with any problems that occurred when parsing
*/
function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config) {
const configuredRules = {};
const enabledGlobals = Object.create(null);
const exportedVariables = {};
Expand Down Expand Up @@ -438,8 +439,50 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
return;
}

let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];

/*
* If the rule was already configured, inline rule configuration that
* only has severity should retain options from the config and just override the severity.
*
* Example:
*
* {
* rules: {
* curly: ["error", "multi"]
* }
* }
*
* /* eslint curly: ["warn"] * /
*
* Results in:
*
* curly: ["warn", "multi"]
*/
if (

/*
* If inline config for the rule has only severity
*/
ruleOptions.length === 1 &&

/*
* And the rule was already configured
*/
config.rules && Object.hasOwn(config.rules, name)
) {

/*
* Then use severity from the inline config and options from the provided config
*/
ruleOptions = [
ruleOptions[0], // severity from the inline config
...Array.isArray(config.rules[name]) ? config.rules[name].slice(1) : [] // options from the provided config
];
}

try {
validator.validateRuleOptions(rule, name, ruleValue);
validator.validateRuleOptions(rule, name, ruleOptions);
} catch (err) {

/*
Expand All @@ -460,7 +503,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
return;
}

configuredRules[name] = ruleValue;
configuredRules[name] = ruleOptions;
});
} else {
problems.push(parseResult.error);
Expand Down Expand Up @@ -1322,7 +1365,7 @@ class Linter {

const sourceCode = slots.lastSourceCode;
const commentDirectives = options.allowInlineConfig
? getDirectiveComments(sourceCode, ruleId => getRule(slots, ruleId), options.warnInlineConfig)
? getDirectiveComments(sourceCode, ruleId => getRule(slots, ruleId), options.warnInlineConfig, config)
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };

// augment global scope with declared global variables
Expand All @@ -1332,56 +1375,8 @@ class Linter {
{ exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals }
);

/*
* Now we determine the final configurations for rules.
* First, let all inline rule configurations override those from the config.
* Then, check for a special case: if a rule is configured in both places,
* inline rule configuration that only has severity should retain options from
* the config and just override the severity.
*
* Example:
*
* {
* rules: {
* curly: ["error", "multi"]
* }
* }
*
* /* eslint curly: ["warn"] * /
*
* Results in:
*
* curly: ["warn", "multi"]
*/
const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);

if (config.rules) {
for (const [ruleId, ruleInlineConfig] of Object.entries(commentDirectives.configuredRules)) {
if (

/*
* If inline config for the rule has only severity
*/
(!Array.isArray(ruleInlineConfig) || ruleInlineConfig.length === 1) &&

/*
* And provided config for the rule has options
*/
Object.hasOwn(config.rules, ruleId) &&
(Array.isArray(config.rules[ruleId]) && config.rules[ruleId].length > 1)
) {

/*
* Then use severity from the inline config and options from the provided config
*/
configuredRules[ruleId] = [
Array.isArray(ruleInlineConfig) ? ruleInlineConfig[0] : ruleInlineConfig, // severity from the inline config
...config.rules[ruleId].slice(1) // options from the provided config
];
}
}
}

let lintingProblems;

try {
Expand Down Expand Up @@ -1713,17 +1708,67 @@ class Linter {

try {

const ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
let ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];

assertIsRuleOptions(ruleId, ruleValue);
assertIsRuleSeverity(ruleId, ruleOptions[0]);

ruleValidator.validate({
plugins: config.plugins,
rules: {
[ruleId]: ruleOptions
/*
* If the rule was already configured, inline rule configuration that
* only has severity should retain options from the config and just override the severity.
*
* Example:
*
* {
* rules: {
* curly: ["error", "multi"]
* }
* }
*
* /* eslint curly: ["warn"] * /
*
* Results in:
*
* curly: ["warn", "multi"]
*/

let shouldValidateOptions = true;

if (

/*
* If inline config for the rule has only severity
*/
ruleOptions.length === 1 &&

/*
* And the rule was already configured
*/
config.rules && Object.hasOwn(config.rules, ruleId)
) {

/*
* Then use severity from the inline config and options from the provided config
*/
ruleOptions = [
ruleOptions[0], // severity from the inline config
...config.rules[ruleId].slice(1) // options from the provided config
];

// if the rule was enabled, the options have already been validated
if (config.rules[ruleId][0] > 0) {
shouldValidateOptions = false;
}
});
}

if (shouldValidateOptions) {
ruleValidator.validate({
plugins: config.plugins,
rules: {
[ruleId]: ruleOptions
}
});
}

mergedInlineConfig.rules[ruleId] = ruleOptions;
} catch (err) {

Expand Down Expand Up @@ -1763,58 +1808,8 @@ class Linter {
)
: { problems: [], disableDirectives: [] };

/*
* Now we determine the final configurations for rules.
* First, let all inline rule configurations override those from the config.
* Then, check for a special case: if a rule is configured in both places,
* inline rule configuration that only has severity should retain options from
* the config and just override the severity.
*
* Example:
*
* {
* rules: {
* curly: ["error", "multi"]
* }
* }
*
* /* eslint curly: ["warn"] * /
*
* Results in:
*
* curly: ["warn", "multi"]
*
* At this point, all rule configurations are normalized to arrays.
*/
const configuredRules = Object.assign({}, config.rules, mergedInlineConfig.rules);

if (config.rules) {
for (const [ruleId, ruleInlineConfig] of Object.entries(mergedInlineConfig.rules)) {
if (

/*
* If inline config for the rule has only severity
*/
ruleInlineConfig.length === 1 &&

/*
* And provided config for the rule has options
*/
Object.hasOwn(config.rules, ruleId) &&
config.rules[ruleId].length > 1
) {

/*
* Then use severity from the inline config and options from the provided config
*/
configuredRules[ruleId] = [
ruleInlineConfig[0], // severity from the inline config
...config.rules[ruleId].slice(1) // options from the provided config
];
}
}
}

let lintingProblems;

sourceCode.finalize();
Expand Down

0 comments on commit 0cb4914

Please sign in to comment.