Skip to content

Commit

Permalink
refactor: Move directive gathering to SourceCode (#18328)
Browse files Browse the repository at this point in the history 
* feat: Move directive gathering to SourceCode

refs #16999

* Add caching

* Update lib/linter/apply-disable-directives.js

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

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
@nzakas @mdjermanovic
nzakas and mdjermanovic committed Apr 17, 2024
1 parent 0d8cf63 commit 0588fc5
Show file tree
Hide file tree
Showing 4 changed files with 357 additions and 259 deletions.
48 changes: 24 additions & 24 deletions lib/linter/apply-disable-directives.js
View file
Expand Up @@ -38,16 +38,16 @@ function compareLocations(itemA, itemB) {
* @param {Iterable<Directive>} directives Unused directives to be removed.
* @returns {Directive[][]} Directives grouped by their parent comment.
*/
function groupByParentComment(directives) {
function groupByParentDirective(directives) {
const groups = new Map();

for (const directive of directives) {
const { unprocessedDirective: { parentComment } } = directive;
const { unprocessedDirective: { parentDirective } } = directive;

if (groups.has(parentComment)) {
groups.get(parentComment).push(directive);
if (groups.has(parentDirective)) {
groups.get(parentDirective).push(directive);
} else {
groups.set(parentComment, [directive]);
groups.set(parentDirective, [directive]);
}
}

Expand All @@ -57,19 +57,19 @@ function groupByParentComment(directives) {
/**
* Creates removal details for a set of directives within the same comment.
* @param {Directive[]} directives Unused directives to be removed.
* @param {Token} commentToken The backing Comment token.
* @param {Token} node The backing Comment token.
* @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems.
*/
function createIndividualDirectivesRemoval(directives, commentToken) {
function createIndividualDirectivesRemoval(directives, node) {

/*
* `commentToken.value` starts right after `//` or `/*`.
* `node.value` starts right after `//` or `/*`.
* All calculated offsets will be relative to this index.
*/
const commentValueStart = commentToken.range[0] + "//".length;
const commentValueStart = node.range[0] + "//".length;

// Find where the list of rules starts. `\S+` matches with the directive name (e.g. `eslint-disable-line`)
const listStartOffset = /^\s*\S+\s+/u.exec(commentToken.value)[0].length;
const listStartOffset = /^\s*\S+\s+/u.exec(node.value)[0].length;

/*
* Get the list text without any surrounding whitespace. In order to preserve the original
Expand All @@ -78,7 +78,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) {
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
*/
const listText = commentToken.value
const listText = node.value
.slice(listStartOffset) // remove directive name and all whitespace before the list
.split(/\s-{2,}\s/u)[0] // remove `-- comment`, if it exists
.trimEnd(); // remove all whitespace after the list
Expand Down Expand Up @@ -159,13 +159,13 @@ function createIndividualDirectivesRemoval(directives, commentToken) {
}

/**
* Creates a description of deleting an entire unused disable comment.
* Creates a description of deleting an entire unused disable directive.
* @param {Directive[]} directives Unused directives to be removed.
* @param {Token} commentToken The backing Comment token.
* @returns {{ description, fix, unprocessedDirective }} Details for later creation of an output Problem.
* @param {Token} node The backing Comment token.
* @returns {{ description, fix, unprocessedDirective }} Details for later creation of an output problem.
*/
function createCommentRemoval(directives, commentToken) {
const { range } = commentToken;
function createDirectiveRemoval(directives, node) {
const { range } = node;
const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`);

return {
Expand All @@ -186,20 +186,20 @@ function createCommentRemoval(directives, commentToken) {
* @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems.
*/
function processUnusedDirectives(allDirectives) {
const directiveGroups = groupByParentComment(allDirectives);
const directiveGroups = groupByParentDirective(allDirectives);

return directiveGroups.flatMap(
directives => {
const { parentComment } = directives[0].unprocessedDirective;
const remainingRuleIds = new Set(parentComment.ruleIds);
const { parentDirective } = directives[0].unprocessedDirective;
const remainingRuleIds = new Set(parentDirective.ruleIds);

for (const directive of directives) {
remainingRuleIds.delete(directive.ruleId);
}

return remainingRuleIds.size
? createIndividualDirectivesRemoval(directives, parentComment.commentToken)
: [createCommentRemoval(directives, parentComment.commentToken)];
? createIndividualDirectivesRemoval(directives, parentDirective.node)
: [createDirectiveRemoval(directives, parentDirective.node)];
}
);
}
Expand Down Expand Up @@ -372,7 +372,7 @@ function applyDirectives(options) {

const unusedDirectives = processed
.map(({ description, fix, unprocessedDirective }) => {
const { parentComment, type, line, column } = unprocessedDirective;
const { parentDirective, type, line, column } = unprocessedDirective;

let message;

Expand All @@ -388,8 +388,8 @@ function applyDirectives(options) {
return {
ruleId: null,
message,
line: type === "disable-next-line" ? parentComment.commentToken.loc.start.line : line,
column: type === "disable-next-line" ? parentComment.commentToken.loc.start.column + 1 : column,
line: type === "disable-next-line" ? parentDirective.node.loc.start.line : line,
column: type === "disable-next-line" ? parentDirective.node.loc.start.column + 1 : column,
severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2,
nodeType: null,
...options.disableFixes ? {} : { fix }
Expand Down
84 changes: 27 additions & 57 deletions lib/linter/linter.js
View file
Expand Up @@ -273,49 +273,47 @@ function createLintingProblem(options) {
* Creates a collection of disable directives from a comment
* @param {Object} options to create disable directives
* @param {("disable"|"enable"|"disable-line"|"disable-next-line")} options.type The type of directive comment
* @param {token} options.commentToken The Comment token
* @param {string} options.value The value after the directive in the comment
* comment specified no specific rules, so it applies to all rules (e.g. `eslint-disable`)
* @param {string} options.justification The justification of the directive
* @param {function(string): {create: Function}} options.ruleMapper A map from rule IDs to defined rules
* @param {ASTNode|token} options.node The Comment node/token.
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
* @returns {Object} Directives and problems from the comment
*/
function createDisableDirectives(options) {
const { commentToken, type, value, justification, ruleMapper } = options;
function createDisableDirectives({ type, value, justification, node }, ruleMapper) {
const ruleIds = Object.keys(commentParser.parseListConfig(value));
const directiveRules = ruleIds.length ? ruleIds : [null];
const result = {
directives: [], // valid disable directives
directiveProblems: [] // problems in directives
};

const parentComment = { commentToken, ruleIds };
const parentDirective = { node, ruleIds };

for (const ruleId of directiveRules) {

// push to directives, if the rule is defined(including null, e.g. /*eslint enable*/)
if (ruleId === null || !!ruleMapper(ruleId)) {
if (type === "disable-next-line") {
result.directives.push({
parentComment,
parentDirective,
type,
line: commentToken.loc.end.line,
column: commentToken.loc.end.column + 1,
line: node.loc.end.line,
column: node.loc.end.column + 1,
ruleId,
justification
});
} else {
result.directives.push({
parentComment,
parentDirective,
type,
line: commentToken.loc.start.line,
column: commentToken.loc.start.column + 1,
line: node.loc.start.line,
column: node.loc.start.column + 1,
ruleId,
justification
});
}
} else {
result.directiveProblems.push(createLintingProblem({ ruleId, loc: commentToken.loc }));
result.directiveProblems.push(createLintingProblem({ ruleId, loc: node.loc }));
}
}
return result;
Expand Down Expand Up @@ -388,8 +386,12 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
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);
const { directives, directiveProblems } = createDisableDirectives({
type: directiveType,
value: directiveValue,
justification: justificationPart,
node: comment
}, ruleMapper);

disableDirectives.push(...directives);
problems.push(...directiveProblems);
Expand Down Expand Up @@ -543,53 +545,21 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
* 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 = [];
const problems = [];

sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
const { directivePart, justificationPart } = commentParser.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);
const {
directives: directivesSources,
problems: directivesProblems
} = sourceCode.getDisableDirectives();

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);
problems.push(...directivesProblems.map(createLintingProblem));

disableDirectives.push(...directives);
problems.push(...directiveProblems);
break;
}
directivesSources.forEach(directive => {
const { directives, directiveProblems } = createDisableDirectives(directive, ruleMapper);

// no default
}
disableDirectives.push(...directives);
problems.push(...directiveProblems);
});

return {
Expand Down

0 comments on commit 0588fc5

Please sign in to comment.