Skip to content

Commit 83914ad

Browse files
nzakasmdjermanovic
nzakas
and
mdjermanovic
authored
feat: Implement SourceCode#applyInlineConfig() (#17351)
* Implement SourceCode#applyInlineConfig() * Fix lint errors * Add tests for forbidden methods * Clean up naming * Add more tests * Fix last test * Fixing some bugs -- still tests failing * Further refactoring and bug fixes * Fix test for Node.js 19 * Forgot to save the file * Remove proxy; update RuleTester/FlatRuleTester * Clean up * Use WeakSet to track method calls * Update tests/lib/rule-tester/rule-tester.js Co-authored-by: Milos Djermanovic <[email protected]> * Update tests/lib/rule-tester/rule-tester.js Co-authored-by: Milos Djermanovic <[email protected]> * Re-add tests for FlatRuleTester * Apply feedback * Cleanup tests * Update lib/source-code/source-code.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <[email protected]> * Fix inline config problems in flat config mode * Update JSON parse error message * Fix JSON parse message again * Update lib/source-code/source-code.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <[email protected]> * Update tests/lib/linter/linter.js Co-authored-by: Milos Djermanovic <[email protected]> * Apply feedback --------- Co-authored-by: Milos Djermanovic <[email protected]>
1 parent cc4d26b commit 83914ad

10 files changed

Lines changed: 1395 additions & 76 deletions

File tree

‎lib/config/flat-config-schema.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ const eslintrcKeys = [
507507
// Full schema
508508
//-----------------------------------------------------------------------------
509509

510-
exports.flatConfigSchema = {
510+
const flatConfigSchema = {
511511

512512
// eslintrc-style keys that should always error
513513
...Object.fromEntries(eslintrcKeys.map(key => [key, createEslintrcErrorSchema(key)])),
@@ -533,3 +533,13 @@ exports.flatConfigSchema = {
533533
plugins: pluginsSchema,
534534
rules: rulesSchema
535535
};
536+
537+
//-----------------------------------------------------------------------------
538+
// Exports
539+
//-----------------------------------------------------------------------------
540+
541+
module.exports = {
542+
flatConfigSchema,
543+
assertIsRuleSeverity,
544+
assertIsRuleOptions
545+
};

‎lib/linter/linter.js‎

Lines changed: 172 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ const
4242
ruleReplacements = require("../../conf/replacements.json");
4343
const { getRuleFromConfig } = require("../config/flat-config-helpers");
4444
const { FlatConfigArray } = require("../config/flat-config-array");
45-
45+
const { RuleValidator } = require("../config/rule-validator");
46+
const { assertIsRuleOptions, assertIsRuleSeverity } = require("../config/flat-config-schema");
4647
const debug = require("debug")("eslint:linter");
4748
const MAX_AUTOFIX_PASSES = 10;
4849
const DEFAULT_PARSER_NAME = "espree";
4950
const DEFAULT_ECMA_VERSION = 5;
5051
const commentParser = new ConfigCommentParser();
5152
const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, column: 1 } };
5253
const parserSymbol = Symbol.for("eslint.RuleTester.parser");
53-
const globals = require("../../conf/globals");
5454

5555
//------------------------------------------------------------------------------
5656
// Typedefs
@@ -145,29 +145,6 @@ function isEspree(parser) {
145145
return !!(parser === espree || parser[parserSymbol] === espree);
146146
}
147147

148-
/**
149-
* Retrieves globals for the given ecmaVersion.
150-
* @param {number} ecmaVersion The version to retrieve globals for.
151-
* @returns {Object} The globals for the given ecmaVersion.
152-
*/
153-
function getGlobalsForEcmaVersion(ecmaVersion) {
154-
155-
switch (ecmaVersion) {
156-
case 3:
157-
return globals.es3;
158-
159-
case 5:
160-
return globals.es5;
161-
162-
default:
163-
if (ecmaVersion < 2015) {
164-
return globals[`es${ecmaVersion + 2009}`];
165-
}
166-
167-
return globals[`es${ecmaVersion}`];
168-
}
169-
}
170-
171148
/**
172149
* Ensures that variables representing built-in properties of the Global Object,
173150
* and any globals declared by special block comments, are present in the global
@@ -361,13 +338,13 @@ function extractDirectiveComment(value) {
361338
* Parses comments in file to extract file-specific config of rules, globals
362339
* and environments and merges them with global config; also code blocks
363340
* where reporting is disabled or enabled and merges them with reporting config.
364-
* @param {ASTNode} ast The top node of the AST.
341+
* @param {SourceCode} sourceCode The SourceCode object to get comments from.
365342
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
366343
* @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.
367344
* @returns {{configuredRules: Object, enabledGlobals: {value:string,comment:Token}[], exportedVariables: Object, problems: LintMessage[], disableDirectives: DisableDirective[]}}
368345
* A collection of the directive comments that were found, along with any problems that occurred when parsing
369346
*/
370-
function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
347+
function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
371348
const configuredRules = {};
372349
const enabledGlobals = Object.create(null);
373350
const exportedVariables = {};
@@ -377,7 +354,7 @@ function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
377354
builtInRules: Rules
378355
});
379356

