Repository navigation
Conversation
…equence An error at the end of a file with CR LF line ends was reported one line past the last line, at column 0, with an empty line text and no code frame. The scan stopped on the LF of the final pair, after it had read the CR. - error_stop_offset replaces clamp_error_offset. It moves every Loc to the first byte of the unit that holds it: the CR of a CR LF pair, the first byte of a UTF-8 sequence. Location::init_or_null_impl calls it once. - The CR arm of the scanner starts the next line at column 1, as the LF, U+2028 and U+2029 arms do. A file with lone CR line ends no longer has columns that count from 0. - LineColumnTracker no longer rescans from byte 0 for an offset on a continuation byte: no scan stops inside a sequence now. - bun_core::strings::wtf8_codepoint_start is the code_point_start of the lint's regex subject, moved so that the logger can call it too.
A warning of the CSS parser had no file for a plain sheet, no line text, and a 0-based line. ParserOptions::warn* had a line and a column only, and logged through Log::add_warning_fmt_line_col, the one logger entry that takes no Source. The parse entry points now take the Source, and each warn site passes the byte offset of the tokenizer to ParserOptions::warn_at. The logger builds the file, the namespace, the line, the column and the line text, as it does for the diagnostics of every other parser. Warnings go through the new Log::add_warning_fmt_no_long_excerpt. It drops an excerpt of 120 bytes or more: the logger cuts such a line around the position and pads the caret by the whole column, so the warnings of a one-line sheet would print in the order of N squared bytes. ParserOptions::warn(&ParseError) stays for the errors that error_recovery skips. A ParseError has no byte position, so such a warning has the file and no line. The warning for a nested "composes" is now at the declaration. It was at the start of a rule, and not always of the rule that holds the declaration. Log::add_warning_fmt_line_col, Log::add_warning_fmt_line_col_with_notes and SourceLocation::to_logger_location are deleted.
|
Updated 4:52 PM PT - Oct 17th, 2026
✅ @robobun, your commit 7bf7a070f6a1f33186027e09015755de621be0fe passed in 🧪 To try this PR locally: bunx bun-pr 45787That installs a local version of the PR into your bun-45787 --bun |
|
Status Reproduced with This PR is stacked on #45554. Its first two commits are that PR. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe change normalizes diagnostic byte offsets for line endings and UTF-8 characters. CSS parsing now uses ChangesDiagnostic location handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: CSS parser warnings now use the Source and a byte position for logging. |
| Description check | |
The description is detailed and covers the problem, implementation, verification, tradeoffs, scope, and follow-up work. It does not use the template headings exactly, but it provides the required info… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description is detailed and covers the problem, implementation, verification, tradeoffs, scope, and follow-up work. It does not use the template headings exactly, but it provides the required information.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked: the move of code_point_start to bun_core::strings::wtf8_codepoint_start (src/bun_core/string/immutable/wtf8_text.rs) is behavior-identical for the lint regex caller, since the old end-of-text length of 1 and the new 0 both fail the > at - start test; error_stop_offset's offset - 1 cannot underflow because a two-byte line break ending at offset implies offset >= 1; and warn(&ParseError) logging at Loc::EMPTY is unreachable today because nothing sets error_recovery.
Extended reasoning...
The change reworks the diagnostics logger's offset location (src/ast/lib.rs), threads a bun_ast::Source through the CSS parser's entry points and all five producers, and replaces the line/column warning API with byte-position warn_at/warn_with_note; it touches no auth, crypto or injection surface. One nit finding is posted inline; the ruled-out items above are the concrete checks made beyond it.
| // When `Location::init_or_null` cuts a line on the left, it keeps 40 bytes before the | ||
| // position and 80 after it. A shorter excerpt starts where its line starts. | ||
| const CUT_EXCERPT_MIN: usize = 40 + 80; |
There was a problem hiding this comment.
🟡 nit (optional): maintainers get a second copy of the excerpt window that can silently drift from the one it mirrors. CUT_EXCERPT_MIN = 40 + 80 at src/ast/lib.rs:1987 restates the literals saturating_sub(40) and + 80 in init_or_null_impl (src/ast/lib.rs:833-835), and the doc comment repeats 80 again. If the window there changes, the drop rule here keeps cut excerpts with a misaligned caret or drops whole ones. Fix: one source of truth for the window — name the before/after sizes once next to init_or_null_impl (e.g. EXCERPT_BEFORE/EXCERPT_AFTER), use them in the window code, and derive CUT_EXCERPT_MIN from their sum.
Why this was flagged
The new entry add_warning_fmt_no_long_excerpt decides whether an excerpt was cut on the left by comparing its length with CUT_EXCERPT_MIN (src/ast/lib.rs:1998), which is hard-coded as 40 + 80 (src/ast/lib.rs:1987). The window it reasons about is built from the separate literals 40 and 80 in Location::init_or_null_impl (src/ast/lib.rs:833-835); nothing ties the two together. A later change to either literal (say 60 before the position) makes the comparison wrong: a left-cut excerpt of 140 bytes is still dropped, but a whole-line excerpt of 120..139 bytes is now dropped too, or with a smaller window a left-cut excerpt is kept and write_format pads the caret by the whole column past the end of the cut text (src/ast/lib.rs:1015-1018). On the base branch there is no such mirrored constant. REVIEW.md asks for one source of truth and named constants derived from what they describe.
Verification: nit. Triggering condition: only when someone later changes the excerpt window in Location::init_or_null_impl without also editing the mirrored constant CUT_EXCERPT_MIN (src/ast/lib.rs:1987). The window it mirrors is built from independent unnamed literals in init_or_null_impl (src/ast/lib.rs:833-835), so nothing ties the two sites together. Nothing breaks on the base or on this head.
This branch has not been deployed
2 participants
Stacked on #45554 (the first two commits). Merge it first.
Problem
bun build ./a.cssprintswarn: invalid @ rule encountered: '@unknown-rule'with noat file:line:column. InBun.buildlogsposition.fileandlineTextare"". No user reported this.ParserOptions::warn*(src/css/css_parser.rs:2909) logs throughLog::add_warning_fmt_line_col, which takes a line, a column and noSource.Fix
Source. Each warn site passes a byte position, and the logger builds file, 1-based line, column andlineText.Log::add_warning_fmt_no_long_excerptdrops an excerpt of 120 bytes or more and keeps the place.composeswarning moves to the declaration.test/bundler/css/css-modules.test.ts(19 new tests fail on main),test/bake/dev/css.test.ts. Self-reviewed: 16 concerns raised, 15 addressed. Rejected: a row pinning a falsecomposeswarning (css modules: keep composes working after a nested rule in the same block #38520).Background
lineText.Downsides
lineTexton lines of 120 bytes or more. CSS errors keep their own positions.ParserOptions: 128 bytes, was 120. Release text: +256 bytes.Notes
Before and after (
.a { color: red }\n.b { color: blue }\n@unknown-rule x;\n,Bun.buildlogs):The threads that quote these warnings (#12878, #20619) are about Tailwind directives, and this PR prints more lines for each such warning.
Why it is stacked. The logger's
LineColumnTrackerscans from byte 0 for a position that is a UTF-8 continuation byte. CSS warnings do not use the logger's scanner on main. With this change alone, 20,000 warnings on stray continuation bytes in a 400 KB sheet take 5.5 s (main: 0.09 s). #45554 removes that scan and also counts columns from 1 after a lone CR, which the CSS tokenizer treats as a line break. On top of #45554 the same sheet takes 83 ms (77 to 85 ms in 5 runs, base 69 to 94 ms). The first two commits here are #45554, cherry-picked onto main. When #45554 merges, a rebase leaves one commit.error_recoverystays.ParserOptions.error_recoveryand its five readers are not changed, andwarn(&ParseError)keeps its name: the draft #44833 sets the option. AParseErrorhas a line and a column and no byte position, so a warning for an error that the option skips has the file and no line. Nothing sets the option on main, so no test reaches that function. The warn function with a byte position iswarn_at.Warn sites. Unknown at-rule,
@nest, the four unsupported pseudo-class and pseudo-element sites, the twocomposeswarnings and thecomposesnote. The texts do not change. The simple pseudo-class parser gets the byte offset as one more argument. A declaration no longer readscurrent_source_location().Producers.
ParseTask(bundler, dev server, HTML entries),Transpiler::build_css_output(bun build --no-bundle),cssInternals,bun pm diff.ParserOptions.sourceis private and the parse entries set it, so a producer cannot log a CSS warning without its sheet.Long lines.
Location::init_or_null(src/ast/lib.rs) keeps the whole line when the position is in its last 80 bytes. Otherwise it keeps 40 bytes before the position and 80 after.Locationdoes not record where a cut starts, socolumndoes not index a cutlineText, andwidth_beforepads the caret by the whole column. Without the new entry a one-line sheet with 100, 200 and 400 warnings prints 197, 734 and 2,860 KB. The entry drops an excerpt of 120 bytes or more: every excerpt that is cut on the left is at least that long, and a shorter one starts where its line starts. With it the same sheets print 10.0, 19.9 and 39.8 KB (main: 5.1, 10.2 and 20.4 KB, with no place). The entry isadd_warning_fmt_no_excerptwith that one exception. It stays until the logger itself stores a line whole up to a bound and nothing of a longer line, which is the rulebun checkalready has (MAX_SHOWN_LINE,src/sema/driver/lib.rs:1226). #41658 records the start column of a cut and does not bound the whole-line case.composes. The "not allowed inside nested selectors" warning was at the start of a rule. For.outer { @media (min-width: 1px) { composes: base } }that was the start of.outer. It is now at the declaration, as the "single class selectors" warning already is.ComposesState::AllowandDisallowNestedno longer carry a position. Bothcomposeswarnings are still followed byerror: Failed to generate CSS for this file (PrintError)with only the file name (#38538 removes that error).Dev server. A CSS warning on line 1 had line 0. The overlay client reads line 0 as "no position" (
src/runtime/bake/client/error-serialization.ts:70), while the server writes the rest of the position for every line that is not negative (src/runtime/bake/dev_server/serialized_failure.rs:208), so the client fails to decode the payload. The new test intest/bake/dev/css.test.tsfails that way on main. This PR covers the CSS producer only. Two other producers still send line 0 (src/ast_jsc/lib.rs:30,src/bundler/ParseTask.rs:1823), and the change to the writer is in #39488.Positions against the tokenizer. The warning has the tokenizer's line + 1 and column in 14 of 19 sheets (LF, CRLF, lone CR, first line, no final line break, tabs, comments, nesting, multi-byte characters, BOM). The 5 that differ:
The logger's line breaks are those of JavaScript. After U+2028 or U+2029 the
lineTextstarts with the last byte of that character, as for JavaScript diagnostics (#41658 has the repair).Measurements. Release builds, linux x64, of #45554's head
be91335a6c8(base) and of that commit with this PR's commit.perfandvalgrindare not available on the machine, so instruction counts come fromgdb(record full, breakpoint hit counts).size_of(gdb):ParserOptions120 to 128,ComposesState12 to 8,NestedRuleParser<BundlerAtRuleParser>120 to 120.ParserState32,SourceLocation8 andParseError<ParserError>48 do not change.size): 106,331,715 to 106,331,971 bytes.nm -S:add_warning_fmt_line_col_with_notes-691,add_warning_fmt_no_long_excerpt+876,ParserOptions::warn_at+56,warn_with_note+75,RuleBodyParser<NestedRuleParser<BundlerAtRuleParser>>::next5,828 to 5,418,parse_declaration_impl1,227 to 1,140,parse_selector<Selectors>20,775 to 20,893.StyleSheet::parse_with, entry to return,record full): normalize.css 492,509 to 490,257 instructions, hint.css 1,579,640 to 1,572,870, random-gradients.css 967,348 to 966,590, trainer-gallery-holo.css 512,641 to 512,452.mi_*, sheets with 1,000 and 5,000 warnings): 18.1 on the base, 29.4 here. Each hand-over of a log copies the file and the excerpt of a message.lineTextbytes (base: 0): 80,000 warnings one per line, sum 3,497,780, max 44. 80,000 warnings on one line, sum 107 (the one warning in the first 40 bytes). 4,000composesin one rule that starts in the last 80 bytes of a 260 KB line, sum 68,000, max 17: the logger copies that line for each note and the entry drops the copy.Bun.build, median of 5 interleaved runs on a machine that was not idle: 80,000 warnings one per line 307 ms (base 275), 80,000 on one line 325 ms (base 304), 32,001composeswarnings 167 ms (base 156), the 260 KB line sheet 335 ms (base 323), 10,000 warnings on stray continuation bytes 48 ms (base 43).Not changed.
ErrorLocation::to_location(src/css/error.rs:177). AParseErrorhas no byte offset (one more field makes it 56 bytes, from 48), and the logger has no position for the end of the input, which CSS error tests pin (test/bake/dev/css.test.ts:62). A warning and an error of one sheet can differ in the 5 cases above. logger: bound the excerpt printed for an error on a long line #43313 bounds the line text of an error on a long line.bun build --no-bundlelogs CSS parse, minify and print errors with no position (src/bundler/transpiler.rs:3072).ErrorLocation::to_locationborrows the line text from a buffer thatbuild_css_outputfrees when it returns, so that call is not a one-line change.invalidorUnsupportedfor CSS that is valid and that the parser does not know (@view-transition,@position-try,:state(),::highlight()). They now have a caret under them. css: parse :state(), ::highlight(), four css-pseudo-4 pseudo-elements, @position-try and @font-feature-values #42918 and css: parse :active-view-transition-type() and the @view-transition rule #42912 add some of these constructs..a { .b { color: blue } composes: x; }still gets the nestedcomposeswarning, which is false there. The warning is now at that declaration. css modules: keep composes working after a nested rule in the same block #38520 removes it.data:URL has the whole URL asposition.fileof every warning, as a CSS error of such a sheet already has.Locconversion, as the other positions of the CSS parser already do. css: reject stylesheets of 2 GiB or more instead of aborting on an int cast #39113 rejects such a sheet at the entry.Test harness. The CLI backend of
itBundledignores a warning with noatline and fails a test for a located warning that the test does not list. Plain-sheet CSS warnings are now located. No test on main is affected (105 bundler and CSS test files on a release build: the same results as the base).Open PRs that need a change after this.
parse_bundler(bump, contents, ..)insrc/runtime/jsc_hooks.rsmust pass theSource.css/URLInNestedAtImportKeptAsWrittenMinifiedneedsbundleWarnings: { "/entry.css": ["invalid @ rule encountered: '@import'"] }.code.len()in the parse entries, which now take aSource.warn(&ParseError)(they compile and log with no line). css modules: drop a rejected composes instead of failing the file at print time #38538 adds a caller of the removedwarn_fmt. css modules: keep composes working after a nested rule in the same block #38520 and css modules: drop a rejected composes instead of failing the file at print time #38538 edit the samecomposesmatch. css: warn about unknown bare pseudo-classes unless they are vendor prefixed #38572 edits one of the pseudo-class warn sites.add_warning_fmt_line_colin a unit test and edits the sameLogfunctions.test/bake/dev/css.test.ts.Question for a maintainer.
.claude/docs/landing-prs.mdsays a rare-case fix must not add cost to the hot path. This passes one moreusizethrough the simple pseudo-class path and removes the source location read from every style-rule declaration. Is the measured net acceptable?