From a9abdb9fe8b2b9ef7f76bfd58da1530fb28ea58e Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Wed, 23 Sep 2020 15:45:00 -0700 Subject: [PATCH 1/4] 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. --- lib/__tests__/disableRanges.test.js | 139 ++++++++++++++++++++++++++++ lib/assignDisabledRanges.js | 58 ++++++++---- 2 files changed, 177 insertions(+), 20 deletions(-) diff --git a/lib/__tests__/disableRanges.test.js b/lib/__tests__/disableRanges.test.js index cef906a581..7ffc583d71 100644 --- a/lib/__tests__/disableRanges.test.js +++ b/lib/__tests__/disableRanges.test.js @@ -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 */ @@ -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 diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index d57043e059..7d4f7946fd 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -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) && + comment.text.startsWith('stylelint-') && + 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) && !current.text.startsWith('stylelint-')) { + 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; From 2ea4fa3343583a311bfb2a895039551aa292d922 Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Wed, 23 Sep 2020 15:49:47 -0700 Subject: [PATCH 2/4] Fix indentation --- lib/__tests__/disableRanges.test.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/__tests__/disableRanges.test.js b/lib/__tests__/disableRanges.test.js index 7ffc583d71..26e6696581 100644 --- a/lib/__tests__/disableRanges.test.js +++ b/lib/__tests__/disableRanges.test.js @@ -854,7 +854,7 @@ it('SCSS /* disable comment (with // comment after blank line)', () => { it('SCSS // disable comment (with // comment immediately before)', () => { const scssSource = `a { - // Unrelated + // Unrelated // stylelint-disable declaration-no-important color: pink !important; }`; @@ -877,8 +877,8 @@ it('SCSS // disable comment (with // comment immediately before)', () => { it('SCSS two adjacent // disable comments ', () => { const scssSource = `a { - // stylelint-disable declaration-no-important - // stylelint-disable foo-bar + // stylelint-disable declaration-no-important + // stylelint-disable foo-bar color: pink !important; }`; @@ -906,11 +906,11 @@ it('SCSS two adjacent // disable comments ', () => { 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 + // stylelint-disable declaration-no-important -- + // Description 1 + // stylelint-disable foo-bar + // -- + // Description 2 color: pink !important; }`; @@ -940,9 +940,9 @@ it('SCSS two adjacent // disable comments with multi-line descriptions ', () => it('SCSS two // disable comments with an unrelated comment between them', () => { const scssSource = `a { - // stylelint-disable declaration-no-important - // Unrelated - // stylelint-disable foo-bar + // stylelint-disable declaration-no-important + // Unrelated + // stylelint-disable foo-bar color: pink !important; }`; From 231f81b2e4c43df44a96e39c87ff64cfe6d109ba Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Thu, 24 Sep 2020 13:13:47 -0700 Subject: [PATCH 3/4] Explicitly check for ^stylelint-(disable|enable) --- lib/assignDisabledRanges.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index 7d4f7946fd..bcde6b5df9 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -76,7 +76,7 @@ module.exports = function (root, result) { if ( !( isInlineComment(comment) && - comment.text.startsWith('stylelint-') && + isStylelintCommand(comment) && next && next.type === 'comment' && (comment.text.includes('--') || next.text.startsWith('--')) @@ -93,7 +93,7 @@ module.exports = function (root, result) { /** @type {PostcssComment} */ let current = next; - while (isInlineComment(current) && !current.text.startsWith('stylelint-')) { + while (isInlineComment(current) && !isStylelintCommand(current)) { const currentLine = (current.source && current.source.end && current.source.end.line) || 0; if (lastLine + 1 !== currentLine) break; @@ -126,6 +126,13 @@ module.exports = function (root, result) { return comment.inline || comment.raws.inline; } + /** + * @param {PostcssComment} comment + */ + function isStylelintCommand(comment) { + return /^stylelint-(disable|enable)/.test(comment.text); + } + /** * @param {PostcssComment} comment */ From 03957cbe4d8ac6f41d0e9daaca60e53a3ca7aad1 Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Thu, 24 Sep 2020 13:31:48 -0700 Subject: [PATCH 4/4] Use existing variables rather than hardcoding --- lib/assignDisabledRanges.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assignDisabledRanges.js b/lib/assignDisabledRanges.js index bcde6b5df9..0ba45742b1 100644 --- a/lib/assignDisabledRanges.js +++ b/lib/assignDisabledRanges.js @@ -130,7 +130,7 @@ module.exports = function (root, result) { * @param {PostcssComment} comment */ function isStylelintCommand(comment) { - return /^stylelint-(disable|enable)/.test(comment.text); + return comment.text.startsWith(disableCommand) || comment.text.startsWith(enableCommand); } /**