Skip to content

Commit 0588fc5

Browse files
nzakasmdjermanovic
andauthored
refactor: Move directive gathering to SourceCode (#18328)
* 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>
1 parent 0d8cf63 commit 0588fc5

4 files changed

Lines changed: 357 additions & 259 deletions

File tree

‎lib/linter/apply-disable-directives.js‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ function compareLocations(itemA, itemB) {
3838
* @param {Iterable<Directive>} directives Unused directives to be removed.
3939
* @returns {Directive[][]} Directives grouped by their parent comment.
4040
*/
41-
function groupByParentComment(directives) {
41+
function groupByParentDirective(directives) {
4242
const groups = new Map();
4343

4444
for (const directive of directives) {
45-
const { unprocessedDirective: { parentComment } } = directive;
45+
const { unprocessedDirective: { parentDirective } } = directive;
4646

47-
if (groups.has(parentComment)) {
48-
groups.get(parentComment).push(directive);
47+
if (groups.has(parentDirective)) {
48+
groups.get(parentDirective).push(directive);
4949
} else {
50-
groups.set(parentComment, [directive]);
50+
groups.set(parentDirective, [directive]);
5151
}
5252
}
5353

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

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

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

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

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

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

191191
return directiveGroups.flatMap(
192192
directives => {
193-
const { parentComment } = directives[0].unprocessedDirective;
194-
const remainingRuleIds = new Set(parentComment.ruleIds);
193+
const { parentDirective } = directives[0].unprocessedDirective;
194+
const remainingRuleIds = new Set(parentDirective.ruleIds);
195195

196196
for (const directive of directives) {
197197
remainingRuleIds.delete(directive.ruleId);
198198
}
199199

200200
return remainingRuleIds.size
201-
? createIndividualDirectivesRemoval(directives, parentComment.commentToken)
202-
: [createCommentRemoval(directives, parentComment.commentToken)];
201+
? createIndividualDirectivesRemoval(directives, parentDirective.node)
202+
: [createDirectiveRemoval(directives, parentDirective.node)];
203203
}
204204
);
205205
}
@@ -372,7 +372,7 @@ function applyDirectives(options) {
372372

373373
const unusedDirectives = processed
374374
.map(({ description, fix, unprocessedDirective }) => {
375-
const { parentComment, type, line, column } = unprocessedDirective;
375+
const { parentDirective, type, line, column } = unprocessedDirective;
376376

377377
let message;
378378

@@ -388,8 +388,8 @@ function applyDirectives(options) {
388388
return {
389389
ruleId: null,
390390
message,
391-
line: type === "disable-next-line" ? parentComment.commentToken.loc.start.line : line,
392-
column: type === "disable-next-line" ? parentComment.commentToken.loc.start.column + 1 : column,
391+
line: type === "disable-next-line" ? parentDirective.node.loc.start.line : line,
392+
column: type === "disable-next-line" ? parentDirective.node.loc.start.column + 1 : column,
393393
severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2,
394394
nodeType: null,
395395
...options.disableFixes ? {} : { fix }

‎lib/linter/linter.js‎

Lines changed: 27 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -273,49 +273,47 @@ function createLintingProblem(options) {
273273
* Creates a collection of disable directives from a comment
274274
* @param {Object} options to create disable directives
275275
* @param {("disable"|"enable"|"disable-line"|"disable-next-line")} options.type The type of directive comment
276-
* @param {token} options.commentToken The Comment token
277276
* @param {string} options.value The value after the directive in the comment
278277
* comment specified no specific rules, so it applies to all rules (e.g. `eslint-disable`)
279278
* @param {string} options.justification The justification of the directive
280-
* @param {function(string): {create: Function}} options.ruleMapper A map from rule IDs to defined rules
279+
* @param {ASTNode|token} options.node The Comment node/token.
280+
* @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
281281
* @returns {Object} Directives and problems from the comment
282282
*/
283-
function createDisableDirectives(options) {
284-
const { commentToken, type, value, justification, ruleMapper } = options;
283+
function createDisableDirectives({ type, value, justification, node }, ruleMapper) {
285284
const ruleIds = Object.keys(commentParser.parseListConfig(value));
286285
const directiveRules = ruleIds.length ? ruleIds : [null];
287286
const result = {
288287
directives: [], // valid disable directives
289288
directiveProblems: [] // problems in directives
290289
};
291-
292-
const parentComment = { commentToken, ruleIds };
290+
const parentDirective = { node, ruleIds };
293291

294292
for (const ruleId of directiveRules) {
295293

296294
// push to directives, if the rule is defined(including null, e.g. /*eslint enable*/)
297295
if (ruleId === null || !!ruleMapper(ruleId)) {
298296
if (type === "disable-next-line") {
299297
result.directives.push({
300-
parentComment,
298+
parentDirective,
301299
type,
302-
line: commentToken.loc.end.line,
303-
column: commentToken.loc.end.column + 1,
300+
line: node.loc.end.line,
301+
column: node.loc.end.column + 1,
304302
ruleId,
305303
justification
306304
});
307305
} else {
308306
result.directives.push({
309-
parentComment,
307+
parentDirective,
310308
type,
311-
line: commentToken.loc.start.line,
312-
column: commentToken.loc.start.column + 1,
309+
line: node.loc.start.line,
310+
column: node.loc.start.column + 1,
313311
ruleId,
314312
justification
315313
});
316314
}
317315
} else {
318-
result.directiveProblems.push(createLintingProblem({ ruleId, loc: commentToken.loc }));
316+
result.directiveProblems.push(createLintingProblem({ ruleId, loc: node.loc }));
319317
}
320318
}
321319
return result;
@@ -388,8 +386,12 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
388386
case "eslint-disable-next-line":
389387
case "eslint-disable-line": {
390388
const directiveType = directiveText.slice("eslint-".length);
391-
const options = { commentToken: comment, type: directiveType, value: directiveValue, justification: justificationPart, ruleMapper };
392-
const { directives, directiveProblems } = createDisableDirectives(options);
389+
const { directives, directiveProblems } = createDisableDirectives({
390+
type: directiveType,
391+
value: directiveValue,
392+
justification: justificationPart,
393+
node: comment
394+
}, ruleMapper);
393395

394396
disableDirectives.push(...directives);
395397
problems.push(...directiveProblems);
@@ -543,53 +545,21 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
543545
* A collection of the directive comments that were found, along with any problems that occurred when parsing
544546
*/
545547
function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper) {
546-
const problems = [];
547548
const disableDirectives = [];
549+
const problems = [];
548550

549-
sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
550-
const { directivePart, justificationPart } = commentParser.extractDirectiveComment(comment.value);
551-
552-
const match = directivesPattern.exec(directivePart);
553-
554-
if (!match) {
555-
return;
556-
}
557-
const directiveText = match[1];
558-
const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);
559-
560-
if (comment.type === "Line" && !lineCommentSupported) {
561-
return;
562-
}
563-
564-
if (directiveText === "eslint-disable-line" && comment.loc.start.line !== comment.loc.end.line) {
565-
const message = `${directiveText} comment should not span multiple lines.`;
566-
567-
problems.push(createLintingProblem({
568-
ruleId: null,
569-
message,
570-
loc: comment.loc
571-
}));
572-
return;
573-
}
574-
575-
const directiveValue = directivePart.slice(match.index + directiveText.length);
551+
const {
552+
directives: directivesSources,
553+
problems: directivesProblems
554+
} = sourceCode.getDisableDirectives();
576555

577-
switch (directiveText) {
578-
case "eslint-disable":
579-
case "eslint-enable":
580-
case "eslint-disable-next-line":
581-
case "eslint-disable-line": {
582-
const directiveType = directiveText.slice("eslint-".length);
583-
const options = { commentToken: comment, type: directiveType, value: directiveValue, justification: justificationPart, ruleMapper };
584-
const { directives, directiveProblems } = createDisableDirectives(options);
556+
problems.push(...directivesProblems.map(createLintingProblem));
585557

586-
disableDirectives.push(...directives);
587-
problems.push(...directiveProblems);
588-
break;
589-
}
558+
directivesSources.forEach(directive => {
559+
const { directives, directiveProblems } = createDisableDirectives(directive, ruleMapper);
590560

591-
// no default
592-
}
561+
disableDirectives.push(...directives);
562+
problems.push(...directiveProblems);
593563
});
594564

595565
return {

0 commit comments

Comments
 (0)