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 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/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 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/analyze.rs b/native/igniter_css/src/analyze.rs index 6bd7a83..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()) }) @@ -308,10 +309,14 @@ 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 - .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)) @@ -568,7 +573,25 @@ 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 { - let ctx = ParseCtx::new(source, options); + if let Err(e) = crate::ctx::check_nesting(source) { + return Validation { + valid: false, + diagnostics: 0, + round_trips: false, + message: e.to_string(), + }; + } + 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 de14d77..0239512 100644 --- a/native/igniter_css/src/ctx.rs +++ b/native/igniter_css/src/ctx.rs @@ -4,10 +4,10 @@ //! `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). +//! types, so an upgrade touches two files rather than twenty. use biome_css_parser::{parse_css, CssParse, CssParserOptions}; use biome_css_syntax::{CssSyntaxKind, CssSyntaxNode}; @@ -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, @@ -112,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 } @@ -130,8 +210,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 +254,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 +300,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 9407fe0..a86b34f 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. @@ -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()); @@ -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/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 47cacdd..0602e5f 100644 --- a/native/igniter_css/src/locate.rs +++ b/native/igniter_css/src/locate.rs @@ -2,16 +2,15 @@ // // 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 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; //! * 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() @@ -272,9 +271,14 @@ 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 + 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); @@ -303,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); @@ -355,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; @@ -402,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(" "); @@ -451,7 +469,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()), @@ -546,7 +564,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 +687,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 @@ -797,9 +815,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)" @@ -1119,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/nif.rs b/native/igniter_css/src/nif.rs index 99de99e..c2b9878 100644 --- a/native/igniter_css/src/nif.rs +++ b/native/igniter_css/src/nif.rs @@ -2,12 +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 (ROADMAP §8 -//! Phase 4). +//! 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 9567f90..bc94323 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; @@ -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(';') { @@ -68,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:?}" @@ -135,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()) { @@ -159,7 +157,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/declaration.rs b/native/igniter_css/src/ops/declaration.rs index 33b98b3..a7911b3 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, @@ -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 { @@ -153,7 +154,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 +188,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 e4f3ae4..1d9eac1 100644 --- a/native/igniter_css/src/ops/mod.rs +++ b/native/igniter_css/src/ops/mod.rs @@ -2,15 +2,15 @@ // // 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, +//! * **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,15 +47,16 @@ 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>, { - let ctx = ParseCtx::new(source, options); + crate::ctx::check_nesting(source)?; + 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(), @@ -89,7 +90,8 @@ pub fn query(source: &str, options: ParseOptions, f: F) -> Result where F: FnOnce(&ParseCtx) -> Result, { - let ctx = ParseCtx::new(source, options); + crate::ctx::check_nesting(source)?; + 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(), @@ -145,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())]; }; @@ -179,6 +183,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/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/transform.rs b/native/igniter_css/src/transform.rs index 950d457..31a98cf 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; @@ -37,7 +37,8 @@ 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 { - let ctx = ParseCtx::new(source, options); + crate::ctx::check_nesting(source)?; + let ctx = ParseCtx::try_new(source, options)?; let mut out = String::with_capacity(source.len()); let mut prev_end: Option = None; @@ -90,7 +91,8 @@ 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 { - let ctx = ParseCtx::new(source, options); + crate::ctx::check_nesting(source)?; + 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/src/trivia.rs b/native/igniter_css/src/trivia.rs index 7565de5..56656c9 100644 --- a/native/igniter_css/src/trivia.rs +++ b/native/igniter_css/src/trivia.rs @@ -2,10 +2,9 @@ // // 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: +//! 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 55430e2..73ed35f 100644 --- a/native/igniter_css/tests/corpus_invariants.rs +++ b/native/igniter_css/tests/corpus_invariants.rs @@ -2,17 +2,17 @@ // // 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. //! //! 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.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.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/property.rs b/native/igniter_css/tests/property.rs index 7111065..76f57e7 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, @@ -137,17 +137,18 @@ 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); + 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. diff --git a/native/igniter_css/tests/phase0_roundtrip.rs b/native/igniter_css/tests/roundtrip.rs similarity index 88% rename from native/igniter_css/tests/phase0_roundtrip.rs rename to native/igniter_css/tests/roundtrip.rs index 8926292..1bb5fc2 100644 --- a/native/igniter_css/tests/phase0_roundtrip.rs +++ b/native/igniter_css/tests/roundtrip.rs @@ -2,11 +2,12 @@ // // 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 -//! 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()); } diff --git a/native/igniter_css/tests/stack_safety.rs b/native/igniter_css/tests/stack_safety.rs new file mode 100644 index 0000000..2237c81 --- /dev/null +++ b/native/igniter_css/tests/stack_safety.rs @@ -0,0 +1,186 @@ +// 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}" + ); +} + +/// 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:?}" + ); + } +}