Skip to content

Commit

Permalink
feat: Implement SourceCode#applyInlineConfig() (#17351)
Browse files Browse the repository at this point in the history 
* 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 <milos.djermanovic@gmail.com>

* Update tests/lib/rule-tester/rule-tester.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Re-add tests for FlatRuleTester

* Apply feedback

* Cleanup tests

* Update lib/source-code/source-code.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/linter/linter.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* 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 <milos.djermanovic@gmail.com>

* Update lib/linter/linter.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/linter/linter.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update tests/lib/linter/linter.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Apply feedback

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
@nzakas @mdjermanovic
nzakas and mdjermanovic committed Sep 22, 2023
1 parent cc4d26b commit 83914ad
Show file tree
Hide file tree
Showing 10 changed files with 1,395 additions and 76 deletions.
12 changes: 11 additions & 1 deletion lib/config/flat-config-schema.js
View file
Expand Up @@ -507,7 +507,7 @@ const eslintrcKeys = [
// Full schema
//-----------------------------------------------------------------------------

exports.flatConfigSchema = {
const flatConfigSchema = {

// eslintrc-style keys that should always error
...Object.fromEntries(eslintrcKeys.map(key => [key, createEslintrcErrorSchema(key)])),
Expand All @@ -533,3 +533,13 @@ exports.flatConfigSchema = {
plugins: pluginsSchema,
rules: rulesSchema
};

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

module.exports = {
flatConfigSchema,
assertIsRuleSeverity,
assertIsRuleOptions
};
229 changes: 172 additions & 57 deletions lib/linter/linter.js
View file
Expand Up @@ -42,15 +42,15 @@ 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";
const DEFAULT_ECMA_VERSION = 5;
const commentParser = new ConfigCommentParser();
const DEFAULT_ERROR_LOC = { start: { line: 1, column: 0 }, end: { line: 1, column: 1 } };
const parserSymbol = Symbol.for("eslint.RuleTester.parser");
const globals = require("../../conf/globals");

//------------------------------------------------------------------------------
// Typedefs
Expand Down Expand Up @@ -145,29 +145,6 @@ function isEspree(parser) {
return !!(parser === espree || parser[parserSymbol] === espree);
}

/**
* Retrieves globals for the given ecmaVersion.
* @param {number} ecmaVersion The version to retrieve globals for.
* @returns {Object} The globals for the given ecmaVersion.
*/
function getGlobalsForEcmaVersion(ecmaVersion) {

switch (ecmaVersion) {
case 3:
return globals.es3;

case 5:
return globals.es5;

default:
if (ecmaVersion < 2015) {
return globals[`es${ecmaVersion + 2009}`];
}

return globals[`es${ecmaVersion}`];
}
}

/**
* Ensures that variables representing built-in properties of the Global Object,
* and any globals declared by special block comments, are present in the global
Expand Down Expand Up @@ -361,13 +338,13 @@ function extractDirectiveComment(value) {
* Parses comments in file to extract file-specific config of rules, globals
* and environments and merges them with global config; also code blocks
* where reporting is disabled or enabled and merges them with reporting config.
* @param {ASTNode} ast The top node of the AST.
* @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 getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
const configuredRules = {};
const enabledGlobals = Object.create(null);
const exportedVariables = {};
Expand All @@ -377,7 +354,7 @@ function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
builtInRules: Rules
});

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

const match = directivesPattern.exec(directivePart);
Expand Down Expand Up @@ -511,6 +488,69 @@ function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
};
}

/**
* Parses comments in file to extract disable directives.
* @param {SourceCode} sourceCode The SourceCode object to get comments from.
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
* @returns {{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) {
const problems = [];
const disableDirectives = [];

sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);

const match = directivesPattern.exec(directivePart);

if (!match) {
return;
}
const directiveText = match[1];
const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);

if (comment.type === "Line" && !lineCommentSupported) {
return;
}

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

problems.push(createLintingProblem({
ruleId: null,
message,
loc: comment.loc
}));
return;
}

const directiveValue = directivePart.slice(match.index + directiveText.length);

switch (directiveText) {
case "eslint-disable":
case "eslint-enable":
case "eslint-disable-next-line":
case "eslint-disable-line": {
const directiveType = directiveText.slice("eslint-".length);
const options = { commentToken: comment, type: directiveType, value: directiveValue, justification: justificationPart, ruleMapper };
const { directives, directiveProblems } = createDisableDirectives(options);

disableDirectives.push(...directives);
problems.push(...directiveProblems);
break;
}

// no default
}
});

return {
problems,
disableDirectives
};
}

/**
* Normalize ECMAScript version from the initial config
* @param {Parser} parser The parser which uses this options.
Expand Down Expand Up @@ -1313,7 +1353,7 @@ class Linter {

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

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

const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);

let lintingProblems;

try {
Expand Down Expand Up @@ -1540,19 +1579,6 @@ class Linter {
languageOptions.ecmaVersion
);

/*
* add configured globals and language globals
*
* using Object.assign instead of object spread for performance reasons
* https://github.com/eslint/eslint/issues/16302
*/
const configuredGlobals = Object.assign(
{},
getGlobalsForEcmaVersion(languageOptions.ecmaVersion),
languageOptions.sourceType === "commonjs" ? globals.commonjs : void 0,
languageOptions.globals
);

// double check that there is a parser to avoid mysterious error messages
if (!languageOptions.parser) {
throw new TypeError(`No parser specified for ${options.filename}`);
Expand Down Expand Up @@ -1608,25 +1634,113 @@ class Linter {
}

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

// augment global scope with declared global variables
addDeclaredGlobals(
sourceCode.scopeManager.scopes[0],
configuredGlobals,
{ exportedVariables: commentDirectives.exportedVariables, enabledGlobals: commentDirectives.enabledGlobals }
);
/*
* Make adjustments based on the language options. For JavaScript,
* this is primarily about adding variables into the global scope
* to account for ecmaVersion and configured globals.
*/
sourceCode.applyLanguageOptions(languageOptions);

const configuredRules = Object.assign({}, config.rules, commentDirectives.configuredRules);
const mergedInlineConfig = {
rules: {}
};
const inlineConfigProblems = [];

/*
* Inline config can be either enabled or disabled. If disabled, it's possible
* to detect the inline config and emit a warning (though this is not required).
* So we first check to see if inline config is allowed at all, and if so, we
* need to check if it's a warning or not.
*/
if (options.allowInlineConfig) {

// if inline config should warn then add the warnings
if (options.warnInlineConfig) {
sourceCode.getInlineConfigNodes().forEach(node => {
inlineConfigProblems.push(createLintingProblem({
ruleId: null,
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
}));

});
} else {
const inlineConfigResult = sourceCode.applyInlineConfig();

inlineConfigProblems.push(
...inlineConfigResult.problems
.map(createLintingProblem)
.map(problem => {
problem.fatal = true;
return problem;
})
);

// next we need to verify information about the specified rules
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, loc: node.loc }));
return;
}

try {

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(":") + 1
).trim();

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

inlineConfigProblems.push(createLintingProblem({
ruleId,
message: `Inline configuration for rule "${ruleId}" is invalid:\n\t${baseMessage}\n`,
loc: node.loc
}));
}
});
}
}
}

const commentDirectives = options.allowInlineConfig && !options.warnInlineConfig
? getDirectiveCommentsForFlatConfig(
sourceCode,
ruleId => getRuleFromConfig(ruleId, config)
)
: { problems: [], disableDirectives: [] };

const configuredRules = Object.assign({}, config.rules, mergedInlineConfig.rules);
let lintingProblems;

sourceCode.finalize();

try {
lintingProblems = runRules(
sourceCode,
Expand Down Expand Up @@ -1667,6 +1781,7 @@ class Linter {
disableFixes: options.disableFixes,
problems: lintingProblems
.concat(commentDirectives.problems)
.concat(inlineConfigProblems)
.sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column),
reportUnusedDisableDirectives: options.reportUnusedDisableDirectives
});
Expand Down

0 comments on commit 83914ad

Please sign in to comment.