Skip to content

Commit

Permalink
Fix isFirstNested util to handle shared-line comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nwoltman committed Aug 22, 2017
1 parent 58c7853 commit 5ec4ed3
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 6 deletions.
9 changes: 9 additions & 0 deletions lib/rules/at-rule-empty-line-before/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,10 @@ testRule(
accept: [
{
code: "a {\n @mixin foo;\n color: pink;\n}"
},
{
code: "a { /* comment */\n @mixin foo;\n color: pink;\n}",
description: "shared-line comment"
}
],

Expand All @@ -358,6 +362,11 @@ testRule(
code: "a {\n\n @mixin foo;\n color: pink;\n}",
fixed: "a {\n @mixin foo;\n color: pink;\n}",
message: messages.rejected
},
{
code: "a {/* comment */\n\n @mixin foo;\n color: pink;\n}",
fixed: "a {/* comment */\n @mixin foo;\n color: pink;\n}",
message: messages.rejected
}
]
})
Expand Down
16 changes: 16 additions & 0 deletions lib/rules/comment-empty-line-before/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ testRule(
{
code: "a {\n /* comment */\n color: pink;\n}",
description: "first-nested without empty line before"
},
{
code:
"a { /* shared-line comment */\n /* comment */\n color: pink;\n}",
description:
"first-nested without empty line before and shared-line comment"
}
],

Expand All @@ -114,6 +120,16 @@ testRule(
message: messages.rejected,
line: 3,
column: 3
},
{
code:
"a { /* shared-line comment */\n\n /* comment */\n color: pink;\n}",
fixed:
"a { /* shared-line comment */\n /* comment */\n color: pink;\n}",
description: "first-nested with empty line before",
message: messages.rejected,
line: 3,
column: 3
}
]
})
Expand Down
11 changes: 11 additions & 0 deletions lib/rules/custom-property-empty-line-before/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ testRule(rule, {
},
{
code: "a {\r\n --custom-prop: value;\r\n}"
},
{
code: "a { /* comment */\n --custom-prop: value;\n}",
description: "shared-line comment"
}
],

Expand All @@ -188,6 +192,13 @@ testRule(rule, {
message: messages.rejected,
line: 3,
column: 2
},
{
code: "a { /* comment*/\n\n --custom-prop: value;\n}",
fixed: "a { /* comment*/\n --custom-prop: value;\n}",
message: messages.rejected,
line: 3,
column: 2
}
]
});
Expand Down
11 changes: 11 additions & 0 deletions lib/rules/declaration-empty-line-before/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,10 @@ testRule(rule, {
},
{
code: "a {\r\n top: 15px;\r\n}"
},
{
code: "a { /* comment */\n top: 15px;\n}",
description: "shared-line comment"
}
],

