From 4602bb6b95a37eb38f423df0fd7ef19ae5ee2f58 Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Fri, 20 Mar 2020 19:09:07 +0300 Subject: [PATCH] refactor css comments (#7021) * refactor css comments * workarounds for postcss-less location bugs * improve replaceQuotesInInlineComments * update changelog * fix comments in selectors * fix comments in values * use \ufffe instead of \0 because that's what these characters are for see http://www.unicode.org/faq/private_use.html#nonchar6 * fix quoted urls Co-authored-by: Georgii Dolzhykov --- changelog_unreleased/less/pr-6981.md | 2 +- cspell.json | 2 + src/language-css/loc.js | 178 ++++++++++++++++++ src/language-css/parser-postcss.js | 23 ++- src/language-css/printer-postcss.js | 35 ++-- .../css_less/__snapshots__/jsfmt.spec.js.snap | 127 ++++++++++++- tests/css_less/comments.less | 12 ++ tests/css_less/less.less | 4 +- tests/css_less/quotes-in-comments.less | 25 +++ .../css_scss/__snapshots__/jsfmt.spec.js.snap | 73 +++++++ tests/css_scss/comments.scss | 12 ++ 11 files changed, 455 insertions(+), 38 deletions(-) create mode 100644 src/language-css/loc.js create mode 100644 tests/css_less/comments.less create mode 100644 tests/css_less/quotes-in-comments.less create mode 100644 tests/css_scss/comments.scss diff --git a/changelog_unreleased/less/pr-6981.md b/changelog_unreleased/less/pr-6981.md index 7c35f860b..b8dbacd6c 100644 --- a/changelog_unreleased/less/pr-6981.md +++ b/changelog_unreleased/less/pr-6981.md @@ -1,4 +1,4 @@ -#### Fix multiple long-standing issues by updating `postcss-less` ([#6981](https://github.com/prettier/prettier/pull/6981) by [@fisker](https://github.com/fisker)) +#### Fix multiple long-standing issues by updating `postcss-less` ([#6981](https://github.com/prettier/prettier/pull/6981) by [@fisker](https://github.com/fisker), [#7021](https://github.com/prettier/prettier/pull/7021) by [@evilebottnawi](https://github.com/evilebottnawi), [@thorn0](https://github.com/thorn0)) - [`each`](http://lesscss.org/functions/#list-functions-each) is supported now ([#5653](https://github.com/prettier/prettier/issues/5653). - `!important` was being moved out of mixin call parameters ([#3544](https://github.com/prettier/prettier/issues/3544)). diff --git a/cspell.json b/cspell.json index dcbc447f6..9c7199325 100644 --- a/cspell.json +++ b/cspell.json @@ -237,6 +237,7 @@ "nicolo", "nnoremap", "nodir", + "noncharacters", "nonenumerable", "nonspacing", "normalise", @@ -359,6 +360,7 @@ "typeof", "udbff", "udfff", + "ufffe", "uffff", "Umidbek", "unaries", diff --git a/src/language-css/loc.js b/src/language-css/loc.js new file mode 100644 index 000000000..b3bad38db --- /dev/null +++ b/src/language-css/loc.js @@ -0,0 +1,178 @@ +"use strict"; + +const lineColumnToIndex = require("../utils/line-column-to-index"); +const { getLast, skipEverythingButNewLine } = require("../common/util"); + +function calculateLocStart(node, text) { + if (node.source) { + return lineColumnToIndex(node.source.start, text) - 1; + } + return null; +} + +function calculateLocEnd(node, text) { + if (node.type === "css-comment" && node.inline) { + return skipEverythingButNewLine(text, node.source.startOffset); + } + const endNode = node.nodes && getLast(node.nodes); + if (endNode && node.source && !node.source.end) { + node = endNode; + } + if (node.source && node.source.end) { + return lineColumnToIndex(node.source.end, text); + } + return null; +} + +function calculateLoc(node, text) { + if (node && typeof node === "object") { + if (node.source) { + node.source.startOffset = calculateLocStart(node, text); + node.source.endOffset = calculateLocEnd(node, text); + } + + for (const key in node) { + calculateLoc(node[key], text); + } + } +} + +/** + * Workaround for a bug: quotes in inline comments corrupt loc data of subsequent nodes. + * This function replaces the quotes with U+FFFE and U+FFFF. Later, when the comments are printed, + * their content is extracted from the original text or restored by replacing the placeholder + * characters back with quotes. + * - https://github.com/prettier/prettier/issues/7780 + * - https://github.com/shellscape/postcss-less/issues/145 + * - About noncharacters (U+FFFE and U+FFFF): http://www.unicode.org/faq/private_use.html#nonchar1 + * @param text {string} + */ +function replaceQuotesInInlineComments(text) { + /** @typedef { 'initial' | 'single-quotes' | 'double-quotes' | 'url' | 'comment-block' | 'comment-inline' } State */ + /** @type {State} */ + let state = "initial"; + /** @type {State} */ + let stateToReturnFromQuotes = "initial"; + let inlineCommentStartIndex; + let inlineCommentContainsQuotes = false; + const inlineCommentsToReplace = []; + + for (let i = 0; i < text.length; i++) { + const c = text[i]; + + switch (state) { + case "initial": + if (c === "'") { + state = "single-quotes"; + continue; + } + + if (c === '"') { + state = "double-quotes"; + continue; + } + + if ( + (c === "u" || c === "U") && + text.slice(i, i + 4).toLowerCase() === "url(" + ) { + state = "url"; + i += 3; + continue; + } + + if (c === "*" && text[i - 1] === "/") { + state = "comment-block"; + continue; + } + + if (c === "/" && text[i - 1] === "/") { + state = "comment-inline"; + inlineCommentStartIndex = i - 1; + continue; + } + + continue; + + case "single-quotes": + if (c === "'" && text[i - 1] !== "\\") { + state = stateToReturnFromQuotes; + stateToReturnFromQuotes = "initial"; + } + if (c === "\n" || c === "\r") { + return text; // invalid input + } + continue; + + case "double-quotes": + if (c === '"' && text[i - 1] !== "\\") { + state = stateToReturnFromQuotes; + stateToReturnFromQuotes = "initial"; + } + if (c === "\n" || c === "\r") { + return text; // invalid input + } + continue; + + case "url": + if (c === ")") { + state = "initial"; + } + if (c === "\n" || c === "\r") { + return text; // invalid input + } + if (c === "'") { + state = "single-quotes"; + stateToReturnFromQuotes = "url"; + continue; + } + if (c === '"') { + state = "double-quotes"; + stateToReturnFromQuotes = "url"; + continue; + } + continue; + + case "comment-block": + if (c === "/" && text[i - 1] === "*") { + state = "initial"; + } + continue; + + case "comment-inline": + if (c === '"' || c === "'") { + inlineCommentContainsQuotes = true; + } + if (c === "\n" || c === "\r") { + if (inlineCommentContainsQuotes) { + inlineCommentsToReplace.push([inlineCommentStartIndex, i]); + } + state = "initial"; + inlineCommentContainsQuotes = false; + } + continue; + } + } + + for (const [start, end] of inlineCommentsToReplace) { + text = + text.slice(0, start) + + text + .slice(start, end) + .replace(/'/g, "\ufffe") + .replace(/"/g, "\uffff") + + text.slice(end); + } + + return text; +} + +function restoreQuotesInInlineComments(text) { + return text.replace(/\ufffe/g, "'").replace(/\uffff/g, '"'); +} + +module.exports = { + calculateLoc, + replaceQuotesInInlineComments, + restoreQuotesInInlineComments +}; diff --git a/src/language-css/parser-postcss.js b/src/language-css/parser-postcss.js index 7232ab955..dedbed4fa 100644 --- a/src/language-css/parser-postcss.js +++ b/src/language-css/parser-postcss.js @@ -2,9 +2,9 @@ const createError = require("../common/parser-create-error"); const parseFrontMatter = require("../utils/front-matter"); -const lineColumnToIndex = require("../utils/line-column-to-index"); const { hasPragma } = require("./pragma"); const { isLessParser, isSCSS, isSCSSNestedPropertyNode } = require("./utils"); +const { calculateLoc, replaceQuotesInInlineComments } = require("./loc"); function parseValueNodes(nodes) { let parenGroup = { @@ -294,7 +294,7 @@ function parseNestedCSS(node, options) { value = value.trim(); - node.raws.value = selector; + node.raws.value = value; } let params = ""; @@ -541,6 +541,8 @@ function parseWithParser(parser, text, options) { result = parseNestedCSS(addTypePrefix(result, "css-"), options); + calculateLoc(result, text); + if (frontMatter) { result.nodes.unshift(frontMatter); } @@ -553,7 +555,12 @@ function requireParser(isSCSSParser) { return require("postcss-scss"); } - return require("postcss-less"); + const lessParser = require("postcss-less"); + return { + // Workaround for https://github.com/shellscape/postcss-less/issues/145 + // See comments for `replaceQuotesInInlineComments` in `loc.js`. + parse: text => lessParser.parse(replaceQuotesInInlineComments(text)) + }; } function parse(text, parsers, options) { @@ -582,17 +589,13 @@ const parser = { hasPragma, locStart(node) { if (node.source) { - return lineColumnToIndex(node.source.start, node.source.input.css) - 1; + return node.source.startOffset; } return null; }, locEnd(node) { - const endNode = node.nodes && node.nodes[node.nodes.length - 1]; - if (endNode && node.source && !node.source.end) { - node = endNode; - } - if (node.source && node.source.end) { - return lineColumnToIndex(node.source.end, node.source.input.css); + if (node.source) { + return node.source.endOffset; } return null; } diff --git a/src/language-css/printer-postcss.js b/src/language-css/printer-postcss.js index a795e8ef7..78de43f1c 100644 --- a/src/language-css/printer-postcss.js +++ b/src/language-css/printer-postcss.js @@ -10,6 +10,7 @@ const { hasNewline } = require("../common/util"); const { isNextLineEmpty } = require("../common/util-shared"); +const { restoreQuotesInInlineComments } = require("./loc"); const { builders: { @@ -110,31 +111,13 @@ function genericPrint(path, options, print) { return nodes; } case "css-comment": { - if (node.raws.content) { - return node.raws.content; - } + const isInlineComment = node.inline || node.raws.inline; const text = options.originalText.slice( options.locStart(node), options.locEnd(node) ); - const rawText = node.raws.text || node.text; - // Workaround a bug where the location is off. - // https://github.com/postcss/postcss-scss/issues/63 - if (!text.includes(rawText)) { - if (node.raws.inline || node.inline) { - const needBreakAfter = !( - node.source.input.css.split(/[\r\n]/)[node.source.end.line] || "" - ).trim(); - return concat([ - "//", - node.raws.left + rawText, - needBreakAfter ? line : "" - ]); - } - return concat(["/* ", rawText, " */"]); - } - return text; + return isInlineComment ? text.trimEnd() : text; } case "css-rule": { return concat([ @@ -481,6 +464,14 @@ function genericPrint(path, options, print) { ); } + // originalText has to be used for Less, see replaceQuotesInInlineComments in loc.js + const parentNode = path.getParentNode(); + if (parentNode.raws && parentNode.raws.selector) { + const start = options.locStart(parentNode); + const end = start + parentNode.raws.selector.length; + return options.originalText.slice(start, end).trim(); + } + return node.value; } // postcss-values-parser @@ -491,7 +482,9 @@ function genericPrint(path, options, print) { case "value-comment": { return concat([ node.inline ? "//" : "/*", - node.value, + // see replaceQuotesInInlineComments in loc.js + // value-* nodes don't have correct location data, so we have to rely on placeholder characters. + restoreQuotesInInlineComments(node.value), node.inline ? "" : "*/" ]); } diff --git a/tests/css_less/__snapshots__/jsfmt.spec.js.snap b/tests/css_less/__snapshots__/jsfmt.spec.js.snap index 7fe3a1de9..2ad764216 100644 --- a/tests/css_less/__snapshots__/jsfmt.spec.js.snap +++ b/tests/css_less/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,41 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`comments.less 1`] = ` +====================================options===================================== +parsers: ["less"] +printWidth: 80 + | printWidth +=====================================input====================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +=====================================output===================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +================================================================================ +`; + exports[`function-each.less 1`] = ` ====================================options===================================== parsers: ["less"] @@ -1703,8 +1739,8 @@ li + li { // Variables @link-color: #428bca; // Comment -[href]:hover &, // Comment -[href]:focus &, // Comment +[href]:hover &, // Comment' +[href]:focus &, // Comment" [href]:active & { .tooltip { opacity: 1; @@ -3315,8 +3351,8 @@ li + li { // Variables @link-color: #428bca; // Comment -[href]:hover &, // Comment -[href]:focus &, // Comment +[href]:hover &, // Comment' +[href]:focus &, // Comment" [href]:active & { .tooltip { opacity: 1; @@ -3344,3 +3380,86 @@ label { ================================================================================ `; + +exports[`quotes-in-comments.less 1`] = ` +====================================options===================================== +parsers: ["less"] +printWidth: 80 + | printWidth +=====================================input====================================== +// Animation +// ' +.a { color: red } + +.aa:hover::after { content: "\\""; } // " + + +.b { background:url(http://example.com); } .b::after { content: "a"; } + +.bb { background:url('http://example.com/(foo)bar'); } .bb::after { content: "'a"; } // """ + +// Animation ! +.c { color:red } + +.d { color: red /* // */ } .d::after { content: "'"; } + +/* fff + // */ .d:hover::after { + // qw'erty + content: "'''"; } + +[href]:active { + opacity: // '!' "?" + 1; +} +=====================================output===================================== +// Animation +// ' +.a { + color: red; +} + +.aa:hover::after { + content: '"'; +} // " + +.b { + background: url(http://example.com); +} +.b::after { + content: "a"; +} + +.bb { + background: url("http://example.com/(foo)bar"); +} +.bb::after { + content: "'a"; +} // """ + +// Animation ! +.c { + color: red; +} + +.d { + color: red; /* // */ +} +.d::after { + content: "'"; +} + +/* fff + // */ +.d:hover::after { + // qw'erty + content: "'''"; +} + +[href]:active { + opacity: // '!' "?" + 1; +} + +================================================================================ +`; diff --git a/tests/css_less/comments.less b/tests/css_less/comments.less new file mode 100644 index 000000000..afe15239d --- /dev/null +++ b/tests/css_less/comments.less @@ -0,0 +1,12 @@ +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. diff --git a/tests/css_less/less.less b/tests/css_less/less.less index d7a13f052..56250af0f 100644 --- a/tests/css_less/less.less +++ b/tests/css_less/less.less @@ -1648,8 +1648,8 @@ li + li { // Variables @link-color: #428bca; // Comment -[href]:hover &, // Comment -[href]:focus &, // Comment +[href]:hover &, // Comment' +[href]:focus &, // Comment" [href]:active & { .tooltip { opacity: 1; diff --git a/tests/css_less/quotes-in-comments.less b/tests/css_less/quotes-in-comments.less new file mode 100644 index 000000000..e58cbc460 --- /dev/null +++ b/tests/css_less/quotes-in-comments.less @@ -0,0 +1,25 @@ +// Animation +// ' +.a { color: red } + +.aa:hover::after { content: "\""; } // " + + +.b { background:url(http://example.com); } .b::after { content: "a"; } + +.bb { background:url('http://example.com/(foo)bar'); } .bb::after { content: "'a"; } // """ + +// Animation ! +.c { color:red } + +.d { color: red /* // */ } .d::after { content: "'"; } + +/* fff + // */ .d:hover::after { + // qw'erty + content: "'''"; } + +[href]:active { + opacity: // '!' "?" + 1; +} \ No newline at end of file diff --git a/tests/css_scss/__snapshots__/jsfmt.spec.js.snap b/tests/css_scss/__snapshots__/jsfmt.spec.js.snap index db5b29400..c3ed95054 100644 --- a/tests/css_scss/__snapshots__/jsfmt.spec.js.snap +++ b/tests/css_scss/__snapshots__/jsfmt.spec.js.snap @@ -1,5 +1,78 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`comments.scss 1`] = ` +====================================options===================================== +parsers: ["scss"] +printWidth: 80 +trailingComma: "none" + | printWidth +=====================================input====================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +=====================================output===================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +================================================================================ +`; + +exports[`comments.scss 2`] = ` +====================================options===================================== +parsers: ["scss"] +printWidth: 80 + | printWidth +=====================================input====================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +=====================================output===================================== +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent. + +================================================================================ +`; + exports[`import_comma.scss 1`] = ` ====================================options===================================== parsers: ["scss"] diff --git a/tests/css_scss/comments.scss b/tests/css_scss/comments.scss new file mode 100644 index 000000000..afe15239d --- /dev/null +++ b/tests/css_scss/comments.scss @@ -0,0 +1,12 @@ +// This comment won't be included in the CSS. +//This comment won't be included in the CSS. +// This comment won't be included in the CSS. + +/* But this comment will, except in compressed mode. */ +/* But this comment will, except in compressed mode. */ +/*But this comment will, except in compressed mode.*/ + +/*! This comment will be included even in compressed mode. */ + +/// Computes an exponent. +///// Computes an exponent.