Merge pull request #322 from kristerkari/bugfix/double-slash-comment-…

…empty-line-before-autofix

double-slash-comment-empty-line-before: bug with "never" option autofix

src/rules/double-slash-comment-empty-line-before/__tests__/index.js

@@ -281,6 +281,7 @@ testRule(rule, {
   config: ["never"],
   syntax: "scss",
   skipBasicChecks: true,
+  fix: true,
 
   accept: [
     {
@@ -328,6 +329,10 @@ testRule(rule, {
       code: `
       // comment
 
+      /// comment
+    `,
+      fixed: `
+      // comment
       /// comment
     `,
       message: messages.rejected
@@ -338,10 +343,41 @@ testRule(rule, {
 
       // comment
     `,
+      fixed: `
+      a {}
+      // comment
+    `,
+      message: messages.rejected
+    },
+    {
+      code: `
+      a {}
+
+
+      // comment
+    `,
+      fixed: `
+      a {}
+      // comment
+    `,
+      description: "multiple lines",
+      message: messages.rejected
+    },
+    {
+      code: "a {}\n\n\n\n\n\n// comment",
+      fixed: "a {}\n// comment",
+      description: "multiple lines",
+      message: messages.rejected
+    },
+    {
+      code: "a {}\r\n\r\n// comment",
+      fixed: "a {}\r\n// comment",
+      description: "CRLF",
       message: messages.rejected
     },
     {
       code: "a {}\r\n\r\n\r\n// comment",
+      fixed: "a {}\r\n// comment",
       description: "CRLF",
       message: messages.rejected
     },
@@ -354,6 +390,41 @@ testRule(rule, {
         top: 0;
       }
     `,
+      fixed: `
+      a {
+        color: pink;
+        // comment
+        top: 0;
+      }
+    `,
+      message: messages.rejected
+    },
+    {
+      code: `
+      h3 {
+
+        // font-family: $fontStack;
+        color: $textBlack;
+        font-weight: 400;
+        font-size: 24px;
+
+        // font-weight: 700;
+        line-height: 35px;
+
+      }
+    `,
+      fixed: `
+      h3 {
+        // font-family: $fontStack;
+        color: $textBlack;
+        font-weight: 400;
+        font-size: 24px;
+        // font-weight: 700;
+        line-height: 35px;
+
+      }
+    `,
+      description: "issue #321",
       message: messages.rejected
     }
   ]

src/rules/double-slash-comment-empty-line-before/index.js

@@ -1,10 +1,12 @@
+import { utils } from "stylelint";
 import {
+  addEmptyLineBefore,
   isInlineComment,
   namespace,
   optionsHaveException,
-  optionsHaveIgnored
+  optionsHaveIgnored,
+  removeEmptyLinesBefore
 } from "../../utils";
-import { utils } from "stylelint";
 
 export const ruleName = namespace("double-slash-comment-empty-line-before");
 
@@ -38,17 +40,6 @@ export default function(expectation, options, context) {
       return;
     }
 
-    const fix = (comment, match, replace) => {
-      const escapedMatch = match.replace(/(\r)?\n/g, (_, r) =>
-        r ? "\\r\\n" : "\\n"
-      );
-
-      comment.raws.before = comment.raws.before.replace(
-        new RegExp(`^${escapedMatch}`),
-        replace
-      );
-    };
-
     root.walkComments(comment => {
       // Only process // comments
       if (!comment.raws.inline && !comment.inline) {
@@ -106,13 +97,13 @@ export default function(expectation, options, context) {
 
       if (context.fix) {
         if (expectEmptyLineBefore && !hasEmptyLineBefore) {
-          fix(comment, context.newline, context.newline + context.newline);
+          addEmptyLineBefore(comment, context.newline);
 
           return;
         }
 
         if (!expectEmptyLineBefore && hasEmptyLineBefore) {
-          fix(comment, context.newline + context.newline, context.newline);
+          removeEmptyLinesBefore(comment, context.newline);
 
           return;
         }

src/utils/__tests__/addEmptyLineBefore.test.js

@@ -0,0 +1,44 @@
+import postcss from "postcss";
+import { addEmptyLineBefore } from "../addEmptyLineBefore";
+
+describe("addEmptyLineBefore", () => {
+  it("adds single newline to the newline at the beginning", () => {
+    expect(run("a {}\n  b{}", "\n")).toBe("a {}\n\n  b{}");
+  });
+
+  it("adds single newline to newline at the beginning with CRLF", () => {
+    expect(run("a {}\r\n  b{}", "\r\n")).toBe("a {}\r\n\r\n  b{}");
+  });
+
+  it("adds single newline to newline at the end", () => {
+    expect(run("a {}\t\nb{}", "\n")).toBe("a {}\t\n\nb{}");
+  });
+
+  it("adds single newline to newline at the end with CRLF", () => {
+    expect(run("a {}\t\r\nb{}", "\r\n")).toBe("a {}\t\r\n\r\nb{}");
+  });
+
+  it("adds single newline to newline in the middle", () => {
+    expect(run("a {}  \n\tb{}", "\n")).toBe("a {}  \n\n\tb{}");
+  });
+
+  it("adds single newline to newline in the middle with CRLF", () => {
+    expect(run("a {}  \r\n\tb{}", "\r\n")).toBe("a {}  \r\n\r\n\tb{}");
+  });
+
+  it("adds two newlines if there aren't any existing newlines", () => {
+    expect(run("a {}  b{}", "\n")).toBe("a {}\n\n  b{}");
+  });
+
+  it("adds two newlines if there aren't any existing newlines with CRLF", () => {
+    expect(run("a {}  b{}", "\r\n")).toBe("a {}\r\n\r\n  b{}");
+  });
+});
+
+function run(css, lineEnding) {
+  const root = postcss.parse(css);
+
+  addEmptyLineBefore(root.nodes[1], lineEnding);
+
+  return root.toString();
+}

src/utils/__tests__/removeEmptyLineBefore.test.js

@@ -0,0 +1,52 @@
+import postcss from "postcss";
+import { removeEmptyLinesBefore } from "../removeEmptyLinesBefore";
+
+describe("removeEmptyLineBefore", () => {
+  it("removes single newline from the newline at the beginning", () => {
+    expect(run("a {}\n\n  b{}", "\n")).toBe("a {}\n  b{}");
+  });
+
+  it("removes single newline from newline at the beginning with CRLF", () => {
+    expect(run("a {}\r\n\r\n  b{}", "\r\n")).toBe("a {}\r\n  b{}");
+  });
+
+  it("removes single newline from newline at the end", () => {
+    expect(run("a {}\t\n\nb{}", "\n")).toBe("a {}\t\nb{}");
+  });
+
+  it("removes single newline from newline at the end with CRLF", () => {
+    expect(run("a {}\t\r\n\r\nb{}", "\r\n")).toBe("a {}\t\r\nb{}");
+  });
+
+  it("removes single newline from newline in the middle", () => {
+    expect(run("a {}  \n\n\tb{}", "\n")).toBe("a {}  \n\tb{}");
+  });
+
+  it("removes single newline to newline in the middle with CRLF", () => {
+    expect(run("a {}  \r\n\r\n\tb{}", "\r\n")).toBe("a {}  \r\n\tb{}");
+  });
+
+  it("removes two newlines if there are three newlines", () => {
+    expect(run("a {}\n\n\n  b{}", "\n")).toBe("a {}\n  b{}");
+  });
+
+  it("removes two newlines if there are three newlines with CRLF", () => {
+    expect(run("a {}\r\n\r\n\r\n  b{}", "\r\n")).toBe("a {}\r\n  b{}");
+  });
+
+  it("removes three newlines if there are four newlines", () => {
+    expect(run("a {}\n\n\n\n  b{}", "\n")).toBe("a {}\n  b{}");
+  });
+
+  it("removes three newlines if there are four newlines with CRLF", () => {
+    expect(run("a {}\r\n\r\n\r\n\r\n  b{}", "\r\n")).toBe("a {}\r\n  b{}");
+  });
+});
+
+function run(css, lineEnding) {
+  const root = postcss.parse(css);
+
+  removeEmptyLinesBefore(root.nodes[1], lineEnding);
+
+  return root.toString();
+}

src/utils/addEmptyLineBefore.js

@@ -0,0 +1,15 @@
+import { repeat } from "lodash";
+
+// Add an empty line before a node. Mutates the node.
+export function addEmptyLineBefore(
+  node /*: postcss$node*/,
+  newline /*: '\n' | '\r\n'*/
+) /*: postcss$node*/ {
+  if (!/\r?\n/.test(node.raws.before)) {
+    node.raws.before = repeat(newline, 2) + node.raws.before;
+  } else {
+    node.raws.before = node.raws.before.replace(/(\r?\n)/, `${newline}$1`);
+  }
+
+  return node;
+}

src/utils/index.js

@@ -1,15 +1,16 @@
+export { addEmptyLineBefore } from "./addEmptyLineBefore";
 export { default as atRuleParamIndex } from "./atRuleParamIndex";
 export { default as beforeBlockString } from "./beforeBlockString";
 export { default as blockString } from "./blockString";
 export { default as declarationValueIndex } from "./declarationValueIndex";
 // Todo should be deleted after upgrade `stylelint` to `10`
 export { default as eachRoot } from "./eachRoot";
 export { default as findCommentsInRaws } from "./findCommentsInRaws";
+export { default as hasBlock } from "./hasBlock";
+export { default as hasEmptyLine } from "./hasEmptyLine";
 export {
   default as hasInterpolatingAmpersand
 } from "./hasInterpolatingAmpersand";
-export { default as hasBlock } from "./hasBlock";
-export { default as hasEmptyLine } from "./hasEmptyLine";
 export { default as isInlineComment } from "./isInlineComment";
 export { default as isNativeCssFunction } from "./isNativeCssFunction";
 export { default as isSingleLineString } from "./isSingleLineString";
@@ -22,9 +23,10 @@ export { default as isWhitespace } from "./isWhitespace";
 export { default as namespace } from "./namespace";
 export { default as optionsHaveException } from "./optionsHaveException";
 export { default as optionsHaveIgnored } from "./optionsHaveIgnored";
+export { parseFunctionArguments } from "./parseFunctionArguments";
 export { default as parseNestedPropRoot } from "./parseNestedPropRoot";
 export { default as parseSelector } from "./parseSelector";
-export { default as findOperators } from "./sassValueParser";
 export { default as rawNodeString } from "./rawNodeString";
+export { removeEmptyLinesBefore } from "./removeEmptyLinesBefore";
+export { default as findOperators } from "./sassValueParser";
 export { default as whitespaceChecker } from "./whitespaceChecker";
-export { parseFunctionArguments } from "./parseFunctionArguments";

src/utils/removeEmptyLinesBefore.js

@@ -0,0 +1,9 @@
+// Remove empty lines before a node. Mutates the node.
+export function removeEmptyLinesBefore(
+  node /*: postcss$node*/,
+  newline /*: '\n' | '\r\n'*/
+) /*: postcss$node*/ {
+  node.raws.before = node.raws.before.replace(/(\r?\n\s*\r?\n)+/g, newline);
+
+  return node;
+}