Skip to content

Commit

Permalink
Fix: preserve formatting when rules are removed from disable directiv…
Browse files Browse the repository at this point in the history 
…es (#15081)
  • Loading branch information
@mdjermanovic
mdjermanovic committed Sep 24, 2021
1 parent 14a4739 commit c9efb5f
Show file tree
Hide file tree
Showing 3 changed files with 644 additions and 17 deletions.
91 changes: 80 additions & 11 deletions lib/linter/apply-disable-directives.js
View file
Expand Up @@ -46,26 +46,95 @@ function groupByParentComment(directives) {
* @returns {{ description, fix, position }[]} Details for later creation of output Problems.
*/
function createIndividualDirectivesRemoval(directives, commentToken) {
const listOffset = /^\s*\S+\s+/u.exec(commentToken.value)[0].length;

/*
* `commentToken.value` starts right after `//` or `/*`.
* All calculated offsets will be relative to this index.
*/
const commentValueStart = commentToken.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;

/*
* Get the list text without any surrounding whitespace. In order to preserve the original
* formatting, we don't want to change that whitespace.
*
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
*/
const listText = commentToken.value
.slice(listOffset) // remove eslint-*
.split(/\s-{2,}\s/u)[0] // remove -- directive comment
.trimRight();
const listStart = commentToken.range[0] + 2 + listOffset;
.slice(listStartOffset) // remove directive name and all whitespace before the list
.split(/\s-{2,}\s/u)[0] // remove `-- comment`, if it exists
.trimRight(); // remove all whitespace after the list

/*
* We can assume that `listText` contains multiple elements.
* Otherwise, this function wouldn't be called - if there is
* only one rule in the list, then the whole comment must be removed.
*/

return directives.map(directive => {
const { ruleId } = directive;
const match = new RegExp(String.raw`(?:^|,)\s*${escapeRegExp(ruleId)}\s*(?:$|,)`, "u").exec(listText);
const ruleOffset = match.index;
const ruleEndOffset = ruleOffset + match[0].length;
const ruleText = listText.slice(ruleOffset, ruleEndOffset);

const regex = new RegExp(String.raw`(?:^|\s*,\s*)${escapeRegExp(ruleId)}(?:\s*,\s*|$)`, "u");
const match = regex.exec(listText);
const matchedText = match[0];
const matchStartOffset = listStartOffset + match.index;
const matchEndOffset = matchStartOffset + matchedText.length;

const firstIndexOfComma = matchedText.indexOf(",");
const lastIndexOfComma = matchedText.lastIndexOf(",");

let removalStartOffset, removalEndOffset;

if (firstIndexOfComma !== lastIndexOfComma) {

/*
* Since there are two commas, this must one of the elements in the middle of the list.
* Matched range starts where the previous rule name ends, and ends where the next rule name starts.
*
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^^^^
*
* We want to remove only the content between the two commas, and also one of the commas.
*
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^
*/
removalStartOffset = matchStartOffset + firstIndexOfComma;
removalEndOffset = matchStartOffset + lastIndexOfComma;

} else {

/*
* This is either the first element or the last element.
*
* If this is the first element, matched range starts where the first rule name starts
* and ends where the second rule name starts. This is exactly the range we want
* to remove so that the second rule name will start where the first one was starting
* and thus preserve the original formatting.
*
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^
*
* Similarly, if this is the last element, we've already matched the range we want to
* remove. The previous rule name will end where the last one was ending, relative
* to the content on the right side.
*
* // eslint-disable-line rule-one , rule-two , rule-three -- comment
* ^^^^^^^^^^^^^
*/
removalStartOffset = matchStartOffset;
removalEndOffset = matchEndOffset;
}

return {
description: `'${ruleId}'`,
fix: {
range: [
listStart + ruleOffset + (ruleText.startsWith(",") && ruleText.endsWith(",") ? 1 : 0),
listStart + ruleEndOffset
commentValueStart + removalStartOffset,
commentValueStart + removalEndOffset
],
text: ""
},
Expand Down
12 changes: 6 additions & 6 deletions tests/lib/linter/apply-disable-directives.js
View file
Expand Up @@ -1451,7 +1451,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 24,
fix: {
range: [24, 33],
range: [23, 32],
text: ""
},
severity: 2,
Expand Down Expand Up @@ -1492,7 +1492,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 18,
fix: {
range: [18, 25],
range: [18, 26],
text: ""
},
severity: 2,
Expand Down Expand Up @@ -1581,7 +1581,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 18,
fix: {
range: [18, 27],
range: [18, 28],
text: ""
},
severity: 2,
Expand All @@ -1593,7 +1593,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 28,
fix: {
range: [27, 37],
range: [26, 36],
text: ""
},
severity: 2,
Expand Down Expand Up @@ -1648,7 +1648,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 18,
fix: {
range: [18, 27],
range: [18, 28],
text: ""
},
severity: 2,
Expand All @@ -1660,7 +1660,7 @@ describe("apply-disable-directives", () => {
line: 1,
column: 28,
fix: {
range: [27, 37],
range: [26, 36],
text: ""
},
severity: 2,
Expand Down

0 comments on commit c9efb5f

Please sign in to comment.