Skip to content

Commit

Permalink
Further refactoring and bug fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@nzakas
nzakas committed Jul 20, 2023
1 parent 0c7df35 commit 29fcc1d
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 76 deletions.
10 changes: 10 additions & 0 deletions lib/config/flat-config-schema.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,3 +512,13 @@ exports.flatConfigSchema = {
plugins: pluginsSchema,
rules: rulesSchema
};

//-----------------------------------------------------------------------------
// Exports
//-----------------------------------------------------------------------------

module.exports = {
flatConfigSchema,
assertIsRuleSeverity,
assertIsRuleOptions
};
149 changes: 86 additions & 63 deletions lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ const
ruleReplacements = require("../../conf/replacements.json");
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 debug = require("debug")("eslint:linter");
const MAX_AUTOFIX_PASSES = 10;
const DEFAULT_PARSER_NAME = "espree";
Expand Down Expand Up @@ -493,19 +494,15 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
* where reporting is disabled or enabled and merges them with reporting config.
* @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.
* @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 getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper, warnInlineConfig) {
function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper) {
const configuredRules = {};
const enabledGlobals = Object.create(null);
const exportedVariables = {};
const problems = [];
const disableDirectives = [];
const validator = new ConfigValidator({
builtInRules: Rules
});

sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);
Expand All @@ -522,18 +519,6 @@ function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper, warnInlineCon
return;
}

if (warnInlineConfig) {
const kind = comment.type === "Block" ? `/*${directiveText}*/` : `//${directiveText}`;

problems.push(createLintingProblem({
ruleId: null,
message: `'${kind}' has no effect because you have 'noInlineConfig' setting in ${warnInlineConfig}.`,
loc: comment.loc,
severity: 1
}));
return;
}

if (directiveText === "eslint-disable-line" && comment.loc.start.line !== comment.loc.end.line) {
const message = `${directiveText} comment should not span multiple lines.`;

Expand Down Expand Up @@ -561,40 +546,54 @@ function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper, warnInlineCon
break;
}

// case "eslint": {
// const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);

// if (parseResult.success) {
// Object.keys(parseResult.config).forEach(name => {
// const rule = ruleMapper(name);
// const ruleValue = parseResult.config[name];

// if (!rule) {
// problems.push(createLintingProblem({ ruleId: name, loc: comment.loc }));
// return;
// }

// try {
// validator.validateRuleOptions(rule, name, ruleValue);
// } catch (err) {
// problems.push(createLintingProblem({
// ruleId: name,
// message: err.message,
// loc: comment.loc
// }));

// // do not apply the config, if found invalid options.
// return;
// }

// configuredRules[name] = ruleValue;
// });
// } else {
// problems.push(parseResult.error);
// }

// break;
// }
/*
* case "eslint": {
* const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);
*/

/*
* if (parseResult.success) {
* Object.keys(parseResult.config).forEach(name => {
* const rule = ruleMapper(name);
* const ruleValue = parseResult.config[name];
*/

/*
* if (!rule) {
* problems.push(createLintingProblem({ ruleId: name, loc: comment.loc }));
* return;
* }
*/

/*
* try {
* validator.validateRuleOptions(rule, name, ruleValue);
* } catch (err) {
* problems.push(createLintingProblem({
* ruleId: name,
* message: err.message,
* loc: comment.loc
* }));
*/

/*
* // do not apply the config, if found invalid options.
* return;
* }
*/

/*
* configuredRules[name] = ruleValue;
* });
* } else {
* problems.push(parseResult.error);
* }
*/

/*
* break;
* }
*/

