From fd891610a54efd3da8f6cd3f95af189257f13810 Mon Sep 17 00:00:00 2001 From: krister Date: Tue, 30 Apr 2019 22:42:55 +0300 Subject: [PATCH] double-slash-comment-empty-line-before: bug with "never" option autofix --- .../__tests__/index.js | 71 +++++++++++++++++++ .../index.js | 21 ++---- .../__tests__/addEmptyLineBefore.test.js | 44 ++++++++++++ .../__tests__/removeEmptyLineBefore.test.js | 52 ++++++++++++++ src/utils/addEmptyLineBefore.js | 15 ++++ src/utils/index.js | 10 +-- src/utils/removeEmptyLinesBefore.js | 9 +++ 7 files changed, 203 insertions(+), 19 deletions(-) create mode 100644 src/utils/__tests__/addEmptyLineBefore.test.js create mode 100644 src/utils/__tests__/removeEmptyLineBefore.test.js create mode 100644 src/utils/addEmptyLineBefore.js create mode 100644 src/utils/removeEmptyLinesBefore.js diff --git a/src/rules/double-slash-comment-empty-line-before/__tests__/index.js b/src/rules/double-slash-comment-empty-line-before/__tests__/index.js index d9b24fa5..2361aac3 100644 --- a/src/rules/double-slash-comment-empty-line-before/__tests__/index.js +++ b/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 } ] diff --git a/src/rules/double-slash-comment-empty-line-before/index.js b/src/rules/double-slash-comment-empty-line-before/index.js index c694264e..d0d97309 100644 --- a/src/rules/double-slash-comment-empty-line-before/index.js +++ b/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; } diff --git a/src/utils/__tests__/addEmptyLineBefore.test.js b/src/utils/__tests__/addEmptyLineBefore.test.js new file mode 100644 index 00000000..3cdc05c7 --- /dev/null +++ b/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(); +} diff --git a/src/utils/__tests__/removeEmptyLineBefore.test.js b/src/utils/__tests__/removeEmptyLineBefore.test.js new file mode 100644 index 00000000..cf75b49e --- /dev/null +++ b/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(); +} diff --git a/src/utils/addEmptyLineBefore.js b/src/utils/addEmptyLineBefore.js new file mode 100644 index 00000000..f459c9a2 --- /dev/null +++ b/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; +} diff --git a/src/utils/index.js b/src/utils/index.js index 1014d62b..df0f4407 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -1,3 +1,4 @@ +export { addEmptyLineBefore } from "./addEmptyLineBefore"; export { default as atRuleParamIndex } from "./atRuleParamIndex"; export { default as beforeBlockString } from "./beforeBlockString"; export { default as blockString } from "./blockString"; @@ -5,11 +6,11 @@ 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"; diff --git a/src/utils/removeEmptyLinesBefore.js b/src/utils/removeEmptyLinesBefore.js new file mode 100644 index 00000000..250f87f8 --- /dev/null +++ b/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; +}