380-
ast.comments.filter(token => token.type !== "Shebang").forEach(comment => {
357+
sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
381358
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);
382359

383360
const match = directivesPattern.exec(directivePart);
@@ -511,6 +488,69 @@ function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
511488
};
512489
}
513490

491+
/**
492+
* Parses comments in file to extract disable directives.
493+
* @param {SourceCode} sourceCode The SourceCode object to get comments from.
494+
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
495+
* @returns {{problems: LintMessage[], disableDirectives: DisableDirective[]}}
496+
* A collection of the directive comments that were found, along with any problems that occurred when parsing
497+
*/
498+
function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper) {
499+
const problems = [];
500+
const disableDirectives = [];
501+
502+
sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
503+
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);
504+
505+
const match = directivesPattern.exec(directivePart);
506+
507+
if (!match) {
508+
return;
509+
}
510+
const directiveText = match[1];
511+
const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);
512+
513+
if (comment.type === "Line" && !lineCommentSupported) {
514+
return;
515+
}
516+
517+
if (directiveText === "eslint-disable-line" && comment.loc.start.line !== comment.loc.end.line) {
518+
const message = `${directiveText} comment should not span multiple lines.`;
519+
520+
problems.push(createLintingProblem({
521+
ruleId: null,
522+
message,
523+
loc: comment.loc
524+
}));
525+
return;
526+
}
527+
528+
const directiveValue = directivePart.slice(match.index + directiveText.length);
529+
530+
switch (directiveText) {
531+
case "eslint-disable":
532+
case "eslint-enable":
533+
case "eslint-disable-next-line":
534+
case "eslint-disable-line": {
535+
const directiveType = directiveText.slice("eslint-".length);
536+
const options = { commentToken: comment, type: directiveType, value: directiveValue, justification: justificationPart, ruleMapper };
537+
const { directives, directiveProblems } = createDisableDirectives(options);
538+
539+
disableDirectives.push(...directives);
540+
problems.push(...directiveProblems);
541+
break;
542+
}
543+
544+
// no default
545+
}
546+
});
547+
548+
return {
549+
problems,
550+
disableDirectives
551+
};
552+
}
553+
514554
/**
515555
* Normalize ECMAScript version from the initial config
516556
* @param {Parser} parser The parser which uses this options.
@@ -1313,7 +1353,7 @@ class Linter {
13131353

13141354
const sourceCode = slots.lastSourceCode;
13151355
const commentDirectives = options.allowInlineConfig
1316-
? getDirectiveComments(sourceCode.ast, ruleId => getRule(slots, ruleId), options.warnInlineConfig)
1356+
? getDirectiveComments(sourceCode, ruleId => getRule(slots, ruleId), options.warnInlineConfig)
13171357
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };
13181358

13191359
// augment global scope with declared global variables
@@ -1324,7 +1364,6 @@ class Linter {
13241364
);
13251365

13261366
const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
1327-
13281367
let lintingProblems;
13291368

13301369
try {
@@ -1540,19 +1579,6 @@ class Linter {
15401579
languageOptions.ecmaVersion
15411580
);
15421581

1543-
/*
1544-
* add configured globals and language globals
1545-
*
1546-
* using Object.assign instead of object spread for performance reasons
1547-
* https://github.com/eslint/eslint/issues/16302
1548-
*/
1549-
const configuredGlobals = Object.assign(
1550-
{},
1551-
getGlobalsForEcmaVersion(languageOptions.ecmaVersion),
1552-
languageOptions.sourceType === "commonjs" ? globals.commonjs : void 0,
1553-
languageOptions.globals
1554-
);
1555-
15561582
// double check that there is a parser to avoid mysterious error messages
15571583
if (!languageOptions.parser) {
15581584
throw new TypeError(`No parser specified for ${options.filename}`);
@@ -1608,25 +1634,113 @@ class Linter {
16081634
}
16091635

16101636
const sourceCode = slots.lastSourceCode;
1611-
const commentDirectives = options.allowInlineConfig
1612-
? getDirectiveComments(
1613-
sourceCode.ast,
1614-
ruleId => getRuleFromConfig(ruleId, config),
1615-
options.warnInlineConfig
1616-
)
1617-
: { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] };
16181637