Expand All @@ -382,6 +386,13 @@ testRule(rule, {
message: messages.rejected,
line: 3,
column: 2
},
{
code: "a { /* comment */\n\n top: 15px;\n}",
fixed: "a { /* comment */\n top: 15px;\n}",
message: messages.rejected,
line: 3,
column: 2
}
]
});
Expand Down
14 changes: 14 additions & 0 deletions lib/rules/rule-empty-line-before/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ testRule(rule, {
code: "@media {\r\n a {}\r\n\r\n}",
description: "CRLF"
},
{
code: "@media { /* comment */\n a {}\n\n}",
description: "shared-line comment boog"
},
{
code: "@media {\n a {}\n\n b{}\n\n}"
},
Expand Down Expand Up @@ -268,6 +272,16 @@ testRule(rule, {
fixed: "@media {\n a {}\n}",
message: messages.rejected
},
{
code: "@media { /* comment */\n\n a {}\n}",
fixed: "@media { /* comment */\n a {}\n}",
message: messages.rejected
},
{
code: "@media { /* comment */\n\n a {}\n}",
fixed: "@media { /* comment */\n a {}\n}",
message: messages.rejected
},
{
code: "@media {\n\n a {}\n\n b{}\n}",
fixed: "@media {\n a {}\n\n b{}\n}",
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/rule-empty-line-before/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const addEmptyLineBefore = require("../../utils/addEmptyLineBefore");
const hasEmptyLine = require("../../utils/hasEmptyLine");
const isFirstNested = require("../../utils/isFirstNested");
const isSingleLineString = require("../../utils/isSingleLineString");
const isStandardSyntaxRule = require("../../utils/isStandardSyntaxRule");
const optionsMatches = require("../../utils/optionsMatches");
Expand Down Expand Up @@ -86,7 +87,7 @@ const rule = function(expectation, options, context) {
// Optionally reverse the expectation for the first nested node
if (
optionsMatches(options, "except", "first-nested") &&
rule === rule.parent.first
isFirstNested(rule)
) {
expectEmptyLineBefore = !expectEmptyLineBefore;
}
Expand Down
57 changes: 57 additions & 0 deletions lib/utils/__tests__/isFirstNested.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ describe("isFirstNested", () => {
@media (min-width: 0px) {
a { color: 'pink'; }
}
@media (min-width: 0px) { /* comment */
a { color: 'pink'; }
}
`);

root.walkRules(rule => {
Expand All @@ -29,18 +32,39 @@ describe("isFirstNested", () => {
a {
@include foo;
}
b { /* comment */
@include foo;
}
`);

root.walkAtRules(atRule => {
expect(isFirstNested(atRule)).toBe(true);
});
});

it("returns true with the first-nested declaration", () => {
const root = postcss.parse(`
a {
color: pink;
}
b { /* comment */
color: pink;
}
`);

root.walkDecls(atRule => {
expect(isFirstNested(atRule)).toBe(true);
});
});

it("returns true with first-nested non-statement", () => {
const root = postcss.parse(`
a {
/* comment */
}
b { /* shared-line comment */
/* comment */
}
`);

root.walkComments(comment => {
Expand All @@ -54,6 +78,10 @@ describe("isFirstNested", () => {
a { color: 'pink'; }
b { color: 'pink'; }
}
@media (min-width: 0px) {
/* comment */
b { color: 'pink'; }
}
`);

root.walkRules("b", rule => {
Expand All @@ -67,10 +95,39 @@ describe("isFirstNested", () => {
@include foo;
@expect bar;
}
b {
/* comment */
@expect bar;
}
`);

root.walkAtRules("expect", atRule => {
expect(isFirstNested(atRule)).toBe(false);
});
});

it("returns false with not-first-nested declaration", () => {
const root = postcss.parse(`
a {
font-size: 0;
color: pink;
}
b {
/* comment */
color: pink;
}
c { /* comment */ /* comment */
/* comment */
color: pink;
}
d { /* shared-line/multi-line
comment */
color: pink;
}
`);

root.walkDecls("color", atRule => {
expect(isFirstNested(atRule)).toBe(false);
});
});
});
42 changes: 37 additions & 5 deletions lib/utils/isFirstNested.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,41 @@
module.exports = function(statement /*: postcss$node*/) /*: boolean*/ {
const parentNode = statement.parent;

return (
parentNode !== undefined &&
parentNode.type !== "root" &&
statement === parentNode.first
);
if (parentNode === undefined || parentNode.type === "root") {
return false;
}

if (statement === parentNode.first) {
return true;
}

/*
* Search for the statement in the parent's nodes, ignoring comment
* nodes on the same line as the parent's opening brace.
*/

const parentNodes = parentNode.nodes;
const firstNode = parentNodes[0];

if (firstNode.type !== "comment" || firstNode.raws.before.includes("\n")) {
return false;
}

const openingBraceLine = firstNode.source.start.line;

if (openingBraceLine !== firstNode.source.end.line) {
return false;
}

for (let i = 1; i < parentNodes.length; i++) {
const node = parentNodes[i];

if (node === statement) {
return true;
}

if (node.type !== "comment" || node.source.end.line !== openingBraceLine) {
return false;
}
}
};

0 comments on commit 5ec4ed3

Please sign in to comment.