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 <thorn.mailbox@gmail.com>

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)).

cspell.json

@@ -237,6 +237,7 @@
         "nicolo",
         "nnoremap",
         "nodir",
+        "noncharacters",
         "nonenumerable",
         "nonspacing",
         "normalise",
@@ -359,6 +360,7 @@
         "typeof",
         "udbff",
         "udfff",
+        "ufffe",
         "uffff",
         "Umidbek",
         "unaries",

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
+};

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;
   }

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 ? "" : "*/"
       ]);
     }

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;
+}
+
+================================================================================
+`;

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.

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;

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;
+}

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"]

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.