1619-
// augment global scope with declared global variables
1620-
addDeclaredGlobals(
1621-
sourceCode.scopeManager.scopes[0],
1622-
configuredGlobals,
1623-
{ exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals }
1624-
);
1638+
/*
1639+
* Make adjustments based on the language options. For JavaScript,
1640+
* this is primarily about adding variables into the global scope
1641+
* to account for ecmaVersion and configured globals.
1642+
*/
1643+
sourceCode.applyLanguageOptions(languageOptions);
16251644

1626-
const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
1645+
const mergedInlineConfig = {
1646+
rules: {}
1647+
};
1648+
const inlineConfigProblems = [];
1649+
1650+
/*
1651+
* Inline config can be either enabled or disabled. If disabled, it's possible
1652+
* to detect the inline config and emit a warning (though this is not required).
1653+
* So we first check to see if inline config is allowed at all, and if so, we
1654+
* need to check if it's a warning or not.
1655+
*/
1656+
if (options.allowInlineConfig) {
1657+
1658+
// if inline config should warn then add the warnings
1659+
if (options.warnInlineConfig) {
1660+
sourceCode.getInlineConfigNodes().forEach(node => {
1661+
inlineConfigProblems.push(createLintingProblem({
1662+
ruleId: null,
1663+
message: `'${sourceCode.text.slice(node.range[0], node.range[1])}' has no effect because you have 'noInlineConfig' setting in ${options.warnInlineConfig}.`,
1664+
loc: node.loc,
1665+
severity: 1
1666+
}));
1667+
1668+
});
1669+
} else {
1670+
const inlineConfigResult = sourceCode.applyInlineConfig();
1671+
1672+
inlineConfigProblems.push(
1673+
...inlineConfigResult.problems
1674+
.map(createLintingProblem)
1675+
.map(problem => {
1676+
problem.fatal = true;
1677+
return problem;
1678+
})
1679+
);
1680+
1681+
// next we need to verify information about the specified rules
1682+
const ruleValidator = new RuleValidator();
1683+
1684+
for (const { config: inlineConfig, node } of inlineConfigResult.configs) {
1685+
1686+
Object.keys(inlineConfig.rules).forEach(ruleId => {
1687+
const rule = getRuleFromConfig(ruleId, config);
1688+
const ruleValue = inlineConfig.rules[ruleId];
1689+
1690+
if (!rule) {
1691+
inlineConfigProblems.push(createLintingProblem({ ruleId, loc: node.loc }));
1692+
return;
1693+
}
1694+
1695+
try {
1696+
1697+
const ruleOptions = Array.isArray(ruleValue) ? ruleValue : [ruleValue];
1698+
1699+
assertIsRuleOptions(ruleId, ruleValue);
1700+
assertIsRuleSeverity(ruleId, ruleOptions[0]);
1701+
1702+
ruleValidator.validate({
1703+
plugins: config.plugins,
1704+
rules: {
1705+
[ruleId]: ruleOptions
1706+
}
1707+
});
1708+
mergedInlineConfig.rules[ruleId] = ruleValue;
1709+
} catch (err) {
1710+
1711+
let baseMessage = err.message.slice(
1712+
err.message.startsWith("Key \"rules\":")
1713+
? err.message.indexOf(":", 12) + 1
1714+
: err.message.indexOf(":") + 1
1715+
).trim();
1716+
1717+
if (err.messageTemplate) {
1718+
baseMessage += ` You passed "${ruleValue}".`;
1719+
}
1720+
1721+
inlineConfigProblems.push(createLintingProblem({
1722+
ruleId,
1723+
message: `Inline configuration for rule "${ruleId}" is invalid:\n\t${baseMessage}\n`,
1724+
loc: node.loc
1725+
}));
1726+
}
1727+
});
1728+
}
1729+
}
1730+
}
16271731

1732+
const commentDirectives = options.allowInlineConfig && !options.warnInlineConfig
1733+
? getDirectiveCommentsForFlatConfig(
1734+
sourceCode,
1735+
ruleId => getRuleFromConfig(ruleId, config)
1736+
)
1737+
: { problems: [], disableDirectives: [] };
1738+
1739+
const configuredRules = Object.assign({}, config.rules, mergedInlineConfig.rules);
16281740
let lintingProblems;
16291741

1742+
sourceCode.finalize();
1743+
16301744
try {
16311745
lintingProblems = runRules(
16321746
sourceCode,
@@ -1667,6 +1781,7 @@ class Linter {
16671781
disableFixes: options.disableFixes,
16681782
problems: lintingProblems
16691783
.concat(commentDirectives.problems)
1784+
.concat(inlineConfigProblems)
16701785
.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column),
16711786
reportUnusedDisableDirectives: options.reportUnusedDisableDirectives
16721787
});

0 commit comments

Comments
 (0)