Repository: postcss/postcss-import
Pull request #545 (2023-10-03 18:01:56)

parser fixes and test coverage
This makes the parser for `@import` preludes a bit stricter.


Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ca3a969..a6bf20f 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -14,7 +14,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
+      uses: actions/setup-node@v4
       with:
         node-version: ${{ matrix.node-version }}
     - run: npm install
diff --git a/lib/parse-statements.js b/lib/parse-statements.js
index 88a90eb..00acfc3 100644
--- a/lib/parse-statements.js
+++ b/lib/parse-statements.js
@@ -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)
   }
 
@@ -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 {
@@ -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


Test patch:
diff --git a/test/lint.js b/test/lint.js
index 743b1b9..fc6d9bf 100644
--- a/test/lint.js
+++ b/test/lint.js
@@ -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 })
@@ -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 })