css: report parser warnings with 1-based line numbers

src/ast/lib.rs

@@ -1862,23 +1862,13 @@ impl Log {
         )
     }
 
+    /// For producers that already know the (1-based) line and column of the
+    /// warning but have no `Source` to compute a `Range` against, such as the
+    /// CSS parser.
     #[cold]
-    pub fn add_warning_fmt_line_col(
+    pub fn add_warning_fmt_with_location(
         &mut self,
-        filepath: Str,
-        line: u32,
-        col: u32,
-        args: fmt::Arguments<'_>,
-    ) {
-        self.add_warning_fmt_line_col_with_notes(filepath, line, col, args, Box::default())
-    }
-
-    #[cold]
-    pub fn add_warning_fmt_line_col_with_notes(
-        &mut self,
-        filepath: Str,
-        line: u32,
-        col: u32,
+        location: Location,
         args: fmt::Arguments<'_>,
         notes: Box<[Data]>,
     ) {
@@ -1887,18 +1877,9 @@ impl Log {
         }
         self.warnings += 1;
 
-        // TODO: do this properly
-
         let data = Data {
             text: alloc_print(args),
-            location: Some(Location {
-                // `Location.file` borrows the lifetime-erased `Str`; see the
-                // module-level OWNERSHIP note.
-                file: Cow::Borrowed(filepath),
-                line: i32::try_from(line).expect("int cast"),
-                column: i32::try_from(col).expect("int cast"),
-                ..Default::default()
-            }),
+            location: Some(location),
         }
         .clone_line_text(self.clone_line_text);
 

src/css/css_parser.rs

@@ -153,15 +153,6 @@ impl VendorPrefix {
 pub use crate::SourceLocation;
 
 impl SourceLocation {
-    pub(crate) fn to_logger_location(self, file: &'static [u8]) -> bun_ast::Location {
-        bun_ast::Location {
-            file: std::borrow::Cow::Borrowed(file),
-            line: i32::try_from(self.line).expect("int cast"),
-            column: i32::try_from(self.column).expect("int cast"),
-            ..Default::default()
-        }
-    }
-
     /// Create a new BasicParseError at this location for an unexpected token
     pub(crate) fn new_basic_unexpected_token_error(self, token: Token) -> ParseError<ParserError> {
         BasicParseError {
@@ -2941,41 +2932,38 @@ pub struct ParserOptions<'a> {
 }
 
 impl<'a> ParserOptions<'a> {
-    pub(crate) fn warn(&self, warning: &ParseError<ParserError>) {
-        if let Some(lg) = self.logger {
-            // SAFETY: `logger` was constructed from a unique `&'a mut Log` (see
-            // `default`); the pointee outlives `'a` and no other borrow of the
-            // Log exists for the duration of parsing.
-            let lg: &mut Log = unsafe { &mut *lg.as_ptr() };
-            lg.add_warning_fmt_line_col(
-                self.filename,
-                warning.location.line,
-                warning.location.column,
-                format_args!("{}", warning.kind),
-            );
+    /// Every warning (and warning note) crosses into the logger through here,
+    /// like errors do through `ErrorLocation::to_location`: `SourceLocation`
+    /// lines are 0-based, `bun_ast::Location` lines are 1-based.
+    pub(crate) fn logger_location(&self, loc: SourceLocation) -> bun_ast::Location {
+        bun_ast::Location {
+            file: std::borrow::Cow::Borrowed(self.filename),
+            line: i32::try_from(loc.line + 1).expect("int cast"),
+            column: i32::try_from(loc.column).expect("int cast"),
+            ..Default::default()
         }
     }
 
-    pub(crate) fn warn_fmt(&self, args: fmt::Arguments<'_>, line: u32, column: u32) {
-        if let Some(lg) = self.logger {
-            // SAFETY: see `warn` — `logger` carries `*mut Log` provenance from a
-            // unique `&'a mut Log`; no other borrow exists during this call.
-            let lg: &mut Log = unsafe { &mut *lg.as_ptr() };
-            lg.add_warning_fmt_line_col(self.filename, line, column, args);
-        }
+    pub(crate) fn warn(&self, warning: &ParseError<ParserError>) {
+        self.warn_fmt(format_args!("{}", warning.kind), warning.location);
+    }
+
+    pub(crate) fn warn_fmt(&self, args: fmt::Arguments<'_>, loc: SourceLocation) {
+        self.warn_fmt_with_notes(args, loc, Box::default());
     }
 
     pub(crate) fn warn_fmt_with_notes(
         &self,
         args: fmt::Arguments<'_>,
-        line: u32,
-        column: u32,
+        loc: SourceLocation,
         notes: Box<[bun_ast::Data]>,
     ) {
         if let Some(lg) = self.logger {
-            // SAFETY: see `warn`.
+            // SAFETY: `logger` was constructed from a unique `&'a mut Log` (see
+            // `default`); the pointee outlives `'a` and no other borrow of the
+            // Log exists for the duration of parsing.
             let lg: &mut Log = unsafe { &mut *lg.as_ptr() };
-            lg.add_warning_fmt_line_col_with_notes(self.filename, line, column, args, notes);
+            lg.add_warning_fmt_with_location(self.logger_location(loc), args, notes);
         }
     }
 

src/css/declaration.rs

@@ -357,20 +357,18 @@ where
                 css::ComposesState::DisallowNested(info) => {
                     options.warn_fmt(
                         format_args!("\"composes\" is not allowed inside nested selectors"),
-                        info.line,
-                        info.column,
+                        info,
                     );
                 }
                 css::ComposesState::DisallowNotSingleClass(info) => {
                     options.warn_fmt_with_notes(
                         format_args!("\"composes\" only works inside single class selectors"),
-                        source_location.line,
-                        source_location.column,
+                        source_location,
                         Box::new([bun_ast::Data {
                             text: b"The parent selector is not a single class selector because of the syntax here:"
                                 .as_slice()
                                 .into(),
-                            location: Some(info.to_logger_location(options.filename)),
+                            location: Some(options.logger_location(info)),
                         }]),
                     );
                 }

test/bundler/css/css-modules.test.ts

@@ -1,3 +1,6 @@
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
+import { join } from "node:path";
 import { itBundled } from "../expectBundled";
 
 describe("css", () => {
@@ -213,3 +216,131 @@ describe("css", () => {
     },
   });
 });
+
+describe.concurrent("css parser warning locations", () => {
+  // The CSS parser tracks lines 0-based internally. Parse errors are converted
+  // to the logger's 1-based lines; warnings (and their notes) must be too.
+  test("warnings and notes report 1-based lines, like errors", async () => {
+    using dir = tempDir("css-module-warnings", {
+      "styles.module.css": [
+        /*  1 */ "@unknownrule foo;",
+        /*  2 */ ".base { color: red; }",
+        /*  3 */ ".outer {",
+        /*  4 */ "  .inner {",
+        /*  5 */ "    composes: base;",
+        /*  6 */ "  }",
+        /*  7 */ "}",
+        /*  8 */ ".a .b {",
+        /*  9 */ "  color: blue;",
+        /* 10 */ "  composes: base;",
+        /* 11 */ "}",
+        /* 12 */ ".c::custom-thing { color: green; }",
+        /* 13 */ "@other bar;",
+      ].join("\n"),
+    });
+
+    const result = await Bun.build({
+      entrypoints: [join(String(dir), "styles.module.css")],
+      throw: false,
+    });
+
+    const toPlain = (log: { message: string; position: Position | null }) => ({
+      message: log.message,
+      file: log.position!.file,
+      line: log.position!.line,
+      column: log.position!.column,
+    });
+    // The `.notes` getter is not declared in bun-types yet.
+    type WithNotes = BuildMessage & { notes: BuildMessage[] };
+    const warnings = result.logs
+      // The misplaced `composes` declarations also make printing the stylesheet fail.
+      .filter(log => log.level !== "error")
+      .map(log => ({ ...toPlain(log), notes: (log as WithNotes).notes.map(toPlain) }));
+
+    expect(warnings).toEqual([
+      {
+        message: "invalid @ rule encountered: '@unknownrule'",
+        file: "styles.module.css",
+        line: 1,
+        column: 13,
+        notes: [],
+      },
+      {
+        message: '"composes" is not allowed inside nested selectors',
+        file: "styles.module.css",
+        line: 4,
+        column: 3,
+        notes: [],
+      },
+      {
+        message: '"composes" only works inside single class selectors',
+        file: "styles.module.css",
+        line: 10,
+        column: 12,
+        notes: [
+          {
+            message: "The parent selector is not a single class selector because of the syntax here:",
+            file: "styles.module.css",
+            line: 8,
+            column: 1,
+          },
+        ],
+      },
+      {
+        message: "Invalid selector. Unsupported pseudo-class or pseudo-element 'custom-thing'",
+        file: "styles.module.css",
+        line: 12,
+        column: 4,
+        notes: [],
+      },
+      {
+        message: "invalid @ rule encountered: '@other'",
+        file: "styles.module.css",
+        line: 13,
+        column: 7,
+        notes: [],
+      },
+    ]);
+  });
+
+  test("warnings in a stylesheet that is not a CSS module report 1-based lines", async () => {
+    using dir = tempDir("css-warnings", {
+      "styles.css": "@unknownrule foo;\n.a { color: red; }\n\n@other bar;\n",
+    });
+
+    const result = await Bun.build({
+      entrypoints: [join(String(dir), "styles.css")],
+      throw: false,
+    });
+
+    expect(result.logs.map(log => [log.message, log.position!.line, log.position!.column])).toEqual([
+      ["invalid @ rule encountered: '@unknownrule'", 1, 13],
+      ["invalid @ rule encountered: '@other'", 4, 7],
+    ]);
+    expect(result.success).toBe(true);
+  });
+
+  test("bun build prints warning locations as file:line:column", async () => {
+    using dir = tempDir("css-module-warnings-cli", {
+      "styles.module.css": "@unknownrule foo;\n.a { color: red; }\n@other bar;\n",
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "build", "./styles.module.css"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "ignore",
+      stderr: "pipe",
+    });
+    const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+
+    expect(normalizeBunSnapshot(stderr, String(dir))).toMatchInlineSnapshot(`
+      "warn: invalid @ rule encountered: '@unknownrule'
+         at styles.module.css:1:13
+
+      warn: invalid @ rule encountered: '@other'
+         at styles.module.css:3:7"
+    `);
+    expect(exitCode).toBe(0);
+  });
+});