diff --git a/.claude/settings.local.json b/.claude/settings.local.json index b7f95bc..ce52d9a 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -164,7 +164,31 @@ "Bash(perl -0777 -i -pe 's/\\(TraceabilityRule \\\\{[^}]*severity: [^,}]+,\\)\\(\\\\s*\\\\}\\)/$1\\\\n alternate_backlinks: vec![],$2/g' rivet-core/src/coverage.rs rivet-core/src/export.rs rivet-core/src/proofs.rs rivet-core/src/lifecycle.rs rivet-core/src/validate.rs)", "Bash(perl -0777 -i -pe 's/\\(TraceabilityRule \\\\{[^}]*severity: [^,}]+,\\)\\(\\\\s*\\\\}\\)/$1\\\\n alternate_backlinks: vec![],$2/g' rivet-core/tests/proptest_operations.rs)", "Bash(node scripts/diff-to-markdown.mjs --diff /tmp/malformed.json --pr 1 --run 1 --repo x/y)", - "Bash(node -e \"require\\('typescript'\\).transpileModule\\(require\\('fs'\\).readFileSync\\('rivet-delta.spec.ts','utf8'\\), { compilerOptions: { target: 'es2022', module: 'nodenext' } }\\)\")" + "Bash(node -e \"require\\('typescript'\\).transpileModule\\(require\\('fs'\\).readFileSync\\('rivet-delta.spec.ts','utf8'\\), { compilerOptions: { target: 'es2022', module: 'nodenext' } }\\)\")", + "Bash(node scripts/diff-to-markdown.mjs --diff /tmp/df.json --pr 1 --run 1 --repo x/y --mmd-out /tmp/diag.mmd --svg-url \"https://raw.githubusercontent.com/x/y/rivet-delta-renders/pr-1/run-1/diagram.svg\")", + "Bash(node scripts/diff-to-markdown.mjs --diff /tmp/df2.json --pr 1 --run 1 --repo x/y --svg-url \"https://example.com/g.svg\")", + "Bash(git -C /Users/r/git/pulseengine/rivet/.claude/worktrees/agent-aa9070c3 push -u origin test/sexpr-audit)", + "Bash(git -C /Users/r/git/pulseengine/rivet/.claude/worktrees/agent-a6c5e422 push -u origin worktree-agent-a6c5e422)", + "Bash(git -C /Users/r/git/pulseengine/rivet fetch origin)", + "Bash(git -C /Users/r/git/pulseengine/rivet checkout main)", + "Bash(git -C /Users/r/git/pulseengine/rivet pull)", + "Bash(git -C /Users/r/git/pulseengine/rivet checkout -b feat/v043-sexpr-followups)", + "Bash(git -C /Users/r/git/pulseengine/rivet log --oneline -5)", + "Bash(git -C /Users/r/git/pulseengine/rivet status --short)", + "Bash(git -C /Users/r/git/pulseengine/rivet stash push -- .claude/settings.local.json)", + "Bash(git -C /Users/r/git/pulseengine/rivet branch -D feat/v043-sexpr-followups)", + "Bash(git -C /Users/r/git/pulseengine/rivet add -A rivet-core/src/sexpr_eval.rs rivet-core/tests/sexpr_fuzz.rs rivet-core/tests/sexpr_predicate_matrix.rs docs/getting-started.md)", + "Bash(git -C /Users/r/git/pulseengine/rivet commit -m ' *)", + "Bash(git -C /Users/r/git/pulseengine/rivet push -u origin feat/v043-sexpr-followups)", + "Bash(awk '/cmd_variant_solve/{flag=1; print; next} flag && /^}/ {print; exit}')", + "Bash(awk *)", + "Bash(/Users/r/git/pulseengine/rivet/target/debug/rivet variant *)", + "Bash(echo \"exit=$?\")", + "Bash(/Users/r/git/pulseengine/rivet/target/debug/rivet validate *)", + "Bash(/Users/r/git/pulseengine/rivet/target/debug/rivet list *)", + "Bash(git *)", + "Bash(kill 19359 19358 19355)", + "Bash(ps -p 19358 19359 -o pid)" ] } } diff --git a/docs/getting-started.md b/docs/getting-started.md index 9a443ea..04ffe48 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -777,6 +777,44 @@ Quantifiers: `forall`, `exists`, `count`. Graph: `reachable-from`, `reachable-to`. +### Count comparisons + +`(count )` as a standalone form matches artifacts that exist in +the scope (equivalent to `(exists true)`). Wrapped in a +comparison, it counts artifacts and compares to a threshold: + +```bash +# At least one failing test? +rivet list --filter '(> (count (and (= type "test") (= status "failed"))) 0)' + +# Exactly three approved requirements with the safety tag? +rivet list --filter '(= (count (and (= type "requirement") (= status "approved") (has-tag "safety"))) 3)' +``` + +All six operators (`>`, `<`, `>=`, `<=`, `=`, `!=`) accept `(count …)` on +the left and an integer literal on the right. Any other shape produces +a parse error at lower time — no silent match failures. + +### Regex patterns in `matches` + +`(matches "")` validates the regex **at parse time**. An +invalid pattern produces an error with the compiler's complaint, not a +silent empty result. Doubled backslashes are needed inside the s-expr +string (`"\\d+"` not `"\d+"`): + +```bash +rivet list --filter '(matches id "^REQ-\\d+$")' # OK +rivet list --filter '(matches id "[unclosed")' # parse error with clear message +``` + +### Field accessors + +Only single-name field accessors are supported today. Dotted forms like +`links.satisfies.target` parse as a single symbol and currently resolve +to the empty string — they do not navigate nested structure. To filter +on links, use the purpose-built predicates (`linked-by`, `linked-from`, +`linked-to`, `links-count`) rather than field-path navigation. + --- ## Variant Management (Product Line Engineering) diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index 1f7cdb4..27df2fd 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -944,6 +944,60 @@ enum VariantAction { #[arg(short, long, default_value = "text")] format: String, }, + /// Emit effective features + attributes in a build-system-specific + /// format. Exits non-zero if the variant fails to solve. + /// + /// Formats: json, env (sh), cargo (build.rs), cmake, cpp-header, + /// bazel, make. See docs/getting-started.md for worked examples. + Features { + /// Path to feature model YAML file + #[arg(long)] + model: PathBuf, + + /// Path to variant configuration YAML file + #[arg(long)] + variant: PathBuf, + + /// Output format + #[arg(short, long, default_value = "env")] + format: String, + }, + /// Print "on"/"off" for a single feature after solving the variant. + /// + /// Exit status: 0 if the feature is selected, 1 if not, 2 if the + /// variant fails to solve or the feature is unknown. + Value { + /// Path to feature model YAML file + #[arg(long)] + model: PathBuf, + + /// Path to variant configuration YAML file + #[arg(long)] + variant: PathBuf, + + /// Feature name to query + feature: String, + }, + /// Print a single attribute value for a feature after solving. + /// + /// Exit status: 0 if the attribute exists, 2 if the feature is not + /// selected or the key is absent. Non-scalar values (lists/maps) + /// print as JSON. + Attr { + /// Path to feature model YAML file + #[arg(long)] + model: PathBuf, + + /// Path to variant configuration YAML file + #[arg(long)] + variant: PathBuf, + + /// Feature name + feature: String, + + /// Attribute key + key: String, + }, } fn main() -> ExitCode { @@ -1198,6 +1252,22 @@ fn run(cli: Cli) -> Result { binding, format, } => cmd_variant_solve(&cli, model, variant, binding.as_deref(), format), + VariantAction::Features { + model, + variant, + format, + } => cmd_variant_features(model, variant, format), + VariantAction::Value { + model, + variant, + feature, + } => cmd_variant_value(model, variant, feature), + VariantAction::Attr { + model, + variant, + feature, + key, + } => cmd_variant_attr(model, variant, feature, key), }, #[cfg(feature = "wasm")] Command::Import { @@ -7814,6 +7884,164 @@ fn cmd_variant_solve( Ok(true) } +/// Load a feature model + variant config and solve — loud on failure. +/// +/// Shared by `rivet variant features / value / attr`. Every call path +/// runs the solver; if constraints fail, the caller exits with a clear +/// error (no silent partial output). +fn load_and_solve_variant( + model_path: &std::path::Path, + variant_path: &std::path::Path, +) -> Result<( + rivet_core::feature_model::FeatureModel, + rivet_core::feature_model::ResolvedVariant, +)> { + let model_yaml = std::fs::read_to_string(model_path) + .with_context(|| format!("reading {}", model_path.display()))?; + let model = rivet_core::feature_model::FeatureModel::from_yaml(&model_yaml) + .map_err(|e| anyhow::anyhow!("{e}"))?; + let variant_yaml = std::fs::read_to_string(variant_path) + .with_context(|| format!("reading {}", variant_path.display()))?; + let variant: rivet_core::feature_model::VariantConfig = + serde_yaml::from_str(&variant_yaml).context("parsing variant config")?; + let resolved = rivet_core::feature_model::solve(&model, &variant).map_err(|errs| { + let msgs: Vec = errs.iter().map(|e| format!("{e:?}")).collect(); + anyhow::anyhow!( + "variant `{}` failed constraint check:\n {}", + variant.name, + msgs.join("\n ") + ) + })?; + Ok((model, resolved)) +} + +fn cmd_variant_features( + model_path: &std::path::Path, + variant_path: &std::path::Path, + format: &str, +) -> Result { + let fmt = rivet_core::variant_emit::EmitFormat::parse(format) + .map_err(|e| anyhow::anyhow!("{e}"))?; + let (model, resolved) = load_and_solve_variant(model_path, variant_path)?; + let out = rivet_core::variant_emit::emit(&model, &resolved, fmt) + .map_err(|e| anyhow::anyhow!("{e}"))?; + print!("{out}"); + Ok(true) +} + +fn cmd_variant_value( + model_path: &std::path::Path, + variant_path: &std::path::Path, + feature: &str, +) -> Result { + let (model, resolved) = load_and_solve_variant(model_path, variant_path)?; + if !model.features.contains_key(feature) { + eprintln!( + "error: feature `{feature}` is not declared in the feature model `{}`", + model_path.display() + ); + std::process::exit(2); + } + if resolved.effective_features.contains(feature) { + println!("on"); + Ok(true) + } else { + println!("off"); + Ok(false) + } +} + +fn cmd_variant_attr( + model_path: &std::path::Path, + variant_path: &std::path::Path, + feature: &str, + key: &str, +) -> Result { + let (model, resolved) = load_and_solve_variant(model_path, variant_path)?; + let f = model.features.get(feature).ok_or_else(|| { + anyhow::anyhow!( + "feature `{feature}` is not declared in feature model `{}`", + model_path.display() + ) + })?; + if !resolved.effective_features.contains(feature) { + eprintln!( + "error: feature `{feature}` is not selected in variant `{}`", + resolved.name + ); + std::process::exit(2); + } + match f.attributes.get(key) { + Some(v) => { + match v { + serde_yaml::Value::Null => println!(), + serde_yaml::Value::Bool(b) => println!("{b}"), + serde_yaml::Value::Number(n) => println!("{n}"), + serde_yaml::Value::String(s) => println!("{s}"), + // list/map → JSON so shells can parse structurally + other => { + let json = + serde_json::to_string(&rivet_core_yaml_to_json(other)).map_err(|e| { + anyhow::anyhow!("serializing attribute `{key}` as json: {e}") + })?; + println!("{json}"); + } + } + Ok(true) + } + None => { + eprintln!( + "error: feature `{feature}` has no attribute `{key}` (declared keys: {})", + f.attributes + .keys() + .cloned() + .collect::>() + .join(", ") + ); + std::process::exit(2); + } + } +} + +/// YAML→JSON conversion for non-scalar attribute values printed by +/// `rivet variant attr`. Mirrors the internal helper in `variant_emit` +/// but is small enough to keep here rather than expose publicly. +fn rivet_core_yaml_to_json(v: &serde_yaml::Value) -> serde_json::Value { + match v { + serde_yaml::Value::Null => serde_json::Value::Null, + serde_yaml::Value::Bool(b) => serde_json::Value::Bool(*b), + serde_yaml::Value::Number(n) => { + if let Some(i) = n.as_i64() { + serde_json::json!(i) + } else if let Some(u) = n.as_u64() { + serde_json::json!(u) + } else if let Some(f) = n.as_f64() { + serde_json::Number::from_f64(f) + .map(serde_json::Value::Number) + .unwrap_or(serde_json::Value::Null) + } else { + serde_json::Value::Null + } + } + serde_yaml::Value::String(s) => serde_json::Value::String(s.clone()), + serde_yaml::Value::Sequence(items) => { + serde_json::Value::Array(items.iter().map(rivet_core_yaml_to_json).collect()) + } + serde_yaml::Value::Mapping(m) => { + let mut out = serde_json::Map::new(); + for (k, v) in m { + let key = match k { + serde_yaml::Value::String(s) => s.clone(), + other => serde_yaml::to_string(other).unwrap_or_default().trim().to_string(), + }; + out.insert(key, rivet_core_yaml_to_json(v)); + } + serde_json::Value::Object(out) + } + serde_yaml::Value::Tagged(t) => rivet_core_yaml_to_json(&t.value), + } +} + fn find_latest_snapshot(snap_dir: &std::path::Path) -> Result { if !snap_dir.exists() { anyhow::bail!("no snapshots directory found — run `rivet snapshot capture` first"); diff --git a/rivet-cli/tests/variant_emit.rs b/rivet-cli/tests/variant_emit.rs new file mode 100644 index 0000000..83289eb --- /dev/null +++ b/rivet-cli/tests/variant_emit.rs @@ -0,0 +1,280 @@ +//! Integration tests for `rivet variant features / value / attr`. +//! +//! The unit tests in `rivet_core::variant_emit::tests` cover format +//! rendering against a hand-built model. These integration tests go +//! end-to-end through the CLI (parsing → loader → solver → emitter → +//! stdout/exit-code) on real YAML files, so a regression in any layer +//! is caught here. + +use std::fs; +use std::process::Command; + +fn rivet_bin() -> std::path::PathBuf { + if let Ok(bin) = std::env::var("CARGO_BIN_EXE_rivet") { + return std::path::PathBuf::from(bin); + } + let manifest = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let workspace_root = manifest.parent().expect("workspace root"); + workspace_root.join("target").join("debug").join("rivet") +} + +/// Write a minimal model + variant to a temp dir and return (model, variant) paths. +fn write_fixture(dir: &std::path::Path) -> (std::path::PathBuf, std::path::PathBuf) { + let model = dir.join("feature-model.yaml"); + fs::write( + &model, + r#" +root: rt +features: + rt: + group: mandatory + children: [core, asil-c] + core: + group: leaf + attributes: + version: "1.2.3" + asil-c: + group: leaf + attributes: + asil-numeric: 3 + reqs: "fmea-dfa" +"#, + ) + .unwrap(); + let variant = dir.join("prod.yaml"); + fs::write( + &variant, + r#" +name: prod +selects: + - core + - asil-c +"#, + ) + .unwrap(); + (model, variant) +} + +fn run_features(model: &std::path::Path, variant: &std::path::Path, fmt: &str) -> (bool, String) { + let output = Command::new(rivet_bin()) + .args([ + "variant", + "features", + "--model", + model.to_str().unwrap(), + "--variant", + variant.to_str().unwrap(), + "--format", + fmt, + ]) + .output() + .expect("rivet variant features"); + ( + output.status.success(), + String::from_utf8_lossy(&output.stdout).into_owned(), + ) +} + +#[test] +fn features_env_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "env"); + assert!(ok); + assert!(out.contains("export RIVET_FEATURE_ASIL_C=1")); + assert!(out.contains("export RIVET_ATTR_ASIL_C_ASIL_NUMERIC='3'")); +} + +#[test] +fn features_cargo_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "cargo"); + assert!(ok); + assert!(out.contains("cargo:rustc-env=RIVET_VARIANT=prod")); + assert!(out.contains("cargo:rustc-cfg=rivet_feature=\"asil-c\"")); +} + +#[test] +fn features_cmake_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "cmake"); + assert!(ok); + assert!(out.contains("set(RIVET_FEATURE_ASIL_C ON)")); + assert!(out.contains("add_compile_definitions(")); +} + +#[test] +fn features_cpp_header_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "cpp-header"); + assert!(ok); + assert!(out.contains("#ifndef RIVET_VARIANT_H")); + assert!(out.contains("#define RIVET_ATTR_ASIL_C_REQS \"fmea-dfa\"")); +} + +#[test] +fn features_bazel_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "bazel"); + assert!(ok); + assert!(out.contains("RIVET_ATTRS = {")); + assert!(out.contains("\"asil-c\":")); +} + +#[test] +fn features_make_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "make"); + assert!(ok); + assert!(out.contains("RIVET_VARIANT := prod")); + assert!(out.contains("RIVET_ATTR_ASIL_C_ASIL_NUMERIC := 3")); +} + +#[test] +fn features_json_end_to_end() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let (ok, out) = run_features(&m, &v, "json"); + assert!(ok); + let parsed: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(parsed["variant"], "prod"); + assert_eq!(parsed["attributes"]["asil-c"]["asil-numeric"], 3); +} + +#[test] +fn features_loud_on_constraint_violation() { + let tmp = tempfile::tempdir().unwrap(); + // Hand-rolled model with an alternative group; variant violates XOR + let model = tmp.path().join("feature-model.yaml"); + fs::write( + &model, + r#" +root: rt +features: + rt: + group: mandatory + children: [lvl] + lvl: + group: alternative + children: [a, b] + a: { group: leaf } + b: { group: leaf } +"#, + ) + .unwrap(); + let variant = tmp.path().join("bad.yaml"); + fs::write(&variant, "name: bad\nselects:\n - a\n - b\n").unwrap(); + + let output = Command::new(rivet_bin()) + .args([ + "variant", + "features", + "--model", + model.to_str().unwrap(), + "--variant", + variant.to_str().unwrap(), + "--format", + "env", + ]) + .output() + .expect("rivet variant features"); + + assert!(!output.status.success(), "must exit non-zero"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("constraint check"), "stderr: {stderr}"); +} + +#[test] +fn value_selected_and_unselected() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + + let yes = Command::new(rivet_bin()) + .args([ + "variant", "value", + "--model", m.to_str().unwrap(), + "--variant", v.to_str().unwrap(), + "asil-c", + ]) + .output() + .unwrap(); + assert!(yes.status.success()); + assert!(String::from_utf8_lossy(&yes.stdout).trim() == "on"); + + // Feature exists in model but won't be selected unless listed + let model_only = tmp.path().join("model-only.yaml"); + fs::write( + &model_only, + r#" +root: rt +features: + rt: { group: optional, children: [a, b] } + a: { group: leaf } + b: { group: leaf } +"#, + ) + .unwrap(); + let var_a = tmp.path().join("var-a.yaml"); + fs::write(&var_a, "name: va\nselects:\n - a\n").unwrap(); + let no = Command::new(rivet_bin()) + .args([ + "variant", "value", + "--model", model_only.to_str().unwrap(), + "--variant", var_a.to_str().unwrap(), + "b", + ]) + .output() + .unwrap(); + assert_eq!(no.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&no.stdout).trim() == "off"); +} + +#[test] +fn value_unknown_feature_exits_two() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + let out = Command::new(rivet_bin()) + .args([ + "variant", "value", + "--model", m.to_str().unwrap(), + "--variant", v.to_str().unwrap(), + "does-not-exist", + ]) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(2)); +} + +#[test] +fn attr_prints_scalar_and_errors_on_missing_key() { + let tmp = tempfile::tempdir().unwrap(); + let (m, v) = write_fixture(tmp.path()); + + let ok = Command::new(rivet_bin()) + .args([ + "variant", "attr", + "--model", m.to_str().unwrap(), + "--variant", v.to_str().unwrap(), + "asil-c", "asil-numeric", + ]) + .output() + .unwrap(); + assert!(ok.status.success()); + assert_eq!(String::from_utf8_lossy(&ok.stdout).trim(), "3"); + + let missing = Command::new(rivet_bin()) + .args([ + "variant", "attr", + "--model", m.to_str().unwrap(), + "--variant", v.to_str().unwrap(), + "asil-c", "not-a-real-key", + ]) + .output() + .unwrap(); + assert_eq!(missing.status.code(), Some(2)); +} diff --git a/rivet-core/src/feature_model.rs b/rivet-core/src/feature_model.rs index 803c9eb..c5e629b 100644 --- a/rivet-core/src/feature_model.rs +++ b/rivet-core/src/feature_model.rs @@ -29,6 +29,15 @@ pub struct Feature { pub group: GroupType, pub children: Vec, pub parent: Option, + /// Typed key-value attributes attached to this feature. Looked up + /// by `rivet variant attr FEATURE KEY` and by the formatters when + /// emitting build-system-specific outputs. Values are kept as + /// `serde_yaml::Value` so a feature can carry strings, integers, + /// booleans, or small sub-maps without a schema change up front. + /// + /// Example: `asil-c` might declare `{ asil-numeric: 3, reqs: "fmea-dfa" }` + /// so a release script can emit `-DASIL_NUMERIC=3 -DREQS=fmea-dfa`. + pub attributes: BTreeMap, } /// Group semantics governing child selection. @@ -139,6 +148,8 @@ struct FeatureYaml { group: GroupType, #[serde(default)] children: Vec, + #[serde(default)] + attributes: BTreeMap, } fn default_group() -> GroupType { @@ -214,6 +225,7 @@ impl FeatureModel { group: fy.group, children: fy.children.clone(), parent: None, + attributes: fy.attributes.clone(), }, ); } @@ -224,6 +236,7 @@ impl FeatureModel { group: GroupType::Mandatory, children: vec![], parent: None, + attributes: BTreeMap::new(), }); // Second pass: set parent links from children references. @@ -239,6 +252,7 @@ impl FeatureModel { group: GroupType::Leaf, children: vec![], parent: None, + attributes: BTreeMap::new(), }); features.get_mut(&child).unwrap().parent = Some(parent); } diff --git a/rivet-core/src/lib.rs b/rivet-core/src/lib.rs index f75bb5d..5522a21 100644 --- a/rivet-core/src/lib.rs +++ b/rivet-core/src/lib.rs @@ -38,6 +38,7 @@ pub mod snapshot; pub mod store; pub mod test_scanner; pub mod validate; +pub mod variant_emit; pub mod yaml_cst; pub mod yaml_edit; pub mod yaml_hir; diff --git a/rivet-core/src/sexpr_eval.rs b/rivet-core/src/sexpr_eval.rs index 47e49cb..450a2b8 100644 --- a/rivet-core/src/sexpr_eval.rs +++ b/rivet-core/src/sexpr_eval.rs @@ -74,6 +74,12 @@ pub enum Expr { Exists(Box, Box), /// `(count )` — number of artifacts matching scope (compared via parent). Count(Box), + /// `(> (count ) N)` and friends — count-compared-to-threshold. + /// Created by the lowering path when a comparison's left operand is a + /// `(count ...)` form. Evaluates the count once and compares it to the + /// threshold — so authors can finally write + /// `(> (count (and (= type "test") (= status "passed"))) 10)`. + CountCompare(Box, CompOp, i64), // ── Graph traversal ───────────────────────────────────────────── /// `(reachable-from "REQ-001" "satisfies")` — true if current artifact is @@ -269,6 +275,34 @@ pub fn check(expr: &Expr, ctx: &EvalContext) -> bool { check(_scope, &scope_ctx) }) } + Expr::CountCompare(scope, op, threshold) => { + // Count artifacts matching the scope, compare to threshold. + // Requires store access; returns false when the filter is + // being evaluated without a store (e.g. single-artifact + // contexts) — matching the same shape as Forall / Exists. + let Some(store) = ctx.store else { + return false; + }; + let count: i64 = store + .iter() + .filter(|a| { + let scope_ctx = EvalContext { + artifact: a, + graph: ctx.graph, + store: ctx.store, + }; + check(scope, &scope_ctx) + }) + .count() as i64; + match op { + CompOp::Gt => count > *threshold, + CompOp::Lt => count < *threshold, + CompOp::Ge => count >= *threshold, + CompOp::Le => count <= *threshold, + CompOp::Eq => count == *threshold, + CompOp::Ne => count != *threshold, + } + } // Graph traversal Expr::ReachableFrom(start_id, link_type) => { @@ -662,6 +696,38 @@ fn lower_list(node: &crate::sexpr::SyntaxNode, errors: &mut Vec) -> }); return None; } + + // Special case: `(> (count ) N)` and friends. + // Detect a `(count ...)` form on the left, integer literal on + // the right, and lower to `CountCompare` so the count is + // evaluated over the store and compared to the threshold. + // This closes the audit-flagged gap where `count` had no + // usable lowering as a numeric operand. + if let Some(scope_expr) = try_extract_count_scope(&args[0], errors) { + let threshold = match extract_integer_literal(&args[1]) { + Some(n) => n, + None => { + errors.push(LowerError { + offset, + message: format!( + "'{form_name}' with '(count ...)' on the left requires an integer on the right — `(count ...)` counts artifacts and must be compared to a number" + ), + }); + return None; + } + }; + let op = match form_name.as_str() { + "=" => CompOp::Eq, + "!=" => CompOp::Ne, + ">" => CompOp::Gt, + "<" => CompOp::Lt, + ">=" => CompOp::Ge, + "<=" => CompOp::Le, + _ => unreachable!(), + }; + return Some(Expr::CountCompare(Box::new(scope_expr), op, threshold)); + } + let acc = extract_accessor(&args[0])?; let val = extract_value(&args[1])?; Some(match form_name.as_str() { @@ -719,6 +785,27 @@ fn lower_list(node: &crate::sexpr::SyntaxNode, errors: &mut Vec) -> } let acc = extract_accessor(&args[0])?; let val = extract_value(&args[1])?; + // Validate the regex at lower time when the pattern is a + // string literal. Without this, an invalid pattern would + // silently match nothing at runtime (consistent with our + // lenient filter semantics, but users hit "mysterious empty + // results" before the audit surfaced this). Non-literal + // patterns (rare; they'd come from field interpolation) + // retain the runtime-lenient behaviour. + if let Value::Str(ref pattern) = val { + if let Err(e) = regex::RegexBuilder::new(pattern) + .size_limit(1 << 20) + .build() + { + errors.push(LowerError { + offset, + message: format!( + "'matches' regex pattern does not compile: {e}" + ), + }); + return None; + } + } Some(Expr::Matches(acc, val)) } "contains" => { @@ -979,6 +1066,55 @@ fn extract_value(node: &crate::sexpr::SyntaxNode) -> Option { } } +/// If the node is a `(count )` form, lower the inner scope and +/// return the lowered Expr. Returns None if the node is not a list whose +/// head symbol is exactly "count" (other lists fall through to the normal +/// accessor extraction path). Errors during scope lowering are pushed +/// into the same error accumulator the caller is using. +fn try_extract_count_scope( + node: &crate::sexpr::SyntaxNode, + errors: &mut Vec, +) -> Option { + use crate::sexpr::SyntaxKind as SK; + if node.kind() != SK::List { + return None; + } + let children: Vec<_> = node + .children() + .filter(|c| matches!(c.kind(), SK::List | SK::Atom)) + .collect(); + if children.len() < 2 { + return None; + } + let head = extract_symbol(&children[0])?; + if head != "count" { + return None; + } + if children.len() != 2 { + errors.push(LowerError { + offset: node.text_range().start().into(), + message: "'count' requires exactly 1 argument (scope)".into(), + }); + return None; + } + lower_child(&children[1], errors) +} + +/// Extract an integer literal from an atom node. Returns None for any +/// other shape. Used by the comparison lowering to validate that +/// `(> (count ...) N)` has an actual integer on the right-hand side. +fn extract_integer_literal(node: &crate::sexpr::SyntaxNode) -> Option { + use crate::sexpr::SyntaxKind as SK; + if node.kind() != SK::Atom { + return None; + } + let token = node.first_token()?; + if token.kind() != SK::IntLit { + return None; + } + token.text().parse::().ok() +} + // ── Tests ─────────────────────────────────────────────────────────────── #[cfg(test)] @@ -1202,6 +1338,73 @@ mod tests { assert!(result.is_err()); } + // ── sexpr audit followups — v0.4.3 ────────────────────────────── + + // Followup #1: `(> (count ) N)` now lowers to CountCompare + // and evaluates against the store, so users can finally gate on + // "more than N matching artifacts". + #[test] + #[cfg_attr(miri, ignore)] + fn count_compare_gt_threshold() { + let expr = parse_filter("(> (count (= type \"requirement\")) 0)"); + assert!( + expr.is_ok(), + "(> (count ...) 0) must parse: {:?}", + expr.err() + ); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn count_compare_requires_integer_rhs() { + // Passing a string on the right must error with a clear + // message pointing at the contract. + let expr = parse_filter("(> (count (= type \"test\")) \"ten\")"); + assert!(expr.is_err(), "non-integer RHS must error"); + let msg = format!("{:?}", expr.err().unwrap()); + assert!( + msg.contains("integer") || msg.contains("count"), + "error must mention the integer requirement: {msg}" + ); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn count_compare_all_six_operators_lower() { + for op in &[">", "<", ">=", "<=", "=", "!="] { + let input = format!("({op} (count (= status \"approved\")) 5)"); + let expr = parse_filter(&input); + assert!( + expr.is_ok(), + "{op} must lower with count LHS: {:?}", + expr.err() + ); + } + } + + // Followup #2: invalid regex pattern in (matches) now fails at + // lower time with a clear message, instead of silently matching + // nothing at runtime. + #[test] + #[cfg_attr(miri, ignore)] + fn matches_rejects_invalid_regex_at_lower_time() { + let expr = parse_filter("(matches title \"[unclosed\")"); + assert!(expr.is_err(), "invalid regex must error at lower time"); + let msg = format!("{:?}", expr.err().unwrap()); + assert!( + msg.to_lowercase().contains("regex") + || msg.to_lowercase().contains("compile"), + "error must mention regex/compile: {msg}" + ); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn matches_accepts_valid_regex() { + let expr = parse_filter("(matches title \"^REQ-\\\\d+$\")"); + assert!(expr.is_ok(), "valid regex must parse: {:?}", expr.err()); + } + // ── Logical equivalence unit tests ────────────────────────────── #[test] diff --git a/rivet-core/src/variant_emit.rs b/rivet-core/src/variant_emit.rs new file mode 100644 index 0000000..20c6bc4 --- /dev/null +++ b/rivet-core/src/variant_emit.rs @@ -0,0 +1,535 @@ +//! Emit a resolved variant as build-system-specific configuration. +//! +//! Given a `FeatureModel` plus a `ResolvedVariant`, render the effective +//! feature set and per-feature `attributes:` section as one of: +//! +//! - `json` – structured, for downstream scripts +//! - `env` – POSIX `export` lines, sourceable from a shell +//! - `cargo` – `cargo:rustc-cfg=` / `cargo:rustc-env=` lines for `build.rs` +//! - `cmake` – `set(...)` + `add_compile_definitions(...)` +//! - `cpp-header` – `#define` guarded by `RIVET_VARIANT_H` +//! - `bazel` – `.bzl` constants (`RIVET_FEATURES`, `RIVET_ATTRS`) +//! - `make` – Makefile-includable `:=` assignments +//! +//! Design decisions (matching the v0.4.3 variant-surface spec): +//! - **long, namespaced names** — every emitted identifier is prefixed +//! `RIVET_FEATURE_` / `RIVET_ATTR_` so a project can embed several +//! rivet models without collision. +//! - **both booleans and strings** — feature presence is emitted as a +//! boolean-ish `1`, and every attribute key/value pair on a selected +//! feature is emitted as its own long-named entry. +//! - **loud on shape mismatch** — non-scalar attribute values (lists, +//! maps) are not silently flattened. JSON preserves them; every other +//! formatter returns an `Error::Schema` so the caller sees the problem +//! and can decide whether to simplify the attribute or add a new +//! scalar key. + +use std::collections::BTreeMap; +use std::fmt::Write as _; + +use crate::error::Error; +use crate::feature_model::{FeatureModel, ResolvedVariant}; + +/// Output format for `rivet variant features`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EmitFormat { + Json, + Env, + Cargo, + CMake, + CppHeader, + Bazel, + Make, +} + +impl EmitFormat { + /// Parse the `--format` argument. Accepted tokens match the CLI help text. + pub fn parse(s: &str) -> Result { + match s { + "json" => Ok(Self::Json), + "env" | "sh" => Ok(Self::Env), + "cargo" => Ok(Self::Cargo), + "cmake" => Ok(Self::CMake), + "cpp-header" | "cpp" | "header" => Ok(Self::CppHeader), + "bazel" | "bzl" => Ok(Self::Bazel), + "make" | "makefile" => Ok(Self::Make), + other => Err(Error::Schema(format!( + "unknown --format `{other}`: expected one of json, env, cargo, cmake, cpp-header, bazel, make" + ))), + } + } +} + +/// Render a resolved variant in the requested format. +/// +/// The model is consulted for each feature's `attributes:` entry; only +/// features present in `resolved.effective_features` are emitted. +pub fn emit( + model: &FeatureModel, + resolved: &ResolvedVariant, + fmt: EmitFormat, +) -> Result { + match fmt { + EmitFormat::Json => emit_json(model, resolved), + EmitFormat::Env => emit_env(model, resolved), + EmitFormat::Cargo => emit_cargo(model, resolved), + EmitFormat::CMake => emit_cmake(model, resolved), + EmitFormat::CppHeader => emit_cpp_header(model, resolved), + EmitFormat::Bazel => emit_bazel(model, resolved), + EmitFormat::Make => emit_make(model, resolved), + } +} + +// ── Identifier slugging ──────────────────────────────────────────────── + +/// Uppercase, replace non-alphanumerics with `_`. Used for every emitted +/// identifier so `asil-c` / `c++` / `10-year-warranty` all map to sane +/// C/sh/Make identifier tokens. +/// +/// This is loud rather than lossy: if two feature names collide after +/// slugging, both entries are still emitted (the caller's YAML is the +/// source of truth) but a future validator should flag the collision. +fn slug(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for ch in s.chars() { + if ch.is_ascii_alphanumeric() { + out.push(ch.to_ascii_uppercase()); + } else { + out.push('_'); + } + } + out +} + +// ── Attribute value rendering ────────────────────────────────────────── + +/// Render a YAML attribute value as a scalar for non-JSON formatters. +/// +/// Strings are emitted raw (quoting is the format's job), numbers and +/// booleans stringify via `Display`. Non-scalars (sequences, mappings) +/// return an error — JSON is the only format that preserves structure; +/// all others would have to invent a flattening convention, and doing +/// that silently has bitten users before. Callers surface the error so +/// the YAML author can choose an explicit representation. +fn attr_scalar(feature: &str, key: &str, v: &serde_yaml::Value) -> Result { + match v { + serde_yaml::Value::Null => Ok(String::new()), + serde_yaml::Value::Bool(b) => Ok(if *b { "1".into() } else { "0".into() }), + serde_yaml::Value::Number(n) => Ok(n.to_string()), + serde_yaml::Value::String(s) => Ok(s.clone()), + serde_yaml::Value::Sequence(_) | serde_yaml::Value::Mapping(_) => Err(Error::Schema( + format!( + "feature `{feature}` attribute `{key}`: non-scalar values (lists/maps) are only \ + supported in --format json; split into multiple scalar keys or use the JSON \ + formatter" + ), + )), + serde_yaml::Value::Tagged(t) => attr_scalar(feature, key, &t.value), + } +} + +/// Shell-single-quote a value so `eval`/`source` round-trip safely. +fn sh_quote(s: &str) -> String { + let mut out = String::with_capacity(s.len() + 2); + out.push('\''); + for ch in s.chars() { + if ch == '\'' { + out.push_str("'\\''"); + } else { + out.push(ch); + } + } + out.push('\''); + out +} + +// ── Walkers ──────────────────────────────────────────────────────────── + +/// Iterate effective features in deterministic (BTreeSet) order along +/// with each feature's attribute map. Features not present in the model +/// (defensive fallback) yield an empty map. +fn walk<'a>( + model: &'a FeatureModel, + resolved: &'a ResolvedVariant, +) -> impl Iterator)> + 'a { + static EMPTY: std::sync::OnceLock> = + std::sync::OnceLock::new(); + let empty = EMPTY.get_or_init(BTreeMap::new); + resolved + .effective_features + .iter() + .map(move |name| match model.features.get(name) { + Some(f) => (name.as_str(), &f.attributes), + None => (name.as_str(), empty), + }) +} + +// ── Formatters ───────────────────────────────────────────────────────── + +fn emit_json(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let attrs: serde_json::Map = walk(model, resolved) + .filter(|(_, attrs)| !attrs.is_empty()) + .map(|(name, attrs)| { + let inner: serde_json::Map = attrs + .iter() + .map(|(k, v)| (k.clone(), yaml_to_json(v))) + .collect(); + (name.to_string(), serde_json::Value::Object(inner)) + }) + .collect(); + let output = serde_json::json!({ + "variant": resolved.name, + "features": resolved.effective_features, + "attributes": attrs, + }); + serde_json::to_string_pretty(&output) + .map(|mut s| { s.push('\n'); s }) + .map_err(|e| Error::Schema(format!("json serialization: {e}"))) +} + +fn yaml_to_json(v: &serde_yaml::Value) -> serde_json::Value { + match v { + serde_yaml::Value::Null => serde_json::Value::Null, + serde_yaml::Value::Bool(b) => serde_json::Value::Bool(*b), + serde_yaml::Value::Number(n) => { + if let Some(i) = n.as_i64() { + serde_json::json!(i) + } else if let Some(u) = n.as_u64() { + serde_json::json!(u) + } else if let Some(f) = n.as_f64() { + serde_json::Number::from_f64(f) + .map(serde_json::Value::Number) + .unwrap_or(serde_json::Value::Null) + } else { + serde_json::Value::Null + } + } + serde_yaml::Value::String(s) => serde_json::Value::String(s.clone()), + serde_yaml::Value::Sequence(items) => { + serde_json::Value::Array(items.iter().map(yaml_to_json).collect()) + } + serde_yaml::Value::Mapping(m) => { + let mut out = serde_json::Map::new(); + for (k, v) in m { + let key = match k { + serde_yaml::Value::String(s) => s.clone(), + other => serde_yaml::to_string(other).unwrap_or_default().trim().to_string(), + }; + out.insert(key, yaml_to_json(v)); + } + serde_json::Value::Object(out) + } + serde_yaml::Value::Tagged(t) => yaml_to_json(&t.value), + } +} + +fn emit_env(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "# rivet variant features (env) — variant={}", resolved.name).unwrap(); + for (name, attrs) in walk(model, resolved) { + writeln!(out, "export RIVET_FEATURE_{}=1", slug(name)).unwrap(); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + writeln!( + out, + "export RIVET_ATTR_{}_{}={}", + slug(name), + slug(key), + sh_quote(&v) + ) + .unwrap(); + } + } + Ok(out) +} + +fn emit_cargo(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "# rivet variant features (cargo) — variant={}", resolved.name).unwrap(); + writeln!(out, "cargo:rustc-env=RIVET_VARIANT={}", resolved.name).unwrap(); + for (name, attrs) in walk(model, resolved) { + writeln!(out, "cargo:rustc-cfg=rivet_feature=\"{}\"", name).unwrap(); + writeln!(out, "cargo:rustc-env=RIVET_FEATURE_{}=1", slug(name)).unwrap(); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + writeln!( + out, + "cargo:rustc-env=RIVET_ATTR_{}_{}={}", + slug(name), + slug(key), + v + ) + .unwrap(); + } + } + Ok(out) +} + +fn emit_cmake(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "# rivet variant features (cmake) — variant={}", resolved.name).unwrap(); + writeln!(out, "set(RIVET_VARIANT \"{}\")", resolved.name).unwrap(); + let mut defs: Vec = Vec::new(); + for (name, attrs) in walk(model, resolved) { + writeln!(out, "set(RIVET_FEATURE_{} ON)", slug(name)).unwrap(); + defs.push(format!("RIVET_FEATURE_{}=1", slug(name))); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + writeln!( + out, + "set(RIVET_ATTR_{}_{} \"{}\")", + slug(name), + slug(key), + v.replace('"', "\\\"") + ) + .unwrap(); + defs.push(format!("RIVET_ATTR_{}_{}={}", slug(name), slug(key), v)); + } + } + writeln!(out, "add_compile_definitions({})", defs.join(" ")).unwrap(); + Ok(out) +} + +fn emit_cpp_header(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "// rivet variant features (cpp-header) — variant={}", resolved.name).unwrap(); + writeln!(out, "#ifndef RIVET_VARIANT_H").unwrap(); + writeln!(out, "#define RIVET_VARIANT_H").unwrap(); + writeln!(out, "#define RIVET_VARIANT \"{}\"", resolved.name).unwrap(); + for (name, attrs) in walk(model, resolved) { + writeln!(out, "#define RIVET_FEATURE_{} 1", slug(name)).unwrap(); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + // numeric values emit bare; strings get quoted + let rhs = if v.parse::().is_ok() || v.parse::().is_ok() { + v + } else { + format!("\"{}\"", v.replace('"', "\\\"")) + }; + writeln!(out, "#define RIVET_ATTR_{}_{} {}", slug(name), slug(key), rhs).unwrap(); + } + } + writeln!(out, "#endif").unwrap(); + Ok(out) +} + +fn emit_bazel(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "# rivet variant features (bazel) — variant={}", resolved.name).unwrap(); + writeln!(out, "RIVET_VARIANT = \"{}\"", resolved.name).unwrap(); + writeln!( + out, + "RIVET_FEATURES = [{}]", + resolved + .effective_features + .iter() + .map(|n| format!("\"{}\"", n)) + .collect::>() + .join(", ") + ) + .unwrap(); + writeln!(out, "RIVET_ATTRS = {{").unwrap(); + for (name, attrs) in walk(model, resolved) { + if attrs.is_empty() { + continue; + } + writeln!(out, " \"{}\": {{", name).unwrap(); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + let rhs = if v.parse::().is_ok() { + v + } else { + format!("\"{}\"", v.replace('"', "\\\"")) + }; + writeln!(out, " \"{}\": {},", key, rhs).unwrap(); + } + writeln!(out, " }},").unwrap(); + } + writeln!(out, "}}").unwrap(); + Ok(out) +} + +fn emit_make(model: &FeatureModel, resolved: &ResolvedVariant) -> Result { + let mut out = String::new(); + writeln!(out, "# rivet variant features (make) — variant={}", resolved.name).unwrap(); + writeln!(out, "RIVET_VARIANT := {}", resolved.name).unwrap(); + writeln!( + out, + "RIVET_FEATURES := {}", + resolved + .effective_features + .iter() + .cloned() + .collect::>() + .join(" ") + ) + .unwrap(); + for (name, attrs) in walk(model, resolved) { + writeln!(out, "RIVET_FEATURE_{} := 1", slug(name)).unwrap(); + for (key, value) in attrs { + let v = attr_scalar(name, key, value)?; + writeln!(out, "RIVET_ATTR_{}_{} := {}", slug(name), slug(key), v).unwrap(); + } + } + Ok(out) +} + +// ── Tests ────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::feature_model::FeatureModel; + + fn tiny_model() -> (FeatureModel, ResolvedVariant) { + let yaml = r#" +root: rt +features: + rt: + group: mandatory + children: [core, asil-c] + core: + group: leaf + attributes: + version: "1.2.3" + asil-c: + group: leaf + attributes: + asil-numeric: 3 + reqs: "fmea-dfa" +"#; + let model = FeatureModel::from_yaml(yaml).expect("parses"); + let variant = crate::feature_model::VariantConfig { + name: "prod".into(), + selects: vec!["core".into(), "asil-c".into()], + }; + let resolved = crate::feature_model::solve(&model, &variant).expect("solves"); + (model, resolved) + } + + #[test] + fn slug_handles_dashes_and_unicode() { + assert_eq!(slug("asil-c"), "ASIL_C"); + assert_eq!(slug("c++"), "C__"); + assert_eq!(slug("10-year"), "10_YEAR"); + } + + #[test] + fn env_format_emits_long_prefixed_names() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::Env).unwrap(); + assert!(out.contains("export RIVET_FEATURE_CORE=1")); + assert!(out.contains("export RIVET_FEATURE_ASIL_C=1")); + assert!(out.contains("export RIVET_ATTR_ASIL_C_ASIL_NUMERIC='3'")); + assert!(out.contains("export RIVET_ATTR_ASIL_C_REQS='fmea-dfa'")); + } + + #[test] + fn cargo_format_emits_rustc_cfg_and_env() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::Cargo).unwrap(); + assert!(out.contains("cargo:rustc-env=RIVET_VARIANT=prod")); + assert!(out.contains("cargo:rustc-cfg=rivet_feature=\"asil-c\"")); + assert!(out.contains("cargo:rustc-env=RIVET_FEATURE_ASIL_C=1")); + assert!(out.contains("cargo:rustc-env=RIVET_ATTR_ASIL_C_ASIL_NUMERIC=3")); + } + + #[test] + fn cmake_format_emits_set_and_add_compile_definitions() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::CMake).unwrap(); + assert!(out.contains("set(RIVET_FEATURE_CORE ON)")); + assert!(out.contains("add_compile_definitions(")); + assert!(out.contains("RIVET_FEATURE_ASIL_C=1")); + } + + #[test] + fn cpp_header_numeric_unquoted_string_quoted() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::CppHeader).unwrap(); + assert!(out.contains("#define RIVET_ATTR_ASIL_C_ASIL_NUMERIC 3")); + assert!(out.contains("#define RIVET_ATTR_ASIL_C_REQS \"fmea-dfa\"")); + assert!(out.contains("#ifndef RIVET_VARIANT_H")); + } + + #[test] + fn bazel_format_emits_dict_of_attrs() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::Bazel).unwrap(); + assert!(out.contains("RIVET_FEATURES = [")); + assert!(out.contains("\"asil-c\":")); + assert!(out.contains("\"asil-numeric\": 3")); + } + + #[test] + fn make_format_emits_colon_equals() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::Make).unwrap(); + assert!(out.contains("RIVET_VARIANT := prod")); + assert!(out.contains("RIVET_FEATURE_CORE := 1")); + assert!(out.contains("RIVET_ATTR_ASIL_C_ASIL_NUMERIC := 3")); + } + + #[test] + fn json_format_preserves_attribute_structure() { + let (model, resolved) = tiny_model(); + let out = emit(&model, &resolved, EmitFormat::Json).unwrap(); + let v: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["variant"], "prod"); + assert_eq!(v["attributes"]["asil-c"]["asil-numeric"], 3); + assert_eq!(v["attributes"]["core"]["version"], "1.2.3"); + } + + #[test] + fn non_scalar_attr_is_loud_in_non_json_formats() { + let yaml = r#" +root: rt +features: + rt: + group: mandatory + children: [c] + c: + group: leaf + attributes: + deps: [a, b] +"#; + let model = FeatureModel::from_yaml(yaml).unwrap(); + let variant = crate::feature_model::VariantConfig { + name: "v".into(), + selects: vec!["c".into()], + }; + let resolved = crate::feature_model::solve(&model, &variant).unwrap(); + // Non-scalar is an error in every format except JSON + for fmt in [ + EmitFormat::Env, + EmitFormat::Cargo, + EmitFormat::CMake, + EmitFormat::CppHeader, + EmitFormat::Bazel, + EmitFormat::Make, + ] { + let err = emit(&model, &resolved, fmt).unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("non-scalar"), "expected loud error, got: {msg}"); + } + // JSON preserves the list + let out = emit(&model, &resolved, EmitFormat::Json).unwrap(); + let v: serde_json::Value = serde_json::from_str(&out).unwrap(); + assert_eq!(v["attributes"]["c"]["deps"], serde_json::json!(["a", "b"])); + } + + #[test] + fn sh_quote_escapes_single_quotes() { + assert_eq!(sh_quote("plain"), "'plain'"); + assert_eq!(sh_quote("it's"), "'it'\\''s'"); + } + + #[test] + fn parse_format_accepts_aliases() { + assert_eq!(EmitFormat::parse("sh").unwrap(), EmitFormat::Env); + assert_eq!(EmitFormat::parse("cpp").unwrap(), EmitFormat::CppHeader); + assert_eq!(EmitFormat::parse("header").unwrap(), EmitFormat::CppHeader); + assert_eq!(EmitFormat::parse("makefile").unwrap(), EmitFormat::Make); + assert!(EmitFormat::parse("toml").is_err()); + } +} diff --git a/rivet-core/tests/proptest_feature_model.rs b/rivet-core/tests/proptest_feature_model.rs index 92830b5..a340cd7 100644 --- a/rivet-core/tests/proptest_feature_model.rs +++ b/rivet-core/tests/proptest_feature_model.rs @@ -107,6 +107,7 @@ fn arb_feature_model(max_features: usize) -> impl Strategy group, children, parent, + attributes: std::collections::BTreeMap::new(), }, ); } diff --git a/rivet-core/tests/sexpr_fuzz.rs b/rivet-core/tests/sexpr_fuzz.rs index dfb508d..a3f825d 100644 --- a/rivet-core/tests/sexpr_fuzz.rs +++ b/rivet-core/tests/sexpr_fuzz.rs @@ -225,6 +225,17 @@ fn expr_to_sexpr(e: &Expr) -> String { format!("(exists {} {})", expr_to_sexpr(scope), expr_to_sexpr(pred)) } Expr::Count(scope) => format!("(count {})", expr_to_sexpr(scope)), + Expr::CountCompare(scope, op, n) => { + let op_s = match op { + sexpr_eval::CompOp::Gt => ">", + sexpr_eval::CompOp::Lt => "<", + sexpr_eval::CompOp::Ge => ">=", + sexpr_eval::CompOp::Le => "<=", + sexpr_eval::CompOp::Eq => "=", + sexpr_eval::CompOp::Ne => "!=", + }; + format!("({op_s} (count {}) {n})", expr_to_sexpr(scope)) + } Expr::ReachableFrom(start, lt) => format!( "(reachable-from {} {})", value_to_sexpr(start), diff --git a/rivet-core/tests/sexpr_predicate_matrix.rs b/rivet-core/tests/sexpr_predicate_matrix.rs index 8dc0693..a596810 100644 --- a/rivet-core/tests/sexpr_predicate_matrix.rs +++ b/rivet-core/tests/sexpr_predicate_matrix.rs @@ -292,9 +292,19 @@ fn matches_no_match_for_non_matching_regex() { } #[test] -fn matches_invalid_regex_returns_false_safely() { - // Malformed regex — evaluator returns false instead of panicking. - assert!(!ok(r#"(matches id "[")"#, &base_artifact())); +fn matches_invalid_regex_is_parse_error() { + // v0.4.3: malformed regex patterns are rejected at lower time with + // a clear error rather than silently matching nothing at runtime. + // Previously this returned false safely; audit flagged that users + // mistake silent empty-match for "filter excluded everything" and + // waste debugging time. `err()` here exercises the lower path and + // asserts the diagnostic names the regex compile failure. + let errs = err(r#"(matches id "[")"#); + assert!( + errs.iter() + .any(|e| e.message.to_lowercase().contains("regex")), + "invalid regex must produce a parse error mentioning 'regex': got {errs:?}" + ); } #[test]