Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: preserve formatting when rules are removed from disable directives #15081

Merged
merged 1 commit into from Sep 24, 2021
Merged

Fix: preserve formatting when rules are removed from disable directives #15081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 80 additions & 11 deletions lib/linter/apply-disable-directives.js
View file
Open in desktop
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
Open in desktop
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