Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -281,6 +281,7 @@ testRule(rule, {
config: ["never"],
syntax: "scss",
skipBasicChecks: true,
fix: true,

accept: [
{
Expand Down Expand Up @@ -328,6 +329,10 @@ testRule(rule, {
code: `
// comment

/// comment
`,
fixed: `
// comment
/// comment
`,
message: messages.rejected
Expand All @@ -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
},
Expand All @@ -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
}
]
Expand Down
21 changes: 6 additions & 15 deletions 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");

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down
44 changes: 44 additions & 0 deletions 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();
}
52 changes: 52 additions & 0 deletions 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();
}
15 changes: 15 additions & 0 deletions 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;
}
10 changes: 6 additions & 4 deletions 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";
Expand All @@ -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";
9 changes: 9 additions & 0 deletions 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;
}