// no default
}
Expand Down Expand Up @@ -1738,11 +1737,11 @@ class Linter {

// if inline config should warn then add the warnings
if (options.warnInlineConfig) {
sourceCode.getInlineConfigNodes().forEach(comment => {
sourceCode.getInlineConfigNodes().forEach(node => {
inlineConfigProblems.push(createLintingProblem({
ruleId: null,
message: `'${comment.value}' has no effect because you have 'noInlineConfig' setting in ${options.warnInlineConfig}.`,
loc: comment.loc,
message: `'${sourceCode.text.slice(node.range[0], node.range[1])}' has no effect because you have 'noInlineConfig' setting in ${options.warnInlineConfig}.`,
loc: node.loc,
severity: 1
}));

Expand All @@ -1754,25 +1753,50 @@ class Linter {

// need to verify rule existence options
if (inlineConfigResult.ok) {
for (const { config: inlineConfig, comment } of inlineConfigResult.configs) {

const ruleValidator = new RuleValidator();

for (const { config: inlineConfig, node } of inlineConfigResult.configs) {

Object.keys(inlineConfig.rules).forEach(ruleId => {
const rule = getRuleFromConfig(ruleId, config);
const ruleValue = inlineConfig.rules[ruleId];

if (!rule) {
inlineConfigProblems.push(createLintingProblem({ ruleId: ruleId, loc: comment.loc }));
inlineConfigProblems.push(createLintingProblem({ ruleId, loc: node.loc }));
return;
}

try {
validator.validateRuleOptions(rule, ruleId, ruleValue);

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

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

ruleValidator.validate({
plugins: config.plugins,
rules: {
[ruleId]: ruleOptions
}
});
mergedInlineConfig.rules[ruleId] = ruleValue;
} catch (err) {

let baseMessage = err.message.slice(
err.message.startsWith("Key \"rules\":")
? err.message.indexOf(":", 12) + 1
: err.message.indexOf(":")
).trim();

if (err.messageTemplate) {
baseMessage += ` You passed "${ruleValue}".`;
}

inlineConfigProblems.push(createLintingProblem({
ruleId: ruleId,
message: err.message,
loc: comment.loc
ruleId,
message: `Inline configuration for rule "${ruleId}" is invalid:\n\t${baseMessage}\n`,
loc: node.loc
}));
}
});
Expand All @@ -1784,8 +1808,7 @@ class Linter {
const commentDirectives = options.allowInlineConfig
? getDirectiveCommentsForFlatConfig(
sourceCode,
ruleId => getRuleFromConfig(ruleId, config),
options.warnInlineConfig
ruleId => getRuleFromConfig(ruleId, config)
)
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };

Expand Down
6 changes: 3 additions & 3 deletions lib/source-code/source-code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ class SourceCode extends TokenStore {
/**
* Applies configuration found inside of the source code. This method is only
* called when ESLint is running with inline configuration allowed.
* @returns {{ok:boolean,problems:Array<Problem>,configs:{config:FlatConfigArray,comment:ASTNode}}} Information
* @returns {{ok:boolean,problems:Array<Problem>,configs:{config:FlatConfigArray,node:ASTNode}}} Information
* that ESLint needs to further process the inline configuration.
*/
applyInlineConfig() {
Expand Down Expand Up @@ -1019,8 +1019,8 @@ class SourceCode extends TokenStore {
config: {
rules: parseResult.config
},
comment
})
node: comment
});
} else {
problems.push(parseResult.error);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/eslint/flat-eslint.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3531,7 +3531,7 @@ describe("FlatESLint", () => {
const messages = results[0].messages;

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].message, "'/*globals*/' has no effect because you have 'noInlineConfig' setting in your config.");
assert.strictEqual(messages[0].message, "'/* globals foo */' has no effect because you have 'noInlineConfig' setting in your config.");
});

});
Expand Down
12 changes: 6 additions & 6 deletions tests/lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11880,7 +11880,7 @@ describe("Linter with FlatConfigArray", () => {
assert.strictEqual(messages[0].message, "Unexpected alert.");
assert.include(messages[0].nodeType, "CallExpression");

assert.strictEqual(suppressedMessages.length, 0,"Incorrect suppressed message length");
assert.strictEqual(suppressedMessages.length, 0, "Incorrect suppressed message length");
});

it("rules should not change initial config", () => {
Expand Down Expand Up @@ -11968,7 +11968,7 @@ describe("Linter with FlatConfigArray", () => {
});

describe("when evaluating code with invalid comments to enable rules", () => {
it.only("should report a violation when the config is not a valid rule configuration", () => {
it("should report a violation when the config is not a valid rule configuration", () => {
const messages = linter.verify("/*eslint no-alert:true*/ alert('test');", {});
const suppressedMessages = linter.getSuppressedMessages();

Expand All @@ -11978,7 +11978,7 @@ describe("Linter with FlatConfigArray", () => {
{
severity: 2,
ruleId: "no-alert",
message: "Configuration for rule \"no-alert\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed 'true').\n",
message: "Inline configuration for rule \"no-alert\" is invalid:\n\t: Expected severity of \"off\", 0, \"warn\", 1, \"error\", or 2. You passed \"true\".\n",
line: 1,
column: 1,
endLine: 1,
Expand All @@ -12001,7 +12001,7 @@ describe("Linter with FlatConfigArray", () => {
{
severity: 2,
ruleId: "no-alert",
message: "Configuration for rule \"no-alert\" is invalid:\n\tValue [{\"nonExistentPropertyName\":true}] should NOT have more than 0 items.\n",
message: "Inline configuration for rule \"no-alert\" is invalid:\n\tValue [{\"nonExistentPropertyName\":true}] should NOT have more than 0 items.\n",
line: 1,
column: 1,
endLine: 1,
Expand Down Expand Up @@ -14025,7 +14025,7 @@ var a = "test2";
assert.deepStrictEqual(messages[0].fatal, void 0);
assert.deepStrictEqual(messages[0].ruleId, null);
assert.deepStrictEqual(messages[0].severity, 1);
assert.deepStrictEqual(messages[0].message, `'/*${directive.split(" ")[0]}*/' has no effect because you have 'noInlineConfig' setting in your config.`);
assert.deepStrictEqual(messages[0].message, `'/* ${directive} */' has no effect because you have 'noInlineConfig' setting in your config.`);

assert.strictEqual(suppressedMessages.length, 0);
});
Expand All @@ -14048,7 +14048,7 @@ var a = "test2";
assert.deepStrictEqual(messages[0].fatal, void 0);
assert.deepStrictEqual(messages[0].ruleId, null);
assert.deepStrictEqual(messages[0].severity, 1);
assert.deepStrictEqual(messages[0].message, `'//${directive.split(" ")[0]}' has no effect because you have 'noInlineConfig' setting in your config.`);
assert.deepStrictEqual(messages[0].message, `'// ${directive}' has no effect because you have 'noInlineConfig' setting in your config.`);

assert.strictEqual(suppressedMessages.length, 0);
});
Expand Down
21 changes: 18 additions & 3 deletions tests/lib/source-code/source-code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4007,7 +4007,8 @@ describe("SourceCode", () => {
const result = sourceCode.applyInlineConfig();

assert.isTrue(result.ok);
assert.strictEqual(result.config.rules["some-rule"], 2);
assert.strictEqual(result.configs.length, 1);
assert.strictEqual(result.configs[0].config.rules["some-rule"], 2);
});

it("should extract multiple rule configurations", () => {
Expand All @@ -4018,8 +4019,22 @@ describe("SourceCode", () => {
const result = sourceCode.applyInlineConfig();

assert.isTrue(result.ok);
assert.strictEqual(result.config.rules["some-rule"], 2);
assert.deepStrictEqual(result.config.rules["other-rule"], ["error", { skip: true }]);
assert.strictEqual(result.configs.length, 1);
assert.strictEqual(result.configs[0].config.rules["some-rule"], 2);
assert.deepStrictEqual(result.configs[0].config.rules["other-rule"], ["error", { skip: true }]);
});

it("should extract multiple comments into multiple configurations", () => {

const code = "/*eslint some-rule: 2*/ /*eslint other-rule: [\"error\", { skip: true }] */ var foo;";
const ast = espree.parse(code, DEFAULT_CONFIG);
const sourceCode = new SourceCode(code, ast);
const result = sourceCode.applyInlineConfig();

assert.isTrue(result.ok);
assert.strictEqual(result.configs.length, 2);
assert.strictEqual(result.configs[0].config.rules["some-rule"], 2);
assert.deepStrictEqual(result.configs[1].config.rules["other-rule"], ["error", { skip: true }]);
});

it("should report problem with rule configuration parsing", () => {
Expand Down

0 comments on commit 29fcc1d

Please sign in to comment.