From 607b5e0f87ed402e0293fe3d64a904aae97a784c Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 07:45:48 +0200 Subject: [PATCH 1/7] chore: document the biome pin, and drop references to a document we do not ship MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dependency audit found nothing to upgrade. Every direct and transitive dependency, Elixir and Rust, is already at its own newest published version: `mix hex.outdated` reports all up to date, and `cargo update --dry-run` locks zero packages. That includes the biome crates, and the reason is worth writing down rather than rediscovering. biome_css_parser, biome_css_syntax and biome_rowan are all at 0.5.8, the newest published. We can be there only because this crate does not depend on biome_css_formatter, which is stuck at 0.5.7 and requires biome_css_syntax ^0.5.7 and biome_rowan ^0.5.7 -- adding it would pull the whole graph back a release. That is exactly why igniter_js, which does format CSS, pins its entire biome set to 0.5.7. Cargo.toml now says so, including the condition that keeps it true: our pretty-printer is written against the CST, not routed through biome_css_formatter. The lock also holds biome_diagnostics_categories, biome_diagnostics_macros and biome_markup at 0.5.7, which is their own latest, and biome_unicode_table at 0.5.9. Nothing is held back. Separately, sixteen doc comments across thirteen files cited "ROADMAP §..." for a document that is not in this repository -- it was a planning note, never committed. Every citation is now self-contained prose. A pointer to something a reader cannot open is worse than no pointer. Co-Authored-By: Claude Opus 5 (1M context) --- native/igniter_css/Cargo.toml | 15 ++++++++++++--- native/igniter_css/src/ctx.rs | 2 +- native/igniter_css/src/edit.rs | 4 ++-- native/igniter_css/src/error.rs | 4 ++-- native/igniter_css/src/locate.rs | 6 +++--- native/igniter_css/src/nif.rs | 3 +-- native/igniter_css/src/ops/at_rule.rs | 2 +- native/igniter_css/src/ops/mod.rs | 2 +- native/igniter_css/src/transform.rs | 2 +- native/igniter_css/src/trivia.rs | 2 +- native/igniter_css/tests/corpus_invariants.rs | 2 +- native/igniter_css/tests/phase0_roundtrip.rs | 2 +- native/igniter_css/tests/property.rs | 2 +- 13 files changed, 28 insertions(+), 20 deletions(-) diff --git a/native/igniter_css/Cargo.toml b/native/igniter_css/Cargo.toml index f456d9c..fe0ec3a 100644 --- a/native/igniter_css/Cargo.toml +++ b/native/igniter_css/Cargo.toml @@ -16,9 +16,18 @@ path = "src/lib.rs" crate-type = ["cdylib", "rlib"] [dependencies] -# Biome CSS crates are Biome-internal and published at 0.5.x with NO API -# stability guarantee. They are pinned with `=` deliberately. Upgrading is a -# tested activity, never a `cargo update`. See ROADMAP §5/§12. +# The biome crates are Biome-internal, published at 0.5.x with no API stability +# guarantee, and they churn between patch releases. Pinned with `=` on purpose: +# upgrading is a deliberate, tested activity, never a `cargo update`. +# +# 0.5.8 is the newest published version of all three, and it is reachable only +# because this crate does not depend on biome_css_formatter. That crate is stuck +# at 0.5.7 and requires biome_css_syntax ^0.5.7 and biome_rowan ^0.5.7, so +# adding it would drag the whole graph back a release -- which is exactly why +# igniter_js, which does format CSS, pins its entire biome set to 0.5.7. +# +# We do not format: IgniterCss.Transform::beautify is a byte-preserving +# pretty-printer written against the CST. Keep it that way, or this pin drops. biome_css_parser = "=0.5.8" biome_css_syntax = "=0.5.8" biome_rowan = "=0.5.8" diff --git a/native/igniter_css/src/ctx.rs b/native/igniter_css/src/ctx.rs index de14d77..0470855 100644 --- a/native/igniter_css/src/ctx.rs +++ b/native/igniter_css/src/ctx.rs @@ -4,7 +4,7 @@ //! `ParseCtx` owns the source string and its lossless parse, plus the handful //! of formatting facts every codemod needs so that inserted text looks like the -//! text the user already wrote (ROADMAP Phase 1, and rules B/C in §8). +//! text the user already wrote. //! //! This module and `locate` are the only two places allowed to name Biome //! types. Isolating them here is the mitigation for R2 (Biome API churn). diff --git a/native/igniter_css/src/edit.rs b/native/igniter_css/src/edit.rs index 9407fe0..9862143 100644 --- a/native/igniter_css/src/edit.rs +++ b/native/igniter_css/src/edit.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! The edit engine (ROADMAP §6, Phase 1). +//! The edit engine. //! //! Every codemod in this crate produces `Vec` -- byte ranges into the //! *original* source plus replacement text -- and never reprints the tree. @@ -110,7 +110,7 @@ pub fn apply_edits(source: &str, mut edits: Vec) -> Result { } /// Drop edits that would not change anything, so a codemod can report -/// `changed: false` honestly (ROADMAP §8 rule A). +/// `changed: false` honestly. pub fn prune_noop_edits(source: &str, edits: Vec) -> Vec { edits .into_iter() diff --git a/native/igniter_css/src/error.rs b/native/igniter_css/src/error.rs index 0194a12..477e52e 100644 --- a/native/igniter_css/src/error.rs +++ b/native/igniter_css/src/error.rs @@ -21,8 +21,8 @@ pub enum CssError { end: usize, len: usize, }, - /// More than one top-level rule matched the selector. ROADMAP §2 rule 4 and - /// §11 R4: error, never guess. + /// More than one top-level rule matched the selector. Error, never guess: + /// picking one is how a codemod produces a surprising diff. AmbiguousSelector { selector: String, count: usize }, /// The caller asked to operate on something that isn't there. NotFound(String), diff --git a/native/igniter_css/src/locate.rs b/native/igniter_css/src/locate.rs index 47cacdd..4b5b1c6 100644 --- a/native/igniter_css/src/locate.rs +++ b/native/igniter_css/src/locate.rs @@ -5,7 +5,7 @@ //! Phase 2: typed queries over the CST that return **byte ranges**, never owned //! strings to be reprinted. //! -//! Matching rules for v1 are deliberately strict (ROADMAP §8 Phase 2): +//! Matching rules are deliberately strict: //! * top-level rules only, unless the caller explicitly opts into descending; //! * selectors compared on a normalised form, never raw equality and never //! substring/fuzzy; @@ -546,7 +546,7 @@ pub fn find_rule_by_selector(ctx: &ParseCtx, selector: &str) -> MatchResult { } /// Same, but searching every rule in the file including nested ones. Callers -/// must opt in explicitly (ROADMAP Phase 2). +/// must opt in explicitly. pub fn find_rule_by_selector_anywhere(ctx: &ParseCtx, selector: &str) -> MatchResult { find_rule_among(find_all_rules(ctx), selector) } @@ -669,7 +669,7 @@ pub fn last_top_level_at_rule_end(ctx: &ParseCtx, names: &[&str]) -> Option usize { let src = ctx.source(); // `ctx.source()` never contains a BOM -- it is stripped at parse time and diff --git a/native/igniter_css/src/nif.rs b/native/igniter_css/src/nif.rs index 99de99e..03fd341 100644 --- a/native/igniter_css/src/nif.rs +++ b/native/igniter_css/src/nif.rs @@ -6,8 +6,7 @@ //! //! **Elixir sends intent; Rust returns text.** The CST is never exported across //! the boundary in any form -- marshalling trees between languages is what made -//! the previous implementation painful, and it buys nothing here (ROADMAP §8 -//! Phase 4). +//! the previous implementation painful, and it buys nothing here. //! //! Two other rules hold throughout this module: //! diff --git a/native/igniter_css/src/ops/at_rule.rs b/native/igniter_css/src/ops/at_rule.rs index 9567f90..9b5766e 100644 --- a/native/igniter_css/src/ops/at_rule.rs +++ b/native/igniter_css/src/ops/at_rule.rs @@ -159,7 +159,7 @@ fn insertion_offset(ctx: &ParseCtx, spec: &AtRuleSpec) -> usize { } /// Insert `line` at the top level unless an equivalent at-rule is already -/// present (ROADMAP §8, `ensure_at_rule_line`). +/// present. pub fn ensure_at_rule_line(source: &str, line: &str, options: ParseOptions) -> Result { let spec = parse_at_rule_spec(line)?; run(source, options, |ctx| { diff --git a/native/igniter_css/src/ops/mod.rs b/native/igniter_css/src/ops/mod.rs index e4f3ae4..3f5bde7 100644 --- a/native/igniter_css/src/ops/mod.rs +++ b/native/igniter_css/src/ops/mod.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! Phase 3 codemods. Every op in here obeys the shared rules from ROADMAP §8: +//! The codemods. Every op in here obeys these shared rules: //! //! * **A. Idempotent** -- check then edit; if the desired state already holds, //! produce zero edits and report `changed: false`. diff --git a/native/igniter_css/src/transform.rs b/native/igniter_css/src/transform.rs index 950d457..1a151c5 100644 --- a/native/igniter_css/src/transform.rs +++ b/native/igniter_css/src/transform.rs @@ -10,7 +10,7 @@ //! //! Keep them out of Igniter installers. They exist for build-time and reporting //! use, they are never used to patch a user's file in place, and the codemods -//! never route their output through here (ROADMAP §3, §6). +//! never route their output through here. use crate::ctx::{ParseCtx, ParseOptions}; use crate::error::Result; diff --git a/native/igniter_css/src/trivia.rs b/native/igniter_css/src/trivia.rs index 7565de5..5cb5850 100644 --- a/native/igniter_css/src/trivia.rs +++ b/native/igniter_css/src/trivia.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! Rule D -- comment ownership on delete (ROADMAP §8). +//! Comment ownership on delete. //! //! When a codemod removes a node, which of the comments around it go with it? //! The convention, decided deliberately rather than inferred: diff --git a/native/igniter_css/tests/corpus_invariants.rs b/native/igniter_css/tests/corpus_invariants.rs index 55430e2..f317116 100644 --- a/native/igniter_css/tests/corpus_invariants.rs +++ b/native/igniter_css/tests/corpus_invariants.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! ROADMAP §9: the invariants that must hold for **every op** against **every +//! The invariants that must hold for **every op** against **every //! fixture**, not just for the cases somebody remembered to write a unit test //! for. //! diff --git a/native/igniter_css/tests/phase0_roundtrip.rs b/native/igniter_css/tests/phase0_roundtrip.rs index 8926292..e0cb993 100644 --- a/native/igniter_css/tests/phase0_roundtrip.rs +++ b/native/igniter_css/tests/phase0_roundtrip.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! Phase 0 GATE (ROADMAP §8). +//! The lossless round-trip gate. //! //! `parse.syntax().to_string() == source` must hold byte-for-byte across the //! entire fixture corpus. Nothing else in this crate was allowed to exist until diff --git a/native/igniter_css/tests/property.rs b/native/igniter_css/tests/property.rs index 7111065..186e8b8 100644 --- a/native/igniter_css/tests/property.rs +++ b/native/igniter_css/tests/property.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! ROADMAP §9.6: property tests over generated CSS. +//! Property tests over generated CSS. //! //! Unit tests check the cases we thought of. These check the invariants against //! input nobody wrote by hand -- including input that is not valid CSS at all, From 2ff21d9626ff4eb97516dd6d2105a709d14048e3 Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 07:57:52 +0200 Subject: [PATCH 2/7] docs: keep only what a reader of this repository can act on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep of every comment and doc-comment in the Elixir and Rust sources for references a reader cannot follow, and for history that belongs in the changelog rather than in the code. Removed the planning-document vocabulary. Sixteen comments cited "ROADMAP §..." for a file that was never committed; others carried its rule letters (rule A through rule E), its risk IDs (R1, R2, R4) and its "hard constraint #N" numbering. None of those resolve to anything in this repository. Each is now either stated plainly or replaced by a link to the module that actually holds the behaviour -- "see [`crate::trivia`]" instead of "rule D". Removed the migration commentary. The Parsers.Parser moduledoc explained what the Python/tinycss2 implementation did and how this differs; it now just describes what the module does. Comments in locate.rs and nif.rs justified decisions by reference to the old implementation, and one in locate.rs described a text scanner this branch had already deleted. The changelog is the right place for all of it, and it still says so there. Renamed tests/phase0_roundtrip.rs to tests/roundtrip.rs. The gate is not a phase, and its doc now says why it matters -- every byte-range edit depends on the parse reproducing its input -- rather than where it sat in a plan. No behaviour change. 339 Rust tests, 268 Elixir + 39 doctests, clippy clean. Co-Authored-By: Claude Opus 5 (1M context) --- lib/igniter_css/native.ex | 4 ++-- lib/igniter_css/parsers/parser.ex | 16 ++++++--------- native/igniter_css/src/ctx.rs | 10 +++++----- native/igniter_css/src/edit.rs | 2 +- native/igniter_css/src/lib.rs | 2 +- native/igniter_css/src/locate.rs | 16 +++++++-------- native/igniter_css/src/nif.rs | 6 +++--- native/igniter_css/src/ops/at_rule.rs | 2 +- native/igniter_css/src/ops/declaration.rs | 6 +++--- native/igniter_css/src/ops/mod.rs | 20 +++++++++---------- native/igniter_css/src/ops/rule.rs | 4 ++-- native/igniter_css/src/ops/tidy.rs | 2 +- native/igniter_css/src/trivia.rs | 3 +-- native/igniter_css/tests/corpus_invariants.rs | 17 ++++++++-------- native/igniter_css/tests/property.rs | 2 +- .../{phase0_roundtrip.rs => roundtrip.rs} | 9 +++++---- 16 files changed, 58 insertions(+), 63 deletions(-) rename native/igniter_css/tests/{phase0_roundtrip.rs => roundtrip.rs} (89%) diff --git a/lib/igniter_css/native.ex b/lib/igniter_css/native.ex index 3503f11..f8db6f4 100644 --- a/lib/igniter_css/native.ex +++ b/lib/igniter_css/native.ex @@ -4,8 +4,8 @@ defmodule IgniterCss.Native do @moduledoc false - # Precompiled NIFs, so end users never need a Rust toolchain - # (hard constraint #5). Set IGNITERCSS_BUILD=1 to force a local build. + # Precompiled NIFs, so end users never need a Rust toolchain. + # Set IGNITERCSS_BUILD=1 to force a local build. mix_config = Mix.Project.config() version = mix_config[:version] diff --git a/lib/igniter_css/parsers/parser.ex b/lib/igniter_css/parsers/parser.ex index a139043..7fae507 100644 --- a/lib/igniter_css/parsers/parser.ex +++ b/lib/igniter_css/parsers/parser.ex @@ -10,18 +10,14 @@ defmodule IgniterCss.Parsers.Parser do Every function accepts either CSS content or a file path, selected by the trailing `type` argument (`:content`, the default, or `:path`). - This module covers the same ground the previous Python/tinycss2 implementation - did, reimplemented on the Rust parser. Two differences are worth knowing: - - * The mutating functions here are now **diff-minimal** — they patch byte - ranges instead of reprinting the stylesheet, so comments and formatting - outside the edit are preserved exactly. - * `minify/2` and `beautify/2` still rewrite the whole file, because that is - what they are for. Do not use them to patch a file a user maintains. + The mutating functions are **diff-minimal**: they patch byte ranges rather + than reprinting the stylesheet, so comments and formatting outside the edit + are preserved exactly. `minify/2` and `beautify/2` are the exception — they + rewrite the whole file, because that is what they are for. Do not point them + at a file a user maintains. For new code prefer `IgniterCss`, which has a plainer `{:ok, result}` shape - and clearer option handling. This module exists so existing call sites keep - working. + and clearer option handling. """ import IgniterCss.Helpers, only: [call_nif_fn: 4] diff --git a/native/igniter_css/src/ctx.rs b/native/igniter_css/src/ctx.rs index 0470855..5e3e164 100644 --- a/native/igniter_css/src/ctx.rs +++ b/native/igniter_css/src/ctx.rs @@ -7,7 +7,7 @@ //! text the user already wrote. //! //! This module and `locate` are the only two places allowed to name Biome -//! types. Isolating them here is the mitigation for R2 (Biome API churn). +//! types, so an upgrade touches two files rather than twenty. use biome_css_parser::{parse_css, CssParse, CssParserOptions}; use biome_css_syntax::{CssSyntaxKind, CssSyntaxNode}; @@ -130,8 +130,8 @@ impl ParseCtx { /// The parse is lossless by construction, but assert it before we let any /// codemod compute offsets against it. If this ever fails, the byte-range - /// design's foundation is gone and we must refuse to patch (hard constraint - /// #4: never destroy input). + /// design's foundation is gone and we must refuse to patch rather than + /// risk destroying the input. pub fn round_trips(&self) -> bool { self.parse.syntax().to_string() == self.source } @@ -174,7 +174,7 @@ impl ParseCtx { /// Checked against the token stream, so braces inside strings and comments /// do not count. An unbalanced file cannot be patched safely: text inserted /// "at the top level" would land inside somebody's unterminated block, and - /// hard constraint #4 says a wrong patch is far worse than no patch. + /// a wrong patch is far worse than no patch. pub fn braces_are_balanced(&self) -> bool { let mut depth = 0i32; for token in self @@ -220,7 +220,7 @@ impl ParseCtx { } /// Leading whitespace of the line containing `offset` -- the indentation to - /// copy when inserting a sibling next to it (rule B). + /// copy when inserting a sibling next to it. pub fn indent_at(&self, offset: usize) -> &str { let start = self.line_start(offset); let line = &self.source[start..]; diff --git a/native/igniter_css/src/edit.rs b/native/igniter_css/src/edit.rs index 9862143..a86b34f 100644 --- a/native/igniter_css/src/edit.rs +++ b/native/igniter_css/src/edit.rs @@ -56,7 +56,7 @@ impl Edit { /// /// * Overlapping ranges are a hard error -- we never silently merge them. /// * Edits are applied back-to-front so earlier offsets stay valid. -/// * `apply_edits(src, vec![])` returns `src` byte for byte (Phase 1 acceptance). +/// * `apply_edits(src, vec![])` returns `src` byte for byte. pub fn apply_edits(source: &str, mut edits: Vec) -> Result { if edits.is_empty() { return Ok(source.to_string()); diff --git a/native/igniter_css/src/lib.rs b/native/igniter_css/src/lib.rs index 6b639ad..e5cffe3 100644 --- a/native/igniter_css/src/lib.rs +++ b/native/igniter_css/src/lib.rs @@ -11,7 +11,7 @@ //! //! * [`ctx`] -- the source, its parse, and the file's own formatting habits //! * [`locate`] -- typed CST queries that return byte ranges -//! * [`trivia`] -- which comments a deleted node owns (rule D) +//! * [`trivia`] -- which comments a deleted node owns //! * [`edit`] -- overlap-checked text splicing //! * [`ops`] -- the codemods, all diff-minimal and idempotent //! * [`analyze`] -- read-only queries diff --git a/native/igniter_css/src/locate.rs b/native/igniter_css/src/locate.rs index 4b5b1c6..9f365eb 100644 --- a/native/igniter_css/src/locate.rs +++ b/native/igniter_css/src/locate.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: MIT -//! Phase 2: typed queries over the CST that return **byte ranges**, never owned +//! Typed queries over the CST that return **byte ranges**, never owned //! strings to be reprinted. //! //! Matching rules are deliberately strict: @@ -10,8 +10,7 @@ //! * selectors compared on a normalised form, never raw equality and never //! substring/fuzzy; //! * more than one match is `MatchResult::Ambiguous` -- we error, we do not -//! pick one. Guessing here is how the Python version produced surprising -//! diffs (R4). +//! pick one, because guessing produces surprising diffs. use crate::ctx::{is_bogus, ParseCtx}; use biome_css_syntax::{CssSyntaxKind, CssSyntaxNode}; @@ -88,7 +87,7 @@ pub struct DeclRef { /// End of the declaration's own bytes, semicolon included when present. pub end: usize, /// Range of the value alone -- the only bytes `set_declaration` replaces - /// when the property already exists (rule E). + /// when the property already exists. pub value_start: usize, pub value_end: usize, /// Range of the `!important` flag, when present. @@ -139,7 +138,7 @@ fn trimmed(node: &CssSyntaxNode) -> (usize, usize) { } /// A child node that opens with `{`. Kind-agnostic on purpose: CSS has a dozen -/// block kinds and new ones appear between Biome releases (R2). +/// block kinds and new ones appear between Biome releases. fn block_child(node: &CssSyntaxNode) -> Option { node.children().find(|c| { c.first_token() @@ -451,7 +450,7 @@ fn decl_ref_from(node: &CssSyntaxNode, ctx: &ParseCtx) -> Option { // The value is everything after the `:` that is not the `!important` flag, // so replacing this range alone preserves both the flag and any inline - // comment sitting on the declaration (rule E). + // comment sitting on the declaration. let value_node = property_node.children().nth(1); let (value_start, value_end) = match value_node { Some(v) => range_to_pair(v.text_trimmed_range()), @@ -797,9 +796,8 @@ mod tests { #[test] fn a_plus_inside_a_functional_pseudo_is_not_a_combinator() { // Both spellings denote the same selector, and reading the CST rather - // than the text makes them agree -- the old text scanner kept them - // distinct, which meant `:nth-child(2n + 1)` could not be matched by - // `:nth-child(2n+1)`. + // than the text makes them agree, so either spelling matches the + // other. assert_eq!( normalize_selector("li:nth-child(2n + 1)"), "li:nth-child(2n+1)" diff --git a/native/igniter_css/src/nif.rs b/native/igniter_css/src/nif.rs index 03fd341..c2b9878 100644 --- a/native/igniter_css/src/nif.rs +++ b/native/igniter_css/src/nif.rs @@ -2,11 +2,11 @@ // // SPDX-License-Identifier: MIT -//! Phase 4: the NIF boundary. Deliberately thin. +//! The NIF boundary. Deliberately thin. //! //! **Elixir sends intent; Rust returns text.** The CST is never exported across -//! the boundary in any form -- marshalling trees between languages is what made -//! the previous implementation painful, and it buys nothing here. +//! the boundary in any form: marshalling trees between languages is costly to +//! maintain and buys nothing here. //! //! Two other rules hold throughout this module: //! diff --git a/native/igniter_css/src/ops/at_rule.rs b/native/igniter_css/src/ops/at_rule.rs index 9b5766e..50b344f 100644 --- a/native/igniter_css/src/ops/at_rule.rs +++ b/native/igniter_css/src/ops/at_rule.rs @@ -6,7 +6,7 @@ //! `@custom-variant` and friends. //! //! For these, the grammar barely matters: what we need is an *anchor offset*, -//! and even a bogus node provides one (R1, mitigation 3). +//! and even a node Biome could not parse still provides one. use crate::ctx::{ParseCtx, ParseOptions}; use crate::edit::Edit; diff --git a/native/igniter_css/src/ops/declaration.rs b/native/igniter_css/src/ops/declaration.rs index 33b98b3..4cae5ed 100644 --- a/native/igniter_css/src/ops/declaration.rs +++ b/native/igniter_css/src/ops/declaration.rs @@ -90,7 +90,7 @@ fn check_property_and_value(property: &str, value: &str) -> Result<(String, Stri /// Update `property` inside the rule matching `selector`, or append it. /// /// When the property is already present only its **value** bytes are replaced -/// (rule E), so an inline comment on that line and any `!important` the caller +/// so an inline comment on that line and any `!important` the caller /// did not ask to change both survive untouched. pub fn set_declaration( source: &str, @@ -153,7 +153,7 @@ pub fn set_declaration( } /// Remove every declaration of `property` from the rule matching `selector`, -/// together with the comments those declarations own (rule D). +/// together with the comments those declarations own. See [`crate::trivia`]. pub fn remove_declaration( source: &str, selector: &str, @@ -187,7 +187,7 @@ pub fn remove_declaration( /// Each prefixed declaration is inserted immediately **before** the unprefixed /// one, which is the ordering browsers expect: the standard property wins. /// Prefixes already present in the same block are skipped, so re-running the op -/// is a no-op (rule A). +/// is a no-op. pub fn add_vendor_prefixes( source: &str, property: &str, diff --git a/native/igniter_css/src/ops/mod.rs b/native/igniter_css/src/ops/mod.rs index 3f5bde7..fab7b75 100644 --- a/native/igniter_css/src/ops/mod.rs +++ b/native/igniter_css/src/ops/mod.rs @@ -4,13 +4,13 @@ //! The codemods. Every op in here obeys these shared rules: //! -//! * **A. Idempotent** -- check then edit; if the desired state already holds, +//! * **Idempotent** -- check then edit; if the desired state already holds, //! produce zero edits and report `changed: false`. -//! * **B. Indentation** -- inserted lines copy the indentation of the sibling -//! they land next to; empty bodies use the file's inferred indent unit. -//! * **C. Newlines** -- always `ctx.nl()`, never a hardcoded `\n`. -//! * **D. Comment ownership on delete** -- see [`crate::trivia`]. -//! * **E. Value-only replacement** -- changing a value edits the value range +//! * **Indentation** -- inserted lines copy the indentation of the sibling they +//! land next to; empty bodies use the file's inferred indent unit. +//! * **Newlines** -- always `ctx.nl()`, never a hardcoded `\n`. +//! * **Comment ownership on delete** -- see [`crate::trivia`]. +//! * **Value-only replacement** -- changing a value edits the value range //! alone, so inline comments and `!important` survive. pub mod at_rule; @@ -47,10 +47,10 @@ impl Outcome { /// Run a codemod end to end. /// -/// The round-trip assertion here is the enforcement point for hard constraint -/// #4: if the parse does not reproduce the source byte for byte we cannot trust -/// any offset it gives us, so we refuse to patch and leave the file untouched -/// rather than risk a wrong edit. +/// The round-trip assertion here is what keeps every edit honest: if the parse +/// does not reproduce the source byte for byte we cannot trust any offset it +/// gives us, so we refuse to patch and leave the file untouched rather than +/// risk a wrong edit. pub fn run(source: &str, options: ParseOptions, build: F) -> Result where F: FnOnce(&ParseCtx) -> Result>, diff --git a/native/igniter_css/src/ops/rule.rs b/native/igniter_css/src/ops/rule.rs index 9ac75c8..db493d7 100644 --- a/native/igniter_css/src/ops/rule.rs +++ b/native/igniter_css/src/ops/rule.rs @@ -199,7 +199,7 @@ pub fn ensure_rule_with( }) } -/// Remove a top-level rule and the comments it owns (rule D). +/// Remove a top-level rule and the comments it owns. See [`crate::trivia`]. pub fn remove_rule(source: &str, selector: &str, options: ParseOptions) -> Result { let want = normalize_selector(selector); if want.is_empty() { @@ -285,7 +285,7 @@ pub fn append_raw_to_rule( "no top-level rule with selector {selector:?}" ))); }; - // Already present verbatim? Then this is a no-op (rule A). + // Already present verbatim? Then this is a no-op. let body = &ctx.source()[rule.body_open..rule.body_close]; let needle = raw.trim(); if body.contains(needle) { diff --git a/native/igniter_css/src/ops/tidy.rs b/native/igniter_css/src/ops/tidy.rs index bbd897f..c5c180d 100644 --- a/native/igniter_css/src/ops/tidy.rs +++ b/native/igniter_css/src/ops/tidy.rs @@ -5,7 +5,7 @@ //! Whole-file tidying ops that are nevertheless **diff-minimal**. //! //! Sorting and de-duplication are usually implemented by reprinting the tree, -//! which violates hard constraint #2. Here they are implemented as permutations +//! which would reformat the whole file. Here they are implemented as permutations //! and deletions of existing byte ranges instead: lines move or disappear, and //! every other byte in the file is untouched. A block we cannot rearrange //! safely is skipped and reported in `diagnostics` rather than reformatted. diff --git a/native/igniter_css/src/trivia.rs b/native/igniter_css/src/trivia.rs index 5cb5850..56656c9 100644 --- a/native/igniter_css/src/trivia.rs +++ b/native/igniter_css/src/trivia.rs @@ -4,8 +4,7 @@ //! Comment ownership on delete. //! -//! When a codemod removes a node, which of the comments around it go with it? -//! The convention, decided deliberately rather than inferred: +//! When a codemod removes a node, which of the comments around it go with it: //! //! | Comment position | Fate | //! |----------------------------------------------------|-----------| diff --git a/native/igniter_css/tests/corpus_invariants.rs b/native/igniter_css/tests/corpus_invariants.rs index f317116..73ed35f 100644 --- a/native/igniter_css/tests/corpus_invariants.rs +++ b/native/igniter_css/tests/corpus_invariants.rs @@ -7,12 +7,12 @@ //! for. //! //! 1. idempotency -- applying twice equals applying once, and the second run -//! reports `changed: false` (§9.3) -//! 2. comment preservation -- no comment is ever lost (§2 constraint 1) -//! 3. diff minimality -- the changed-line count stays within budget (§9.4) +//! reports `changed: false` +//! 2. comment preservation -- no comment is ever lost +//! 3. diff minimality -- the changed-line count stays within budget //! 4. output validity -- the result still round-trips and gains no new parse //! errors -//! 5. malformed input -- an error is returned and the source is untouched (§9.5) +//! 5. malformed input -- an error is returned and the source is untouched mod support; @@ -87,7 +87,8 @@ fn ops() -> Vec { // `.page { display: flex; /* trailing on a declaration */ }` in the // comments fixture: this is the only op that takes the *update* branch // on a declaration that already carries a trailing comment, so without - // it the sweep never exercises rule E against a real comment. + // it the sweep never exercises value-only replacement against a + // real comment. ("set_declaration_over_a_commented_line", |s| { set_declaration( s, @@ -168,7 +169,7 @@ fn no_op_ever_loses_a_comment() { continue; } for (op_name, op) in ops() { - // Removal ops delete comments on purpose (rule D); they are covered + // Removal ops delete comments on purpose; they are covered // by their own targeted tests. if op_name.starts_with("remove_") { continue; @@ -256,7 +257,7 @@ fn an_unchanged_outcome_returns_the_source_byte_for_byte() { } } -/// §9.4: a codemod touching one thing must not reformat the file around it. +/// A codemod touching one thing must not reformat the file around it. #[test] fn single_target_ops_change_only_a_handful_of_lines() { // (op, budget in changed lines). Generous, but far below "whole file". @@ -310,7 +311,7 @@ fn single_target_ops_change_only_a_handful_of_lines() { } } -/// §9.5: input we cannot understand well enough to patch must come back +/// Input we cannot understand well enough to patch must come back /// untouched, with an error, never half-edited. #[test] fn malformed_input_is_never_half_edited() { diff --git a/native/igniter_css/tests/property.rs b/native/igniter_css/tests/property.rs index 186e8b8..a9eb405 100644 --- a/native/igniter_css/tests/property.rs +++ b/native/igniter_css/tests/property.rs @@ -137,7 +137,7 @@ fn junk() -> impl Strategy { // --------------------------------------------------------------------------- proptest! { - /// The Phase 0 gate, generalised: the parse must reproduce any input. + /// The round-trip gate, generalised: the parse must reproduce any input. #[test] fn round_trip_holds_for_generated_stylesheets(src in stylesheet()) { let ctx = ParseCtx::parse_default(&src); diff --git a/native/igniter_css/tests/phase0_roundtrip.rs b/native/igniter_css/tests/roundtrip.rs similarity index 89% rename from native/igniter_css/tests/phase0_roundtrip.rs rename to native/igniter_css/tests/roundtrip.rs index e0cb993..1bb5fc2 100644 --- a/native/igniter_css/tests/phase0_roundtrip.rs +++ b/native/igniter_css/tests/roundtrip.rs @@ -5,8 +5,9 @@ //! The lossless round-trip gate. //! //! `parse.syntax().to_string() == source` must hold byte-for-byte across the -//! entire fixture corpus. Nothing else in this crate was allowed to exist until -//! this passed, and it stays in CI forever afterwards (§9.1). +//! entire fixture corpus. Every byte-range edit in this crate depends on it: if +//! the parse cannot reproduce its input, the offsets it reports cannot be +//! trusted and no codemod may run. mod support; @@ -80,7 +81,7 @@ fn round_trip_holds_even_when_the_parse_has_errors() { ); } -/// R1: Tailwind v4 at-rules must not merely survive -- they must parse without +/// Tailwind v4 at-rules must not merely survive -- they must parse without /// diagnostics, so the location layer can find them as real nodes. #[test] fn tailwind_v4_at_rules_parse_without_diagnostics() { @@ -89,7 +90,7 @@ fn tailwind_v4_at_rules_parse_without_diagnostics() { assert_eq!( ctx.diagnostics_count(), 0, - "Tailwind v4 fixture produced parse diagnostics; re-evaluate R1" + "Tailwind v4 fixture produced parse diagnostics" ); assert!(ctx.round_trips()); } From a6844acc54d73233eea2ea3ae98c33334b002fcb Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 08:08:36 +0200 Subject: [PATCH 3/7] build(deps): raise the ex_doc and igniter requirements to the versions in use Both declared floors sat well behind what the lock actually resolved: ex_doc at `~> 0.38` while 0.40.3 was installed, and igniter at `~> 0.5` while 0.8.3 was. `mix hex.outdated` does not catch this. It compares the resolved version against the newest published one, and both resolved fine, so it reported everything up to date -- the stale part was the requirement, not the dependency. No resolution changes; this only stops the declared floors drifting further from reality. Docs still build and the suite is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- mix.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.exs b/mix.exs index 5bdaec1..fb7b5e2 100644 --- a/mix.exs +++ b/mix.exs @@ -114,13 +114,13 @@ defmodule IgniterCss.MixProject do [ {:rustler, "~> 0.38.0", optional: true}, {:rustler_precompiled, "~> 0.9"}, - {:igniter, "~> 0.5", optional: true}, + {:igniter, "~> 0.8.3", optional: true}, {:mix_audit, ">= 0.0.0", only: [:dev, :test], runtime: false}, {:sobelow, ">= 0.0.0", only: [:dev, :test], runtime: false}, {:dialyxir, ">= 0.0.0", only: [:dev, :test], runtime: false}, {:ex_check, "~> 0.16", only: [:dev, :test]}, {:credo, ">= 0.0.0", only: [:dev, :test], runtime: false}, - {:ex_doc, "~> 0.38", only: [:dev, :test], runtime: false} + {:ex_doc, "~> 0.40.3", only: [:dev, :test], runtime: false} ] end From 21154f02bf12782961aadcefaa4a05716431c904 Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 08:35:11 +0200 Subject: [PATCH 4/7] fix: refuse deeply nested input instead of overflowing the parser's stack Audit for anything in the Rust that could take the BEAM down. Rustler wraps every NIF body in `catch_unwind` and turns a panic into an Erlang exception in the calling process, so panics are survivable, and there are none left outside tests anyway: no unwrap, expect, panic!, todo!, unreachable! or indexing that is not bounds-checked first. A stack overflow is different. It aborts the process, `catch_unwind` cannot intercept it, and in a NIF that ends the VM rather than the call. Biome's CSS parser is recursive descent and rowan's tree drop recurses too, so deeply nested input overflows. Measured on a 2 MB stack, `@media print{` nesting and `:not(` nesting both survive 1000 levels and abort at 2000 -- and a dirty scheduler thread may have less headroom than that. Every entry point that reaches a parse now measures nesting first, with a plain byte scan that skips strings and comments and cannot itself recurse, and refuses past 256 levels. That is an order of magnitude below the observed failure and far above real CSS, which rarely exceeds ten. The guard also applies to caller-supplied text, which turned up a second problem: a selector nested past the limit did not crash, but `ensure_rule` would splice it in and produce a file we would then refuse to read. Writing what we will not read is its own bug, so `validate_snippet` enforces the same limit, and `set_declaration` now validates the selector it may create a rule from. tests/stack_safety.rs covers both nesting dimensions across every mutating, read-only and transform op, and asserts ordinary CSS is nowhere near the limit. Verified end to end through the real NIF: all ten paths refuse, the VM survives, and ordinary CSS is unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- native/igniter_css/src/analyze.rs | 11 ++ native/igniter_css/src/ctx.rs | 61 ++++++++++ native/igniter_css/src/locate.rs | 3 + native/igniter_css/src/ops/at_rule.rs | 1 + native/igniter_css/src/ops/declaration.rs | 1 + native/igniter_css/src/ops/mod.rs | 7 ++ native/igniter_css/src/transform.rs | 2 + native/igniter_css/tests/stack_safety.rs | 131 ++++++++++++++++++++++ 8 files changed, 217 insertions(+) create mode 100644 native/igniter_css/tests/stack_safety.rs diff --git a/native/igniter_css/src/analyze.rs b/native/igniter_css/src/analyze.rs index 6bd7a83..8c7c9ad 100644 --- a/native/igniter_css/src/analyze.rs +++ b/native/igniter_css/src/analyze.rs @@ -308,6 +308,9 @@ pub fn value_has_color(value: &str) -> bool { if value.trim().is_empty() { return false; } + if crate::ctx::check_nesting(value).is_err() { + return false; + } let probe = format!("a{{b:{value}}}"); let parse = biome_css_parser::parse_css(&probe, biome_css_parser::CssParserOptions::default()); parse @@ -568,6 +571,14 @@ pub struct Validation { /// raised no errors. The round-trip half is the one that actually matters for /// safety -- it is what every codemod checks before touching a file. pub fn validate(source: &str, options: ParseOptions) -> Validation { + if let Err(e) = crate::ctx::check_nesting(source) { + return Validation { + valid: false, + diagnostics: 0, + round_trips: false, + message: e.to_string(), + }; + } let ctx = ParseCtx::new(source, options); let round_trips = ctx.round_trips(); let diagnostics = ctx.diagnostics_count(); diff --git a/native/igniter_css/src/ctx.rs b/native/igniter_css/src/ctx.rs index 5e3e164..b467293 100644 --- a/native/igniter_css/src/ctx.rs +++ b/native/igniter_css/src/ctx.rs @@ -15,6 +15,67 @@ use biome_rowan::TextRange; pub const BOM: &str = "\u{feff}"; +/// Maximum nesting we are willing to hand to the parser. +/// +/// Biome's CSS parser is recursive descent and rowan's tree drop recurses too, +/// so deeply nested input overflows the stack. That is an abort, not a panic: +/// `catch_unwind` cannot intercept it, and inside a NIF it would take the whole +/// VM down rather than raise in the calling process. +/// +/// Measured on a 2 MB test-thread stack, both `{` nesting and `:not(` nesting +/// survive 1000 levels and abort at 2000. A BEAM dirty scheduler thread may +/// have less, so this sits an order of magnitude below the observed failure -- +/// and still far above real CSS, which rarely exceeds ten. +pub const MAX_NESTING_DEPTH: usize = 256; + +/// Deepest `{` or `(` nesting in `source`, ignoring strings and comments. +/// +/// A plain byte scan: it must be cheap and, above all, must not itself recurse. +pub fn nesting_depth(source: &str) -> usize { + let bytes = source.as_bytes(); + let (mut depth, mut max) = (0usize, 0usize); + let mut i = 0usize; + + while i < bytes.len() { + match bytes[i] { + b'"' | b'\'' => { + let quote = bytes[i]; + i += 1; + while i < bytes.len() && bytes[i] != quote { + i += if bytes[i] == b'\\' { 2 } else { 1 }; + } + } + b'/' if bytes.get(i + 1) == Some(&b'*') => { + i += 2; + while i + 1 < bytes.len() && !(bytes[i] == b'*' && bytes[i + 1] == b'/') { + i += 1; + } + i += 1; + } + b'{' | b'(' => { + depth += 1; + max = max.max(depth); + } + b'}' | b')' => depth = depth.saturating_sub(1), + _ => {} + } + i += 1; + } + max +} + +/// Refuse input nested deeply enough to risk a stack overflow in the parser. +pub fn check_nesting(source: &str) -> crate::error::Result<()> { + let depth = nesting_depth(source); + if depth > MAX_NESTING_DEPTH { + return Err(crate::error::CssError::Unparseable(format!( + "nested {depth} levels deep, limit is {MAX_NESTING_DEPTH}; \ + refusing to parse input that could overflow the stack" + ))); + } + Ok(()) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Newline { Lf, diff --git a/native/igniter_css/src/locate.rs b/native/igniter_css/src/locate.rs index 9f365eb..c0ae6be 100644 --- a/native/igniter_css/src/locate.rs +++ b/native/igniter_css/src/locate.rs @@ -271,6 +271,9 @@ pub fn normalize_selector(input: &str) -> String { if trimmed.is_empty() { return String::new(); } + if crate::ctx::check_nesting(trimmed).is_err() { + return trimmed.split_whitespace().collect::>().join(" "); + } let probe = format!("{trimmed} {{}}"); let parse = biome_css_parser::parse_css(&probe, biome_css_parser::CssParserOptions::default()); let selector_list = parse diff --git a/native/igniter_css/src/ops/at_rule.rs b/native/igniter_css/src/ops/at_rule.rs index 50b344f..d9ddaaf 100644 --- a/native/igniter_css/src/ops/at_rule.rs +++ b/native/igniter_css/src/ops/at_rule.rs @@ -60,6 +60,7 @@ pub fn parse_at_rule_spec(line: &str) -> Result { ))); } validate_snippet(trimmed, "at-rule line")?; + crate::ctx::check_nesting(trimmed)?; let has_block = trimmed.contains('{'); let text = if has_block || trimmed.ends_with(';') { diff --git a/native/igniter_css/src/ops/declaration.rs b/native/igniter_css/src/ops/declaration.rs index 4cae5ed..a7911b3 100644 --- a/native/igniter_css/src/ops/declaration.rs +++ b/native/igniter_css/src/ops/declaration.rs @@ -101,6 +101,7 @@ pub fn set_declaration( options: ParseOptions, ) -> Result { let (property, value) = check_property_and_value(property, value)?; + validate_snippet(selector, "selector")?; run(source, options, |ctx| { let Some(rule) = resolve_rule(ctx, selector)? else { diff --git a/native/igniter_css/src/ops/mod.rs b/native/igniter_css/src/ops/mod.rs index fab7b75..56fff2f 100644 --- a/native/igniter_css/src/ops/mod.rs +++ b/native/igniter_css/src/ops/mod.rs @@ -55,6 +55,7 @@ pub fn run(source: &str, options: ParseOptions, build: F) -> Result where F: FnOnce(&ParseCtx) -> Result>, { + crate::ctx::check_nesting(source)?; let ctx = ParseCtx::new(source, options); if !ctx.round_trips() { return Err(CssError::Unparseable( @@ -89,6 +90,7 @@ pub fn query(source: &str, options: ParseOptions, f: F) -> Result where F: FnOnce(&ParseCtx) -> Result, { + crate::ctx::check_nesting(source)?; let ctx = ParseCtx::new(source, options); if !ctx.round_trips() { return Err(CssError::Unparseable( @@ -179,6 +181,11 @@ pub fn ends_with_semicolon(text: &str) -> bool { /// Cheap structural guard, not a full validation: we re-parse the result /// anyway, but catching an unbalanced brace here gives a far better error. pub fn validate_snippet(text: &str, what: &str) -> Result<()> { + // Text spliced into a file has to satisfy the reader's nesting limit too, + // or we would write something we then refuse to parse. + crate::ctx::check_nesting(text) + .map_err(|_| CssError::InvalidInput(format!("{what} is nested too deeply")))?; + let mut depth = 0i32; let mut chars = text.chars().peekable(); while let Some(c) = chars.next() { diff --git a/native/igniter_css/src/transform.rs b/native/igniter_css/src/transform.rs index 1a151c5..48a97e2 100644 --- a/native/igniter_css/src/transform.rs +++ b/native/igniter_css/src/transform.rs @@ -37,6 +37,7 @@ fn is_word_char(c: char) -> bool { /// would change how the result tokenises -- so `and (min-width: 1px)` keeps its /// space while `url(x)` never gains one. pub fn minify(source: &str, options: ParseOptions) -> Result { + crate::ctx::check_nesting(source)?; let ctx = ParseCtx::new(source, options); let mut out = String::with_capacity(source.len()); let mut prev_end: Option = None; @@ -90,6 +91,7 @@ pub fn minify(source: &str, options: ParseOptions) -> Result { /// /// A conventional pretty-printer: whole-file output, so never use it to patch. pub fn beautify(source: &str, options: ParseOptions) -> Result { + crate::ctx::check_nesting(source)?; let ctx = ParseCtx::new(source, options); let nl = ctx.nl(); let unit = ctx.indent(); diff --git a/native/igniter_css/tests/stack_safety.rs b/native/igniter_css/tests/stack_safety.rs new file mode 100644 index 0000000..a8a2ae4 --- /dev/null +++ b/native/igniter_css/tests/stack_safety.rs @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: 2025 igniter_css contributors +// +// SPDX-License-Identifier: MIT + +//! Deeply nested input must be refused, not parsed. +//! +//! Biome's CSS parser is recursive descent, so nesting past a few thousand +//! levels overflows the stack. That is an abort, not a panic: `catch_unwind` +//! cannot intercept it, so inside a NIF it would take the VM down instead of +//! raising in the calling process. Every entry point that reaches a parse has +//! to reject such input first. +//! +//! These cases are all well below the depth that actually aborts -- the point +//! is that the guard fires, not that we survive an overflow. + +use igniter_css::analyze; +use igniter_css::ctx::{check_nesting, nesting_depth, ParseOptions, MAX_NESTING_DEPTH}; +use igniter_css::ops::at_rule::ensure_at_rule_line; +use igniter_css::ops::declaration::{set_declaration, SetOptions}; +use igniter_css::ops::rule::{ensure_rule, has_rule}; +use igniter_css::ops::tidy::{remove_duplicates, sort_properties, DedupeOptions}; +use igniter_css::transform; + +fn opts() -> ParseOptions { + ParseOptions::default() +} + +fn nested_blocks(depth: usize) -> String { + format!( + "{}.a{{color:red}}{}", + "@media print{".repeat(depth), + "}".repeat(depth) + ) +} + +fn nested_selector(depth: usize) -> String { + format!("{}a{}", ":not(".repeat(depth), ")".repeat(depth)) +} + +#[test] +fn depth_is_measured_without_recursing() { + assert_eq!(nesting_depth(""), 0); + assert_eq!(nesting_depth(".a { color: red; }"), 1); + assert_eq!(nesting_depth("@media print { .a { color: red; } }"), 2); + assert_eq!(nesting_depth(".a { background: url(x) }"), 2); + assert_eq!(nesting_depth(&nested_blocks(50)), 51); + // Braces inside strings and comments are not nesting. + assert_eq!(nesting_depth(r#".a { content: "{{{{" }"#), 1); + assert_eq!(nesting_depth(".a { /* {{{{ */ color: red }"), 1); +} + +#[test] +fn ordinary_css_is_nowhere_near_the_limit() { + for source in [ + ".a { color: red; }", + "@media print { @supports (display: grid) { .a { color: red; } } }", + ".a { background: url(data:image/svg+xml;base64,AA==); }", + ] { + assert!( + check_nesting(source).is_ok(), + "rejected ordinary CSS: {source}" + ); + assert!(nesting_depth(source) < 10); + } +} + +#[test] +fn every_mutating_op_refuses_deeply_nested_input() { + let src = nested_blocks(MAX_NESTING_DEPTH + 1); + + assert!(ensure_at_rule_line(&src, "@plugin \"p\";", opts()).is_err()); + assert!(ensure_rule(&src, ".probe", opts()).is_err()); + assert!(sort_properties(&src, opts()).is_err()); + assert!(remove_duplicates(&src, DedupeOptions::default(), opts()).is_err()); + assert!(set_declaration( + &src, + ".a", + "color", + "blue", + SetOptions { + create_rule: true, + ..Default::default() + }, + opts() + ) + .is_err()); +} + +#[test] +fn every_read_only_op_refuses_deeply_nested_input() { + let src = nested_blocks(MAX_NESTING_DEPTH + 1); + + assert!(analyze::analyze(&src, opts()).is_err()); + assert!(analyze::extract_colors(&src, opts()).is_err()); + assert!(analyze::extract_media_queries(&src, opts()).is_err()); + assert!(analyze::extract_animations(&src, opts()).is_err()); + assert!(has_rule(&src, ".a", opts()).is_err()); + + // `validate` reports rather than returning Result, so it must say so. + let v = analyze::validate(&src, opts()); + assert!(!v.valid); + assert!(v.message.contains("nested")); +} + +#[test] +fn transforms_refuse_deeply_nested_input() { + let src = nested_blocks(MAX_NESTING_DEPTH + 1); + assert!(transform::minify(&src, opts()).is_err()); + assert!(transform::beautify(&src, opts()).is_err()); + assert!(transform::merge_stylesheets(&[src], opts()).is_err()); +} + +#[test] +fn a_deeply_nested_selector_argument_is_refused_too() { + // The caller's selector is parsed as well, so it needs the same guard. + let deep = nested_selector(MAX_NESTING_DEPTH + 1); + assert!(has_rule(".a { color: red; }", &deep, opts()).is_ok()); + assert!(ensure_rule(".a {}", &deep, opts()).is_err()); + assert!(ensure_at_rule_line("", &format!("@media {deep} {{ }}"), opts()).is_err()); +} + +#[test] +fn the_error_says_what_happened() { + let src = nested_blocks(MAX_NESTING_DEPTH + 1); + let err = ensure_rule(&src, ".probe", opts()).unwrap_err().to_string(); + assert!(err.contains("nested"), "unhelpful error: {err}"); + assert!( + err.contains(&MAX_NESTING_DEPTH.to_string()), + "no limit given: {err}" + ); +} From 3aa6365e24587b7e25051b8fb737cfce4a978dc9 Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 09:21:24 +0200 Subject: [PATCH 5/7] fix: contain a parser panic, reach nested rules, and stop a quadratic walk Three findings from auditing what can take the VM down, and from testing on Elixir 1.20. Biome's CSS parser can panic. It asserts that it keeps making progress and aborts with "The parser is no longer progressing" on some inputs; the fuzzer found one by splicing comments into generated CSS at arbitrary offsets. Rustler would turn that into an exception in the calling process, which is survivable but breaks the promise that unpatchable input returns {:error, reason}. ParseCtx::try_new now catches it and reports it as one, and every production parse -- ops, transforms, validate, and the probe parses for a caller's selector and value -- goes through it. No unguarded parse_css call remains outside ctx. The property test asserting that spliced input always parses was asserting something Biome cannot guarantee. It now asserts what this library actually promises: either a context that round-trips, or a clean error, never a panic that escapes. Native CSS nesting was invisible to any query that descends. Biome models `&:hover { }` as CSS_NESTED_QUALIFIED_RULE holding a CSS_RELATIVE_SELECTOR_LIST, not CSS_QUALIFIED_RULE, so find_all_rules skipped nested rules entirely and analyze under-reported: a file with three rules counted one. Both shapes are now recognised. Top-level queries are unaffected -- nesting stays scoped, and `.card`'s declarations remain its own. analyze was quadratic in nesting depth. conditions_of rebuilt the list of every at-rule in the file once per ancestor, for a node it already held, and prelude_norm filtered the whole subtree when it only needed the tokens before the block. Reading each ancestor directly and bounding the token scan with take_while made depth 128 eleven times faster, 20.6ms to 1.9ms, and the growth roughly linear. 352 Rust tests, 307 Elixir. Verified on Elixir 1.20.2/OTP 28: no warnings from this project, credo, sobelow, dialyzer and both formatters clean. Co-Authored-By: Claude Opus 5 (1M context) --- native/igniter_css/src/analyze.rs | 26 ++-- native/igniter_css/src/ctx.rs | 19 +++ native/igniter_css/src/locate.rs | 115 ++++++++++++++++-- native/igniter_css/src/ops/at_rule.rs | 7 +- native/igniter_css/src/ops/mod.rs | 8 +- native/igniter_css/src/transform.rs | 4 +- .../tests/property.proptest-regressions | 7 ++ native/igniter_css/tests/property.rs | 22 ++-- 8 files changed, 174 insertions(+), 34 deletions(-) create mode 100644 native/igniter_css/tests/property.proptest-regressions diff --git a/native/igniter_css/src/analyze.rs b/native/igniter_css/src/analyze.rs index 8c7c9ad..13812f6 100644 --- a/native/igniter_css/src/analyze.rs +++ b/native/igniter_css/src/analyze.rs @@ -64,13 +64,14 @@ fn prelude_of(ctx: &ParseCtx, list: &CssSyntaxNode) -> String { /// `@media`/`@supports`/`@container` preludes enclosing this node, outermost /// first. fn conditions_of(ctx: &ParseCtx, node: &CssSyntaxNode) -> Vec { + // Read each ancestor directly. Searching a freshly built list of every + // at-rule in the file, once per ancestor, made this quadratic in nesting + // depth for a node we already hold. let mut out: Vec = node .ancestors() .filter(|a| a.kind() == CssSyntaxKind::CSS_AT_RULE) .filter_map(|a| { - find_all_at_rules(ctx) - .into_iter() - .find(|r| r.node == a) + crate::locate::at_rule_ref(ctx, &a) .filter(|r| matches!(r.name.as_str(), "media" | "supports" | "container")) .map(|r| format!("@{} {}", r.name, r.prelude).trim().to_string()) }) @@ -312,9 +313,10 @@ pub fn value_has_color(value: &str) -> bool { return false; } let probe = format!("a{{b:{value}}}"); - let parse = biome_css_parser::parse_css(&probe, biome_css_parser::CssParserOptions::default()); - parse - .syntax() + let Ok(ctx) = ParseCtx::try_new(&probe, ParseOptions::default()) else { + return false; + }; + ctx.syntax() .descendants() .find(|n| n.kind() == CssSyntaxKind::CSS_GENERIC_COMPONENT_VALUE_LIST) .is_some_and(|list| value_node_has_color(&list)) @@ -579,7 +581,17 @@ pub fn validate(source: &str, options: ParseOptions) -> Validation { message: e.to_string(), }; } - let ctx = ParseCtx::new(source, options); + let ctx = match ParseCtx::try_new(source, options) { + Ok(c) => c, + Err(e) => { + return Validation { + valid: false, + diagnostics: 0, + round_trips: false, + message: e.to_string(), + } + } + }; let round_trips = ctx.round_trips(); let diagnostics = ctx.diagnostics_count(); let has_errors = ctx.has_errors(); diff --git a/native/igniter_css/src/ctx.rs b/native/igniter_css/src/ctx.rs index b467293..0239512 100644 --- a/native/igniter_css/src/ctx.rs +++ b/native/igniter_css/src/ctx.rs @@ -173,6 +173,25 @@ impl ParseCtx { Self::new(source, ParseOptions::default()) } + /// Parse, converting a panic inside the parser into an error. + /// + /// Biome's CSS parser asserts that it keeps making progress and panics if + /// it does not; fuzzing found inputs that trip it ("The parser is no longer + /// progressing"). Rustler would turn that into an exception in the calling + /// process, but this library promises `{:error, reason}` for input it + /// cannot handle, so the panic is caught here and reported as one. + /// + /// `AssertUnwindSafe` is sound because nothing is shared: the closure owns + /// its inputs and any half-built parser state is dropped with them. + pub fn try_new(source: &str, options: ParseOptions) -> crate::error::Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| Self::new(source, options))) + .map_err(|_| { + crate::error::CssError::Unparseable( + "the CSS parser failed on this input and could not report where".to_string(), + ) + }) + } + pub fn source(&self) -> &str { &self.source } diff --git a/native/igniter_css/src/locate.rs b/native/igniter_css/src/locate.rs index c0ae6be..0602e5f 100644 --- a/native/igniter_css/src/locate.rs +++ b/native/igniter_css/src/locate.rs @@ -275,8 +275,10 @@ pub fn normalize_selector(input: &str) -> String { return trimmed.split_whitespace().collect::>().join(" "); } let probe = format!("{trimmed} {{}}"); - let parse = biome_css_parser::parse_css(&probe, biome_css_parser::CssParserOptions::default()); - let selector_list = parse + let Ok(ctx) = ParseCtx::try_new(&probe, crate::ctx::ParseOptions::default()) else { + return trimmed.split_whitespace().collect::>().join(" "); + }; + let selector_list = ctx .syntax() .descendants() .find(|n| n.kind() == CssSyntaxKind::CSS_SELECTOR_LIST); @@ -305,12 +307,21 @@ pub fn normalize_property(input: &str) -> String { // --------------------------------------------------------------------------- fn rule_ref_from(node: &CssSyntaxNode, ctx: &ParseCtx) -> Option { - if node.kind() != CssSyntaxKind::CSS_QUALIFIED_RULE { + // Native CSS nesting gives `&:hover { }` a different shape from a top-level + // rule: `CSS_NESTED_QUALIFIED_RULE` holding a `CSS_RELATIVE_SELECTOR_LIST`. + // Both are rules, and queries that descend must see both. + if !matches!( + node.kind(), + CssSyntaxKind::CSS_QUALIFIED_RULE | CssSyntaxKind::CSS_NESTED_QUALIFIED_RULE + ) { return None; } - let selector_list = node - .children() - .find(|c| c.kind() == CssSyntaxKind::CSS_SELECTOR_LIST)?; + let selector_list = node.children().find(|c| { + matches!( + c.kind(), + CssSyntaxKind::CSS_SELECTOR_LIST | CssSyntaxKind::CSS_RELATIVE_SELECTOR_LIST + ) + })?; let block = block_child(node)?; let (body_open, body_close) = block_bounds(&block)?; let (start, end) = trimmed(node); @@ -357,6 +368,10 @@ fn at_rule_target_token(node: &CssSyntaxNode) -> Option { None } +pub fn at_rule_ref(ctx: &ParseCtx, node: &CssSyntaxNode) -> Option { + at_rule_ref_from(node, ctx) +} + fn at_rule_ref_from(node: &CssSyntaxNode, ctx: &ParseCtx) -> Option { if node.kind() != CssSyntaxKind::CSS_AT_RULE { return None; @@ -404,12 +419,13 @@ fn at_rule_ref_from(node: &CssSyntaxNode, ctx: &ParseCtx) -> Option { // Canonical prelude: the tokens between the name and the `;`/`{`, joined by // single spaces. Trivia (whitespace and comments) is excluded by // construction because we read `text_trimmed` of each token. + // `take_while`, not `filter`: tokens come in document order, so stopping at + // the prelude's end keeps this proportional to the prelude rather than to + // the whole subtree -- which for an outer at-rule is the entire file. let prelude_norm = inner .descendants_tokens(Direction::Next) - .filter(|t| { - let s = usize::from(t.text_trimmed_range().start()); - s >= name_end && s < prelude_end - }) + .take_while(|t| usize::from(t.text_trimmed_range().start()) < prelude_end) + .filter(|t| usize::from(t.text_trimmed_range().start()) >= name_end) .map(|t| t.text_trimmed().to_string()) .collect::>() .join(" "); @@ -1120,3 +1136,82 @@ mod tests { assert_eq!(texts, vec!["/* a */", "/* b */", "/* c */", "/* d */"]); } } + +#[cfg(test)] +mod nesting_tests { + use super::*; + use crate::ctx::ParseCtx; + + fn ctx(src: &str) -> ParseCtx { + ParseCtx::parse_default(src) + } + + const NESTED: &str = ".card {\n color: red;\n &:hover {\n color: blue;\n }\n .title {\n font-size: 2rem;\n }\n}\n"; + + #[test] + fn nested_rules_are_found_when_descending() { + let c = ctx(NESTED); + let all: Vec = find_all_rules(&c) + .into_iter() + .map(|r| r.selector_norm) + .collect(); + assert_eq!(all, vec![".card", "&:hover", ".title"]); + } + + #[test] + fn nested_rules_stay_out_of_the_top_level() { + let c = ctx(NESTED); + let top: Vec = find_top_level_rules(&c) + .into_iter() + .map(|r| r.selector_norm) + .collect(); + assert_eq!(top, vec![".card"]); + assert!(find_rule_by_selector(&c, "&:hover").is_none()); + } + + #[test] + fn a_nested_rule_can_be_addressed_explicitly() { + let c = ctx(NESTED); + assert!(matches!( + find_rule_by_selector_anywhere(&c, "&:hover"), + MatchResult::One(_) + )); + assert!(matches!( + find_rule_by_selector_anywhere(&c, ".title"), + MatchResult::One(_) + )); + } + + #[test] + fn declarations_of_a_nested_rule_are_its_own() { + let c = ctx(NESTED); + let hover = find_rule_by_selector_anywhere(&c, "&:hover").one().unwrap(); + let decls: Vec = declarations_in(&c, &hover) + .into_iter() + .map(|d| d.property) + .collect(); + assert_eq!(decls, vec!["color"]); + } + + #[test] + fn an_outer_rule_lists_only_its_own_declarations() { + let c = ctx(NESTED); + let card = find_rule_by_selector(&c, ".card").one().unwrap(); + let decls: Vec = declarations_in(&c, &card) + .into_iter() + .map(|d| d.property) + .collect(); + assert_eq!(decls, vec!["color"]); + } + + #[test] + fn nesting_inside_an_at_rule_is_reached_too() { + let c = + ctx("@media print {\n .a {\n color: red;\n &:hover { color: blue; }\n }\n}\n"); + let all: Vec = find_all_rules(&c) + .into_iter() + .map(|r| r.selector_norm) + .collect(); + assert_eq!(all, vec![".a", "&:hover"]); + } +} diff --git a/native/igniter_css/src/ops/at_rule.rs b/native/igniter_css/src/ops/at_rule.rs index d9ddaaf..bc94323 100644 --- a/native/igniter_css/src/ops/at_rule.rs +++ b/native/igniter_css/src/ops/at_rule.rs @@ -69,7 +69,7 @@ pub fn parse_at_rule_spec(line: &str) -> Result { format!("{trimmed};") }; - let ctx = ParseCtx::new(&text, ParseOptions::default()); + let ctx = ParseCtx::try_new(&text, ParseOptions::default())?; if !ctx.round_trips() { return Err(CssError::InvalidInput(format!( "cannot understand at-rule line {trimmed:?}" @@ -136,10 +136,7 @@ fn insertion_offset(ctx: &ParseCtx, spec: &AtRuleSpec) -> usize { for node in top_level_nodes(ctx) { match node.kind() { CssSyntaxKind::CSS_AT_RULE => { - let Some(at) = find_top_level_at_rules(ctx) - .into_iter() - .find(|r| r.node == node) - else { + let Some(at) = crate::locate::at_rule_ref(ctx, &node) else { continue; }; if is_prologue_first && !PROLOGUE_FIRST.contains(&at.name.as_str()) { diff --git a/native/igniter_css/src/ops/mod.rs b/native/igniter_css/src/ops/mod.rs index 56fff2f..1d9eac1 100644 --- a/native/igniter_css/src/ops/mod.rs +++ b/native/igniter_css/src/ops/mod.rs @@ -56,7 +56,7 @@ where F: FnOnce(&ParseCtx) -> Result>, { crate::ctx::check_nesting(source)?; - let ctx = ParseCtx::new(source, options); + let ctx = ParseCtx::try_new(source, options)?; if !ctx.round_trips() { return Err(CssError::Unparseable( "the parser did not reproduce the input byte for byte".to_string(), @@ -91,7 +91,7 @@ where F: FnOnce(&ParseCtx) -> Result, { crate::ctx::check_nesting(source)?; - let ctx = ParseCtx::new(source, options); + let ctx = ParseCtx::try_new(source, options)?; if !ctx.round_trips() { return Err(CssError::Unparseable( "the parser did not reproduce the input byte for byte".to_string(), @@ -147,7 +147,9 @@ pub fn split_declarations(text: &str) -> Vec { } let probe = format!("a{{{text}}}"); - let ctx = ParseCtx::parse_default(&probe); + let Ok(ctx) = ParseCtx::try_new(&probe, ParseOptions::default()) else { + return vec![ensure_semicolon(text.trim())]; + }; let Some(rule) = crate::locate::find_top_level_rules(&ctx).into_iter().next() else { return vec![ensure_semicolon(text.trim())]; }; diff --git a/native/igniter_css/src/transform.rs b/native/igniter_css/src/transform.rs index 48a97e2..31a98cf 100644 --- a/native/igniter_css/src/transform.rs +++ b/native/igniter_css/src/transform.rs @@ -38,7 +38,7 @@ fn is_word_char(c: char) -> bool { /// space while `url(x)` never gains one. pub fn minify(source: &str, options: ParseOptions) -> Result { crate::ctx::check_nesting(source)?; - let ctx = ParseCtx::new(source, options); + let ctx = ParseCtx::try_new(source, options)?; let mut out = String::with_capacity(source.len()); let mut prev_end: Option = None; @@ -92,7 +92,7 @@ pub fn minify(source: &str, options: ParseOptions) -> Result { /// A conventional pretty-printer: whole-file output, so never use it to patch. pub fn beautify(source: &str, options: ParseOptions) -> Result { crate::ctx::check_nesting(source)?; - let ctx = ParseCtx::new(source, options); + let ctx = ParseCtx::try_new(source, options)?; let nl = ctx.nl(); let unit = ctx.indent(); let comments = all_comments(&ctx); diff --git a/native/igniter_css/tests/property.proptest-regressions b/native/igniter_css/tests/property.proptest-regressions new file mode 100644 index 0000000..6191099 --- /dev/null +++ b/native/igniter_css/tests/property.proptest-regressions @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc f4f3ce1ddfdc7da01e0bb77898a2d4032b584bdb4a236cf184d869ada620ecd1 # shrinks to src = "@layer base, components;\n\n.a {\n color: var(--brand);\n}\n", cuts = [30, 6] diff --git a/native/igniter_css/tests/property.rs b/native/igniter_css/tests/property.rs index a9eb405..76f57e7 100644 --- a/native/igniter_css/tests/property.rs +++ b/native/igniter_css/tests/property.rs @@ -140,14 +140,15 @@ proptest! { /// The round-trip gate, generalised: the parse must reproduce any input. #[test] fn round_trip_holds_for_generated_stylesheets(src in stylesheet()) { - let ctx = ParseCtx::parse_default(&src); + let ctx = ParseCtx::try_new(&src, opts()).expect("generated CSS must parse"); prop_assert_eq!(ctx.restore_bom(ctx.syntax().to_string()), src); } #[test] fn round_trip_holds_for_junk(src in junk()) { - let ctx = ParseCtx::parse_default(&src); - prop_assert_eq!(ctx.restore_bom(ctx.syntax().to_string()), src); + if let Ok(ctx) = ParseCtx::try_new(&src, opts()) { + prop_assert_eq!(ctx.restore_bom(ctx.syntax().to_string()), src); + } } /// Non-overlapping edits splice cleanly and the result still parses. @@ -178,8 +179,14 @@ proptest! { .collect(); let out = apply_edits(&src, edits).expect("non-overlapping edits must apply"); - let ctx = ParseCtx::parse_default(&out); - prop_assert!(ctx.round_trips()); + + // Splicing comments at arbitrary offsets can produce input that trips + // Biome's own parser-progress assertion. The promise is not that every + // such file parses -- it is that we never let a panic escape: either we + // get a context that round-trips, or a clean error. + if let Ok(ctx) = ParseCtx::try_new(&out, opts()) { + prop_assert!(ctx.round_trips()); + } } /// Overlapping edits are always rejected, never silently merged. @@ -262,8 +269,9 @@ proptest! { fn minifying_shrinks_and_stays_valid(src in stylesheet()) { let out = igniter_css::transform::minify(&src, opts()).unwrap(); prop_assert!(out.len() <= src.len()); - let ctx = ParseCtx::parse_default(&out); - prop_assert!(ctx.round_trips()); + if let Ok(ctx) = ParseCtx::try_new(&out, opts()) { + prop_assert!(ctx.round_trips()); + } } /// Beautify then minify lands on the same text as minify alone. From d3fa63eecc818e6e9e642fd417c9c1ccfc5c707e Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 09:28:55 +0200 Subject: [PATCH 6/7] test: pin the parser panic to a fixed input, and license the proptest seed The proptest regression file committed in the previous change is the seed store proptest writes when a property fails. Its header recommends checking it in so the failing case is replayed for everyone, which is worth having -- but it carried no SPDX header, so REUSE would have failed on it. Adding the sidecar. More usefully, it already held the minimal reproducer I had been searching for by hand. proptest had shrunk the failure to a 24-byte edit, which reconstructs to: @layer/*x*/ color: var(--brand); } Raw biome_css_parser 0.5.8 panics on that; the guarded path returns Err. That is now a deterministic test rather than something only a random fuzzer finds: every entry point is asserted to return Unparseable, and one test asserts the input still panics the underlying parser -- so if Biome fixes it upstream, that test fails and tells us the guard may be removable rather than leaving it in place forever with nobody knowing why. Worth reporting upstream to Biome. Co-Authored-By: Claude Opus 5 (1M context) --- .../property.proptest-regressions.license | 3 + native/igniter_css/tests/stack_safety.rs | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 native/igniter_css/tests/property.proptest-regressions.license diff --git a/native/igniter_css/tests/property.proptest-regressions.license b/native/igniter_css/tests/property.proptest-regressions.license new file mode 100644 index 0000000..afd70dd --- /dev/null +++ b/native/igniter_css/tests/property.proptest-regressions.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2025 igniter_css contributors + +SPDX-License-Identifier: MIT diff --git a/native/igniter_css/tests/stack_safety.rs b/native/igniter_css/tests/stack_safety.rs index a8a2ae4..2237c81 100644 --- a/native/igniter_css/tests/stack_safety.rs +++ b/native/igniter_css/tests/stack_safety.rs @@ -129,3 +129,58 @@ fn the_error_says_what_happened() { "no limit given: {err}" ); } + +/// Biome 0.5.8 panics on some inputs with "The parser is no longer +/// progressing". This one was found by the property tests and shrunk by +/// proptest; the seed is kept in tests/property.proptest-regressions. +/// +/// Rustler would turn the panic into an exception in the calling process, which +/// the VM survives but which breaks the promise that unpatchable input yields +/// `{:error, reason}`. Every entry point must return an error instead. +mod parser_panics { + use super::*; + + /// `@layer` followed immediately by a comment, then an orphaned + /// declaration and a stray brace. + const PANICS_BIOME: &str = "@layer/*x*/\n color: var(--brand);\n}\n"; + + #[test] + fn the_input_still_panics_the_underlying_parser() { + // If this ever stops panicking, Biome has fixed it upstream and the + // guard below is belt and braces rather than load-bearing. + let panicked = std::panic::catch_unwind(|| { + let _ = igniter_css::ctx::ParseCtx::parse_default(PANICS_BIOME); + }) + .is_err(); + assert!( + panicked, + "biome no longer panics on this input -- the guard may be removable" + ); + } + + #[test] + fn every_entry_point_returns_an_error_rather_than_unwinding() { + assert!(ensure_at_rule_line(PANICS_BIOME, "@plugin \"p\";", opts()).is_err()); + assert!(ensure_rule(PANICS_BIOME, ".probe", opts()).is_err()); + assert!(sort_properties(PANICS_BIOME, opts()).is_err()); + assert!(remove_duplicates(PANICS_BIOME, DedupeOptions::default(), opts()).is_err()); + assert!(has_rule(PANICS_BIOME, ".a", opts()).is_err()); + assert!(analyze::analyze(PANICS_BIOME, opts()).is_err()); + assert!(analyze::extract_colors(PANICS_BIOME, opts()).is_err()); + assert!(transform::minify(PANICS_BIOME, opts()).is_err()); + assert!(transform::beautify(PANICS_BIOME, opts()).is_err()); + + let v = analyze::validate(PANICS_BIOME, opts()); + assert!(!v.valid); + assert!(!v.round_trips); + } + + #[test] + fn the_error_is_the_documented_one() { + let err = ensure_rule(PANICS_BIOME, ".probe", opts()).unwrap_err(); + assert!( + matches!(err, igniter_css::error::CssError::Unparseable(_)), + "expected Unparseable, got {err:?}" + ); + } +} From ab65c907dd1a86a635bfff1f57e196dfa8f87830 Mon Sep 17 00:00:00 2001 From: Shahryar Tavakkoli Date: Tue, 4 Aug 2026 09:44:10 +0200 Subject: [PATCH 7/7] docs: add release and codemod skills Two project skills, written for an agent rather than a human reader: constraints first, exact commands, and the failure modes with their causes. `release` covers the tag-triggered flow. The point worth encoding is that a tag is the only manual step -- the checksum generation and Hex publish run in CI, and doing them by hand is a sign something is broken. It also lists the four failures seen in practice: the missing permissions block, OTP too old for hex.pm's certificate chain, a poisoned Actions cache, and the 404 you get from building locally before a release exists. `css-codemod` covers the parts an agent gets wrong here. Chiefly: never reprint the tree, and decide from the CST rather than by scanning text -- with a table of what Biome already models, because every text scanner in this codebase was eventually replaced by a node kind. It also records the safety rules that are not obvious, notably that rustler catches panics but a stack overflow aborts and takes the VM down, and the quadratic trap of calling find_all_* inside a per-node loop. Both carry SPDX sidecars so REUSE stays green. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/css-codemod/SKILL.md | 85 +++++++++++++++++++++ .claude/skills/css-codemod/SKILL.md.license | 3 + .claude/skills/release/SKILL.md | 54 +++++++++++++ .claude/skills/release/SKILL.md.license | 3 + 4 files changed, 145 insertions(+) create mode 100644 .claude/skills/css-codemod/SKILL.md create mode 100644 .claude/skills/css-codemod/SKILL.md.license create mode 100644 .claude/skills/release/SKILL.md create mode 100644 .claude/skills/release/SKILL.md.license diff --git a/.claude/skills/css-codemod/SKILL.md b/.claude/skills/css-codemod/SKILL.md new file mode 100644 index 0000000..029ad7b --- /dev/null +++ b/.claude/skills/css-codemod/SKILL.md @@ -0,0 +1,85 @@ +--- +name: css-codemod +description: Add or change a CSS codemod in igniter_css (native/igniter_css). Use when editing Rust in this repo, adding an operation, touching selector or at-rule matching, or debugging a codemod that reformats too much. Covers the byte-range architecture, the Biome CST, and the required tests. +--- + +# CSS codemods in igniter_css + +## The one rule + +**Never reprint the tree.** Parse losslessly → locate byte ranges → splice text +into the original source. Text outside an edit cannot change, which is why +comments survive by construction. Any change that produces output by printing a +node is wrong. + +``` +parse_css() → locate nodes → node.text_trimmed_range() → Vec → apply_edits(original) +``` + +`text_range()` includes leading trivia (the comment above the node). +`text_trimmed_range()` is the node's own bytes. Almost always you want trimmed. + +## Decide from the CST, never by scanning text + +Biome already models what you are about to hand-parse: + +| need | node/token | +|---|---| +| combinator | `CSS_COMPLEX_SELECTOR` + token; descendant is `CSS_SPACE_LITERAL` | +| nested rule | `CSS_NESTED_QUALIFIED_RULE` + `CSS_RELATIVE_SELECTOR_LIST` (**not** `CSS_QUALIFIED_RULE`) | +| hex colour | `CSS_COLOR` / `CSS_COLOR_LITERAL` (validate digits: `#notahex` parses as a colour) | +| colour fn | `CSS_FUNCTION` + identifier name | +| url payload | `CSS_URL_FUNCTION` / `CSS_URL_VALUE_RAW` — structurally not an identifier | +| at-rule target | first `CSS_STRING_LITERAL` / `CSS_URL_VALUE_RAW_LITERAL` before the block | +| comments | token leading/trailing trivia | + +Text scanning is acceptable in exactly two places: caller-supplied strings that +are not CSS, and the pre-parse nesting guard (which must not recurse, because +recursing is what it prevents). + +## Safety + +- No `unwrap`/`expect`/indexing reachable from a NIF. Rustler catches panics, but + a **stack overflow aborts** and takes the VM down. +- Every parse goes through `ParseCtx::try_new` — Biome 0.5.8 panics on some + input ("parser is no longer progressing"). +- `check_nesting` before any parse. Limit 256; real CSS is depth ~7. +- Caller text spliced in must pass the same nesting limit, or you write a file + you would refuse to read. + +## Matching + +Top-level only unless the caller opts in. Normalised comparison, never substring +or fuzzy. A selector list matches whole. **More than one match is an error** — +never pick one. + +## Required per codemod + +Same commit, no exceptions: + +1. golden test (input + op → expected output, exact string) +2. idempotency test (twice == once, second reports `changed: false`) +3. comment-placement test (trailing, adjacent-above, blank-line-separated, section header) + +Then add the op to the sweeps in `tests/corpus_invariants.rs` and +`test/corpus_invariants_test.exs`, which assert those properties for every op +against every fixture. + +## Verify + +```bash +cd native/igniter_css +cargo test && cargo fmt --check && cargo clippy --all-targets -- -D warnings +cd - && IGNITERCSS_BUILD=1 mix test && mix credo --strict +``` + +`tests/roundtrip.rs` is the gate: `parse.syntax().to_string() == source` across +the whole corpus. If it fails, byte-range editing is unsafe — stop. + +## Gotchas + +- BOM: stripped in `ParseCtx`, restored on output. Biome lexes U+FEFF into the + first identifier and silently breaks selector matching otherwise. +- Unbalanced braces: refuse. "Top level" is meaningless in such a file. +- Never call `find_all_*` inside a per-node loop — that is quadratic. Build the + ref from the node you already hold (`locate::at_rule_ref`). diff --git a/.claude/skills/css-codemod/SKILL.md.license b/.claude/skills/css-codemod/SKILL.md.license new file mode 100644 index 0000000..afd70dd --- /dev/null +++ b/.claude/skills/css-codemod/SKILL.md.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2025 igniter_css contributors + +SPDX-License-Identifier: MIT diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md new file mode 100644 index 0000000..4abdc9a --- /dev/null +++ b/.claude/skills/release/SKILL.md @@ -0,0 +1,54 @@ +--- +name: release +description: Cut an igniter_css release. Use when asked to release, tag, publish to Hex, bump the version, or when a release failed. Covers the tag-triggered CI flow, the precompiled NIF matrix, and the checksum file. +--- + +# Release igniter_css + +A tag is the only trigger. Everything after it is automatic — do not run the +checksum or publish steps by hand unless CI is broken. + +## Steps + +1. Bump `@version` in `mix.exs`. +2. Add a `# Changelog for IgniterCss X.Y.Z` section to `CHANGELOG.md` — the + GitHub release notes are extracted from it by heading match. +3. Commit and push to `main`. +4. `git tag vX.Y.Z && git push origin vX.Y.Z` + +## What CI then does + +| job | result | +|---|---| +| checks | credo, dialyzer, test, format, sobelow, reuse, cargo test/fmt/clippy | +| `build-release` | 10 targets → `libigniter_css-vX.Y.Z-nif-2.15-.so.tar.gz` attached to the GitHub release | +| `hex_publish` | `mix rustler_precompiled.download IgniterCss.Native --only-local --all --print` → `checksum-Elixir.IgniterCss.Native.exs`, then `mix hex.publish` | +| `github_release` | release notes from CHANGELOG | + +## Invariants + +- `checksum-Elixir.IgniterCss.Native.exs` is **never committed**. It hashes + artifacts that do not exist until the tag builds. It is listed in `files:` in + `mix.exs` so it ships in the Hex package. +- `targets:` in `lib/igniter_css/native.ex` must match the CI matrix exactly. A + target built but not listed is never downloaded; one listed but not built is a + hard failure for those users. +- `release:` is not passed in `.github/workflows/elixir.yml`, so it defaults to + `true`. That is what enables `hex_publish`. + +## Failure modes + +| symptom | cause | +|---|---| +| `startup_failure`, "workflow file issue" | caller lacks `permissions:` the callee needs. `elixir.yml` must grant `contents/pages/id-token/security-events: write` | +| `Could not mix rebar from any hex.pm mirror` | OTP too old for hex.pm's cert chain. Needs OTP 28+ | +| `Hex.State ... does not exist` | poisoned `mix-home`/`hex-home` Actions cache from a failed run on a different OTP. Delete the caches via `gh api -X DELETE repos/OWNER/REPO/actions/caches/ID` and re-run | +| 404 downloading the NIF locally | no release exists yet. Use `IGNITERCSS_BUILD=1 mix compile` | + +## Manual fallback + +```bash +mix rustler_precompiled.download IgniterCss.Native --all --print +mix hex.build --unpack +mix hex.publish +``` diff --git a/.claude/skills/release/SKILL.md.license b/.claude/skills/release/SKILL.md.license new file mode 100644 index 0000000..afd70dd --- /dev/null +++ b/.claude/skills/release/SKILL.md.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2025 igniter_css contributors + +SPDX-License-Identifier: MIT