Skip to content

Commit

Permalink
Improve inline comment merging (#4950)
Browse files Browse the repository at this point in the history 
* Improve inline comment merging

This now only starts a merge when the comment begins with `stylelint-`
and either the first line contains `--` or the second line starts with
`--`.

The merge will end whenever another line starts with `stylelint-` to
allow for adjacent stylelint commands.

* Fix indentation

* Explicitly check for ^stylelint-(disable|enable)

* Use existing variables rather than hardcoding
  • Loading branch information
@jathak
jathak committed Sep 24, 2020
1 parent 7449dc0 commit 4f068be
Show file tree
Hide file tree
Showing 2 changed files with 184 additions and 20 deletions.
139 changes: 139 additions & 0 deletions lib/__tests__/disableRanges.test.js
View file
Expand Up @@ -805,6 +805,29 @@ it('SCSS // disable comment (with // comment after blank line)', () => {
});
});

it('SCSS // disable comment (with // comment immediately after)', () => {
const scssSource = `a {
// stylelint-disable declaration-no-important
// Unrelated
color: pink !important;
}`;

return postcss()
.use(assignDisabledRanges)
.process(scssSource, { syntax: scss, from: undefined })
.then((result) => {
expect(result.stylelint.disabledRanges).toEqual({
all: [],
'declaration-no-important': [
{
start: 2,
strictStart: true,
},
],
});
});
});

it('SCSS /* disable comment (with // comment after blank line)', () => {
const scssSource = `a {
/* stylelint-disable declaration-no-important */
Expand All @@ -829,6 +852,122 @@ it('SCSS /* disable comment (with // comment after blank line)', () => {
});
});

it('SCSS // disable comment (with // comment immediately before)', () => {
const scssSource = `a {
// Unrelated
// stylelint-disable declaration-no-important
color: pink !important;
}`;

return postcss()
.use(assignDisabledRanges)
.process(scssSource, { syntax: scss, from: undefined })
.then((result) => {
expect(result.stylelint.disabledRanges).toEqual({
all: [],
'declaration-no-important': [
{
start: 3,
strictStart: true,
},
],
});
});
});

it('SCSS two adjacent // disable comments ', () => {
const scssSource = `a {
// stylelint-disable declaration-no-important
// stylelint-disable foo-bar
color: pink !important;
}`;

return postcss()
.use(assignDisabledRanges)
.process(scssSource, { syntax: scss, from: undefined })
.then((result) => {
expect(result.stylelint.disabledRanges).toEqual({
all: [],
'declaration-no-important': [
{
start: 2,
strictStart: true,
},
],
'foo-bar': [
{
start: 3,
strictStart: true,
},
],
});
});
});

it('SCSS two adjacent // disable comments with multi-line descriptions ', () => {
const scssSource = `a {
// stylelint-disable declaration-no-important --
// Description 1
// stylelint-disable foo-bar
// --
// Description 2
color: pink !important;
}`;

return postcss()
.use(assignDisabledRanges)
.process(scssSource, { syntax: scss, from: undefined })
.then((result) => {
expect(result.stylelint.disabledRanges).toEqual({
all: [],
'declaration-no-important': [
{
start: 2,
strictStart: true,
description: 'Description 1',
},
],
'foo-bar': [
{
start: 4,
strictStart: true,
description: 'Description 2',
},
],
});
});
});

it('SCSS two // disable comments with an unrelated comment between them', () => {
const scssSource = `a {
// stylelint-disable declaration-no-important
// Unrelated
// stylelint-disable foo-bar
color: pink !important;
}`;

return postcss()
.use(assignDisabledRanges)
.process(scssSource, { syntax: scss, from: undefined })
.then((result) => {
expect(result.stylelint.disabledRanges).toEqual({
all: [],
'declaration-no-important': [
{
start: 2,
strictStart: true,
},
],
'foo-bar': [
{
start: 4,
strictStart: true,
},
],
});
});
});

it('Less // line-disabling comment (with description)', () => {
const lessSource = `a {
color: pink !important; // stylelint-disable-line declaration-no-important -- Description
Expand Down
65 changes: 45 additions & 20 deletions lib/assignDisabledRanges.js
View file
Expand Up @@ -66,35 +66,53 @@ module.exports = function (root, result) {
if (inlineEnd) {
// Ignore comments already processed by grouping with a previous one.
if (inlineEnd === comment) inlineEnd = null;
} else if (isInlineComment(comment)) {
const fullComment = comment.clone();
let next = comment.next();
let lastLine = (comment.source && comment.source.end && comment.source.end.line) || 0;

while (next && next.type === 'comment') {
/** @type {PostcssComment} */
const current = next;
return;
}

if (!isInlineComment(current)) break;
const next = comment.next();

// If any of these conditions are not met, do not merge comments.
if (
!(
isInlineComment(comment) &&
isStylelintCommand(comment) &&
next &&
next.type === 'comment' &&
(comment.text.includes('--') || next.text.startsWith('--'))
)
) {
checkComment(comment);

const currentLine = (current.source && current.source.end && current.source.end.line) || 0;
return;
}

if (lastLine + 1 !== currentLine) break;
let lastLine = (comment.source && comment.source.end && comment.source.end.line) || 0;
const fullComment = comment.clone();

fullComment.text += `\n${current.text}`;
/** @type {PostcssComment} */
let current = next;

if (fullComment.source && current.source) {
fullComment.source.end = current.source.end;
}
while (isInlineComment(current) && !isStylelintCommand(current)) {
const currentLine = (current.source && current.source.end && current.source.end.line) || 0;

if (lastLine + 1 !== currentLine) break;

inlineEnd = current;
next = current.next();
lastLine = currentLine;
fullComment.text += `\n${current.text}`;

if (fullComment.source && current.source) {
fullComment.source.end = current.source.end;
}
checkComment(fullComment);
} else {
checkComment(comment);

inlineEnd = current;
const next = current.next();

if (!next || next.type !== 'comment') break;

current = next;
lastLine = currentLine;
}
checkComment(fullComment);
});

return result;
Expand All @@ -108,6 +126,13 @@ module.exports = function (root, result) {
return comment.inline || comment.raws.inline;
}

/**
* @param {PostcssComment} comment
*/
function isStylelintCommand(comment) {
return comment.text.startsWith(disableCommand) || comment.text.startsWith(enableCommand);
}

/**
* @param {PostcssComment} comment
*/
Expand Down

0 comments on commit 4f068be

Please sign in to comment.