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

parser fixes and test coverage #545

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 52 additions & 10 deletions lib/parse-statements.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,39 @@ function parseCharset(result, atRule, from) {

function parseImport(result, atRule, from) {
let prev = atRule.prev()

// `@import` statements may follow other `@import` statements.
if (prev) {
do {
if (
prev.type !== "comment" &&
(prev.type !== "atrule" ||
(prev.name !== "import" &&
prev.name !== "charset" &&
!(prev.name === "layer" && !prev.nodes)))
prev.type === "comment" ||
(prev.type === "atrule" && prev.name === "import")
) {
return result.warn(
"@import must precede all other statements (besides @charset or empty @layer)",
{ node: atRule }
)
prev = prev.prev()
continue
}
prev = prev.prev()

break
} while (prev)
}

// All `@import` statements may be preceded by `@charset` or `@layer` statements.
// But the `@import` statements must be consecutive.
if (prev) {
do {
if (
prev.type === "comment" ||
(prev.type === "atrule" &&
(prev.name === "charset" || (prev.name === "layer" && !prev.nodes)))
) {
prev = prev.prev()
continue
}

return result.warn(
"@import must precede all other statements (besides @charset or empty @layer)",
{ node: atRule }
)
} while (prev)
}

Expand Down Expand Up @@ -177,6 +195,21 @@ function parseImport(result, atRule, from) {
(node.type === "word" || node.type === "function") &&
/^layer$/i.test(node.value)
) {
if (stmt.layer.length > 0) {
return result.warn(`Multiple layers in '${atRule.toString()}'`, {
node: atRule,
})
}

if (stmt.supports.length > 0) {
return result.warn(
`layers must be defined before support conditions in '${atRule.toString()}'`,
{
node: atRule,
}
)
}

if (node.nodes) {
stmt.layer = [stringify(node.nodes)]
} else {
Expand All @@ -187,6 +220,15 @@ function parseImport(result, atRule, from) {
}

if (node.type === "function" && /^supports$/i.test(node.value)) {
if (stmt.supports.length > 0) {
return result.warn(
`Multiple support conditions in '${atRule.toString()}'`,
{
node: atRule,
}
)
}

stmt.supports = [stringify(node.nodes)]

continue
Expand Down
75 changes: 75 additions & 0 deletions test/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,27 @@ test("should warn if non-empty @layer before @import", t => {
})
})

test("should warn when import statements are not consecutive", t => {
return processor
.process(
`
@import "bar.css";
@layer a;
@import "bar.css";
`,
{ from: "test/fixtures/imports/foo.css" }
)
.then(result => {
t.plan(1)
result.warnings().forEach(warning => {
t.is(
warning.text,
"@import must precede all other statements (besides @charset or empty @layer)"
)
})
})
})

test("should not warn if empty @layer before @import", t => {
return processor
.process(`@layer a; @import "";`, { from: undefined })
Expand Down Expand Up @@ -221,6 +242,60 @@ test("should warn on unimplemented features", t => {
})
})

test("should warn on multiple layer clauses", t => {
return processor
.process(
`
@import url('foo') layer layer(bar);
`,
{ from: undefined }
)
.then(result => {
const warnings = result.warnings()
t.is(warnings.length, 1)
t.is(
warnings[0].text,
`Multiple layers in '@import url('foo') layer layer(bar)'`
)
})
})

test("should warn on when support conditions precede layer clauses", t => {
return processor
.process(
`
@import url('foo') supports(selector(&)) layer(bar);
`,
{ from: undefined }
)
.then(result => {
const warnings = result.warnings()
t.is(warnings.length, 1)
t.is(
warnings[0].text,
`layers must be defined before support conditions in '@import url('foo') supports(selector(&)) layer(bar)'`
)
})
})

test("should warn on multiple support conditions", t => {
return processor
.process(
`
@import url('foo') supports(selector(&)) supports((display: grid));
`,
{ from: undefined }
)
.then(result => {
const warnings = result.warnings()
t.is(warnings.length, 1)
t.is(
warnings[0].text,
`Multiple support conditions in '@import url('foo') supports(selector(&)) supports((display: grid))'`
)
})
})

test("should not warn when a user closed an import with ;", t => {
return processor
.process(`@import url('http://');`, { from: undefined })
Expand Down