diff --git a/AGENTS.md b/AGENTS.md index 0c1ebaf..7f1fd42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,7 @@ Current scaffold policy: - `templates/scalar/generic` is the embedded neutral scaffold used by `solverforge new`; `templates/list/generic` is not a public `new` selector - the neutral `solver.toml` declares no `[[phases]]`, so a fresh app solves through the runtime's model-aware default pipeline; `generate scalar-group` refuses a user-authored scalar construction phase that reaches its assignment-owned target - `generate`, `destroy`, and `new` rustfmt every Rust file they write (rustfmt is optional; a missing one leaves files as generated), fresh scaffolds pass `cargo fmt --check`, and managed constraint `mod` lines stay in rustfmt's sorted order so `cargo fmt` never breaks `check` +- `mod.rs` and `lib.rs` files, in this repo and in every scaffold, hold only module declarations and re-exports (plus the domain `planning_model!` manifest); constraint assembly lives in `src/constraints/assemble.rs`, and `solverforge check` rejects code in any module root - `solverforge check` is structural and says it did not compile the project; generated constraint skeletons compile but their placeholders panic with the function and file to edit, so a solve that reaches one ends `FAILED` - generated constraint analysis DTOs carry `weightMultiplier`; selector telemetry DTOs carry no `conflictRepair*` counters (the run-level telemetry keeps them) - generated `Cargo.toml` files must carry `rust-version = "1.95"` plus the current explicit dependency baselines for the selected shell from the template/materialization code diff --git a/WIREFRAME.md b/WIREFRAME.md index e94bea1..32c7f3b 100644 --- a/WIREFRAME.md +++ b/WIREFRAME.md @@ -195,7 +195,11 @@ Required managed surfaces: - solution files: `solution-imports`, `solution-collections`, `solution-constructor-params`, and `solution-constructor-init` - entity files: `entity-variables` and `entity-variable-init` -- `src/constraints/mod.rs`: `constraint-modules` and `constraint-calls` +- `src/constraints/mod.rs`: the `constraint-modules` block, `mod assemble;`, and + `pub use assemble::create_constraints;`, with no code +- `src/constraints/assemble.rs`: `create_constraints` and the + `constraint-calls` block, which calls each module as + `super::::constraint()` - `solver.toml`: one generated model-resource region marked with `# @solverforge:begin solver-config` and `# @solverforge:end solver-config`; each generated phase has an exact-ID @@ -204,6 +208,11 @@ Required managed surfaces: Commands that add or remove generated resources should fail clearly when those current markers are missing or duplicated. +Every `mod.rs` and `lib.rs` under `src/` holds only module declarations, `use` +items, and (in `src/domain/mod.rs`) the `planning_model!` manifest. +`solverforge check` rejects any function, type, impl, constant, or inline module +body in those files. + `constraint-modules` holds `mod` lines in rustfmt's sorted order and matches them to calls by name; `constraint-calls` keeps insertion order because it is the constraint set's evaluation order. `generate`, `destroy`, and `new` pipe diff --git a/skills/solverforge-modeling/SKILL.md b/skills/solverforge-modeling/SKILL.md index 9814a27..9001c1b 100644 --- a/skills/solverforge-modeling/SKILL.md +++ b/skills/solverforge-modeling/SKILL.md @@ -34,7 +34,7 @@ Re-derive specifics from the CLI if the version differs. Decide this before anything else; it changes which parts of this skill apply. - **Scaffolded project** — produced by `solverforge new`, so the CLI owns the - model through `src/domain/mod.rs`, `src/constraints/mod.rs`, managed blocks, + model through `src/domain/mod.rs`, `src/constraints/{mod,assemble}.rs`, managed blocks, `solverforge.app.toml`, and `solver.toml`. Follow Steps 0–8 below. - **Existing model** — a hand-written app that already depends on `solverforge` directly (the @@ -397,7 +397,7 @@ small conventional commits if the user asks for commits. - Explain a plan with framework analysis (`analyze()`, `analyze_snapshot`, `evaluate_detailed`), not by re-deriving constraint predicates in application code. -- Never hand-wire managed blocks, `src/constraints/mod.rs`, or the solution's +- Never hand-wire managed blocks, `src/constraints/{mod,assemble}.rs`, or the solution's collections. Use the CLI commands. (Scaffolded projects only; an existing hand-written model is edited as ordinary Rust — see `references/existing-model.md`.) diff --git a/skills/solverforge-modeling/references/constraint-patterns.md b/skills/solverforge-modeling/references/constraint-patterns.md index 7bc3013..479780c 100644 --- a/skills/solverforge-modeling/references/constraint-patterns.md +++ b/skills/solverforge-modeling/references/constraint-patterns.md @@ -1,8 +1,8 @@ # Constraint patterns Every `solverforge generate constraint -- [--hard|--soft]` writes -`src/constraints/.rs` and wires it into `src/constraints/mod.rs` (managed -blocks) and, for web shells, `static/sf-config.json`. Do not wire constraints by +`src/constraints/.rs` and wires it into `src/constraints/mod.rs` and +`src/constraints/assemble.rs` (managed blocks) and, for web shells, `static/sf-config.json`. Do not wire constraints by hand. The generated file is a **stub**: a real `ConstraintFactory` call streaming from diff --git a/skills/solverforge-modeling/references/verification.md b/skills/solverforge-modeling/references/verification.md index 200ffcc..cfff315 100644 --- a/skills/solverforge-modeling/references/verification.md +++ b/skills/solverforge-modeling/references/verification.md @@ -6,7 +6,8 @@ compilation, and real runtime failures. ## 1. `solverforge check` — structural -Validates `src/domain/mod.rs`, `src/constraints/mod.rs`, the domain model, +Validates `src/domain/mod.rs`, `src/constraints/mod.rs`, +`src/constraints/assemble.rs`, that no `mod.rs` or `lib.rs` holds code, the domain model, solution, entities, model resources, and `solver.toml` refs. It does **not** compile Rust and does **not** run the solver, so it cannot see placeholder `panic!`s or a broken stream source. Exit code 0 means the managed structure is diff --git a/src/commands/check.rs b/src/commands/check.rs index 21df326..3d1628f 100644 --- a/src/commands/check.rs +++ b/src/commands/check.rs @@ -1,11 +1,15 @@ use std::fs; use std::path::Path; -use crate::commands::generate_constraint::{parse_domain, validate_constraint_mod_source}; +use crate::commands::generate_constraint::{ + parse_domain, validate_constraint_mod_source, ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, + CONSTRAINTS_MOD_PATH, +}; use crate::commands::generate_domain::{find_file_for_type, validate_domain_mod_source}; use crate::error::{CliError, CliResult}; use crate::managed_block; use crate::model_contract::{ContractSeverity, ModelContract}; +use crate::module_manifest; use crate::output; use crate::solver_config; @@ -55,29 +59,38 @@ pub fn run() -> CliResult { let constraints_dir = Path::new("src/constraints"); if constraints_dir.exists() { - output::print_status("check", "src/constraints/mod.rs"); - let mod_path = constraints_dir.join("mod.rs"); - let constraints_mod_src = read_path(&mod_path, &mut errors); - if let Some(src) = constraints_mod_src.as_deref() { - if validate_managed_source(&mod_path, src, &mut errors, validate_constraint_mod_source) - { - if let Ok(module_names) = validate_constraint_mod_source(src) { + output::print_status("check", CONSTRAINTS_MOD_PATH); + output::print_status("check", CONSTRAINTS_ASSEMBLE_PATH); + let module = read_path(Path::new(CONSTRAINTS_MOD_PATH), &mut errors); + let assemble = read_path(Path::new(CONSTRAINTS_ASSEMBLE_PATH), &mut errors); + if let (Some(module), Some(assemble)) = (module, assemble) { + match validate_constraint_mod_source(&ConstraintSources { module, assemble }) { + Ok(module_names) => { for mod_name in module_names { let file = constraints_dir.join(format!("{mod_name}.rs")); if !file.exists() { errors.push(format!( - "Constraint module '{}' declared in src/constraints/mod.rs but file not found", - mod_name + "Constraint module '{mod_name}' declared in {CONSTRAINTS_MOD_PATH} but file not found" )); } } } + Err(err) => errors.push(err), } } } else { warnings.push("src/constraints/ directory not found".to_string()); } + output::print_status("check", "src/**/{mod,lib}.rs"); + for root in module_manifest::module_roots(Path::new("src")) { + if let Some(src) = read_path(&root, &mut errors) { + if let Err(err) = module_manifest::ensure_declarations_only(&src) { + errors.push(format!("{}: {err}", output::display_path(&root))); + } + } + } + for (path, required_blocks) in [ (ENTITY_TEMPLATE_PATH, managed_block::ENTITY_REQUIRED_BLOCKS), ( diff --git a/src/commands/check_tests.rs b/src/commands/check_tests.rs index a58bbaa..6e6fe3c 100644 --- a/src/commands/check_tests.rs +++ b/src/commands/check_tests.rs @@ -100,18 +100,14 @@ impl Plan { .expect("failed to corrupt plan"); fs::write( "src/constraints/mod.rs", - r#"pub use self::assemble::create_constraints; - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - () - } -} -"#, + "mod assemble;\n\npub use assemble::create_constraints;\n", ) .expect("failed to corrupt constraints mod"); + fs::write( + "src/constraints/assemble.rs", + "pub fn create_constraints() -> impl ConstraintSet {\n ()\n}\n", + ) + .expect("failed to corrupt constraint assembly"); let err = run().expect_err("malformed managed files should fail check"); let err = err.to_string(); @@ -171,29 +167,7 @@ pub use plan::Plan; "#, ) .expect("failed to rewrite domain mod"); - fs::write( - "src/constraints/mod.rs", - r#"use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod coverage_gap; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - () - // @solverforge:end constraint-calls - } -} -"#, - ) - .expect("failed to rewrite constraints mod"); + test_support::write_constraint_sources("mod coverage_gap;\n", " ()\n"); fs::write( "src/constraints/coverage_gap.rs", "pub fn constraint() {}\n", @@ -318,6 +292,29 @@ impl Task { ); } +#[test] +fn check_rejects_code_in_module_roots() { + let _cwd_guard = test_support::lock_cwd(); + let tmp = tempfile::tempdir().expect("failed to create temp dir"); + let _dir_guard = CwdGuard::enter(tmp.path()); + write_project_layout(); + run().expect("the generated layout holds no code in module roots"); + + fs::create_dir_all("src/api").expect("failed to create api dir"); + fs::write( + "src/api/mod.rs", + "mod routes;\n\npub fn router() -> usize {\n 1\n}\n", + ) + .expect("failed to write api mod"); + + let err = run().expect_err("a function in a mod.rs should fail check"); + let err = err.to_string(); + assert!( + err.contains("1 error(s), 0 warning(s)"), + "unexpected summary: {err}" + ); +} + #[test] fn check_rejects_stale_scalar_group_variable_targets() { let _cwd_guard = test_support::lock_cwd(); @@ -760,29 +757,10 @@ fn check_rejects_constraint_named_value_that_differs_from_module_id() { "#, ) .expect("failed to write constraint"); - fs::write( - "src/constraints/mod.rs", - r#"use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod unary_placeholder; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - unary_placeholder::constraint() - // @solverforge:end constraint-calls - } -} -"#, - ) - .expect("failed to write constraints mod"); + test_support::write_constraint_sources( + "mod unary_placeholder;\n", + " super::unary_placeholder::constraint()\n", + ); let err = run().expect_err("constraint ID drift should fail check"); let err = err.to_string(); @@ -901,27 +879,6 @@ impl Plan { "#, ) .expect("failed to write plan"); - fs::write( - "src/constraints/mod.rs", - r#"use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - () - // @solverforge:end constraint-calls - } -} -"#, - ) - .expect("failed to write constraints mod"); + test_support::write_constraint_sources("", " ()\n"); fs::write("solver.toml", "# test config\n").expect("failed to write solver.toml"); } diff --git a/src/commands/destroy.rs b/src/commands/destroy.rs index 7145d63..c8b8160 100644 --- a/src/commands/destroy.rs +++ b/src/commands/destroy.rs @@ -1,8 +1,11 @@ use std::fs; use std::path::Path; -use crate::commands::generate_constraint::remove_constraint_from_source; use crate::commands::generate_constraint::{domain::DomainModel, parse_domain}; +use crate::commands::generate_constraint::{ + is_constraint_file_stem, remove_constraint_from_source, ConstraintSources, + CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH, +}; use crate::commands::generate_domain::{find_file_for_type, snake_to_pascal}; use crate::commands::generate_domain::{ remove_domain_mod_entry_source, sync_demo_data_module, unwire_collection_from_solution_source, @@ -370,7 +373,7 @@ fn ensure_type_is_unreferenced_in_source( let constraint_id = (file.parent() == Some(constraints_dir)) .then(|| file.file_stem().and_then(|stem| stem.to_str())) .flatten() - .filter(|stem| *stem != "mod"); + .filter(|stem| is_constraint_file_stem(stem)); match constraint_id { Some(id) => constraint_ids.push(id.to_string()), None => other_files.push(slash_path(file)), @@ -562,7 +565,6 @@ pub fn run_variable(field: &str, entity: &str, skip_confirm: bool) -> CliResult pub fn run_constraint(name: &str, skip_confirm: bool) -> CliResult { let constraint_id = ConstraintId::parse(name)?; let file_path = Path::new("src/constraints").join(format!("{}.rs", constraint_id.as_str())); - let mod_path = Path::new("src/constraints/mod.rs"); if !file_path.exists() { return Err(CliError::ResourceNotFound { @@ -576,24 +578,12 @@ pub fn run_constraint(name: &str, skip_confirm: bool) -> CliResult { return Ok(()); } - if !mod_path.exists() { - return Err(CliError::NotInProject { - missing: "src/constraints/mod.rs", - }); - } - - let mod_src = fs::read_to_string(mod_path).map_err(|e| CliError::IoError { - context: "failed to read src/constraints/mod.rs".to_string(), - source: e, - })?; - let new_mod_src = remove_constraint_from_source(&mod_src, constraint_id.as_str()) + let sources = ConstraintSources::read(Path::new("."))?; + let rewritten = remove_constraint_from_source(&sources, constraint_id.as_str()) .map_err(CliError::general)?; model_contract::ensure_constraint_has_no_conflict_repair_refs(constraint_id.as_str())?; - fs::write(mod_path, new_mod_src).map_err(|e| CliError::IoError { - context: "failed to update src/constraints/mod.rs".to_string(), - source: e, - })?; + rewritten.write(Path::new("."))?; fs::remove_file(&file_path).map_err(|e| CliError::IoError { context: format!("failed to delete {}", file_path.display()), source: e, @@ -603,7 +593,8 @@ pub fn run_constraint(name: &str, skip_confirm: bool) -> CliResult { sync_demo_data_module()?; output::print_remove(&file_path.display().to_string()); - output::print_update("src/constraints/mod.rs"); + output::print_update(CONSTRAINTS_MOD_PATH); + output::print_update(CONSTRAINTS_ASSEMBLE_PATH); Ok(()) } diff --git a/src/commands/destroy_tests.rs b/src/commands/destroy_tests.rs index 1d17d99..ce574a2 100644 --- a/src/commands/destroy_tests.rs +++ b/src/commands/destroy_tests.rs @@ -25,58 +25,30 @@ impl Drop for CwdGuard { #[test] fn removes_constraint_from_managed_tuple_entry() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -mod extra; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - all_assigned::constraint(), - extra::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; + let src = test_support::constraint_sources( + "mod all_assigned;\nmod extra;\n", + " (\n super::all_assigned::constraint(),\n super::extra::constraint(),\n )\n", + ); let updated = - remove_constraint_from_source(src, "all_assigned").expect("source should be rewritten"); - assert!(!updated.contains("mod all_assigned;")); - assert!(!updated.contains("all_assigned::constraint(),")); - assert!(updated.contains("extra::constraint(),")); + remove_constraint_from_source(&src, "all_assigned").expect("source should be rewritten"); + assert!(!updated.module.contains("mod all_assigned;")); + assert!(!updated.assemble.contains("all_assigned::constraint()")); + assert!(updated.assemble.contains("super::extra::constraint(),")); } #[test] fn removes_last_constraint_to_empty_tuple() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod extra; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - extra::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; + let src = test_support::constraint_sources( + "mod extra;\n", + " (\n super::extra::constraint(),\n )\n", + ); - let updated = remove_constraint_from_source(src, "extra").expect("source should be rewritten"); - assert!(!updated.contains("mod extra;")); - assert!(updated.contains(" ()")); + let updated = remove_constraint_from_source(&src, "extra").expect("source should be rewritten"); + assert!(!updated.module.contains("mod extra;")); + assert!(updated + .assemble + .contains(" // @solverforge:begin constraint-calls\n ()\n")); } #[test] @@ -661,19 +633,8 @@ fn destroy_constraint_preflights_constraint_mod_before_delete() { let _cwd_guard = test_support::lock_cwd(); let tmp = tempfile::tempdir().expect("failed to create temp dir"); let _dir_guard = CwdGuard::enter(tmp.path()); - fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); - let invalid_mod = r#"pub use self::assemble::create_constraints; - -mod all_assigned; - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - all_assigned::constraint() - } -} -"#; + test_support::write_constraint_sources("", " super::all_assigned::constraint()\n"); + let invalid_mod = "mod all_assigned;\nmod assemble;\n\npub use assemble::create_constraints;\n"; fs::write("src/constraints/mod.rs", invalid_mod).expect("failed to write constraints mod"); fs::write( "src/constraints/all_assigned.rs", @@ -1065,30 +1026,10 @@ impl Plan { } fn write_all_assigned_constraint() { - fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); - fs::write( - "src/constraints/mod.rs", - r#"use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - all_assigned::constraint() - // @solverforge:end constraint-calls - } -} -"#, - ) - .expect("failed to write constraints mod"); + test_support::write_constraint_sources( + "mod all_assigned;\n", + " super::all_assigned::constraint()\n", + ); fs::write( "src/constraints/all_assigned.rs", r#"pub fn constraint() { diff --git a/src/commands/generate_constraint/domain.rs b/src/commands/generate_constraint/domain.rs index 44b7b81..bd48fef 100644 --- a/src/commands/generate_constraint/domain.rs +++ b/src/commands/generate_constraint/domain.rs @@ -749,7 +749,7 @@ pub(crate) fn list_constraints(dir: &Path) -> Vec { let path = entry.path(); if path.extension().and_then(|e| e.to_str()) == Some("rs") { let name = path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); - if name != "mod" { + if super::sources::is_constraint_file_stem(name) { constraints.push(name.to_string()); } } diff --git a/src/commands/generate_constraint/mod.rs b/src/commands/generate_constraint/mod.rs index 1cf8b29..6954f28 100644 --- a/src/commands/generate_constraint/mod.rs +++ b/src/commands/generate_constraint/mod.rs @@ -2,6 +2,7 @@ pub(crate) mod domain; mod mod_rewriter; mod run; mod skeleton; +pub(crate) mod sources; mod utils; mod wizard; @@ -11,4 +12,7 @@ mod tests; pub(crate) use domain::parse_domain; pub(crate) use mod_rewriter::{remove_constraint_from_source, validate_constraint_mod_source}; pub(crate) use run::run; +pub(crate) use sources::{ + is_constraint_file_stem, ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH, +}; pub(crate) use utils::validate_name; diff --git a/src/commands/generate_constraint/mod_rewriter.rs b/src/commands/generate_constraint/mod_rewriter.rs index 760b3be..8f5d29a 100644 --- a/src/commands/generate_constraint/mod_rewriter.rs +++ b/src/commands/generate_constraint/mod_rewriter.rs @@ -1,6 +1,8 @@ use std::collections::BTreeSet; +use super::sources::{ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH}; use crate::managed_block; +use crate::module_manifest; const CONSTRAINT_MODULES_BLOCK: &str = "constraint-modules"; const CONSTRAINT_CALLS_BLOCK: &str = "constraint-calls"; @@ -18,48 +20,38 @@ struct ConstraintSurface { calls: Vec, } -pub(crate) fn rewrite_mod(src: &str, name: &str) -> Result { - let mut surface = ConstraintSurface::parse(src)?; +/// Declares constraint module `name` and appends its call to the constraint +/// set, leaving an already-wired constraint untouched. +pub(crate) fn rewrite_mod( + sources: &ConstraintSources, + name: &str, +) -> Result { + let mut surface = ConstraintSurface::parse(sources)?; if !surface.modules.iter().any(|existing| existing == name) { surface.modules.push(name.to_string()); } - if !surface.calls.iter().any(|existing| existing == name) { surface.calls.push(name.to_string()); } - - let src = managed_block::replace_block( - src, - CONSTRAINT_MODULES_BLOCK, - &render_constraint_modules(&surface.modules), - )?; - managed_block::replace_block( - &src, - CONSTRAINT_CALLS_BLOCK, - &render_constraint_calls(&surface.calls), - ) + surface.render(sources) } -pub(crate) fn remove_constraint_from_source(src: &str, name: &str) -> Result { - let mut surface = ConstraintSurface::parse(src)?; +/// Removes constraint module `name` and its call from the constraint set. +pub(crate) fn remove_constraint_from_source( + sources: &ConstraintSources, + name: &str, +) -> Result { + let mut surface = ConstraintSurface::parse(sources)?; surface.modules.retain(|existing| existing != name); - surface.calls.retain(|existing| existing != name); - - let src = managed_block::replace_block( - src, - CONSTRAINT_MODULES_BLOCK, - &render_constraint_modules(&surface.modules), - )?; - managed_block::replace_block( - &src, - CONSTRAINT_CALLS_BLOCK, - &render_constraint_calls(&surface.calls), - ) + surface.render(sources) } -pub(crate) fn validate_constraint_mod_source(src: &str) -> Result, String> { - Ok(ConstraintSurface::parse(src)?.modules) +/// Validates the constraint wiring and returns the declared module names. +pub(crate) fn validate_constraint_mod_source( + sources: &ConstraintSources, +) -> Result, String> { + Ok(ConstraintSurface::parse(sources)?.modules) } fn parse_constraint_modules(src: &str) -> Result, String> { @@ -119,7 +111,7 @@ fn render_constraint_modules(modules: &[String]) -> String { fn render_constraint_calls(calls: &[String]) -> String { if calls.is_empty() { - return " ()".to_string(); + return " ()".to_string(); } let tree = build_constraint_tree( @@ -129,7 +121,7 @@ fn render_constraint_calls(calls: &[String]) -> String { .map(ConstraintExpr::Call) .collect::>(), ); - render_constraint_expr(&tree, 8, false).join("\n") + render_constraint_expr(&tree, 4, false).join("\n") } fn build_constraint_tree(exprs: &[ConstraintExpr]) -> ConstraintExpr { @@ -156,7 +148,7 @@ fn render_constraint_expr( let pad = " ".repeat(indent); match expr { ConstraintExpr::Call(module) => vec![format!( - "{pad}{module}::constraint(){}", + "{pad}super::{module}::constraint(){}", if as_tuple_item { "," } else { "" } )], ConstraintExpr::Tuple(items) => { @@ -182,9 +174,12 @@ fn is_simple_ident(value: &str) -> bool { } impl ConstraintSurface { - fn parse(src: &str) -> Result { - let modules = parse_constraint_modules(src)?; - let calls = parse_constraint_calls(src)?; + fn parse(sources: &ConstraintSources) -> Result { + let in_mod = |err: String| format!("{CONSTRAINTS_MOD_PATH}: {err}"); + module_manifest::ensure_declarations_only(&sources.module).map_err(in_mod)?; + let modules = parse_constraint_modules(&sources.module).map_err(in_mod)?; + let calls = parse_constraint_calls(&sources.assemble) + .map_err(|err| format!("{CONSTRAINTS_ASSEMBLE_PATH}: {err}"))?; for module in &modules { if !calls.iter().any(|call| call == module) { @@ -208,6 +203,21 @@ impl ConstraintSurface { Ok(Self { modules, calls }) } + + fn render(&self, sources: &ConstraintSources) -> Result { + Ok(ConstraintSources { + module: managed_block::replace_block( + &sources.module, + CONSTRAINT_MODULES_BLOCK, + &render_constraint_modules(&self.modules), + )?, + assemble: managed_block::replace_block( + &sources.assemble, + CONSTRAINT_CALLS_BLOCK, + &render_constraint_calls(&self.calls), + )?, + }) + } } struct ConstraintCallParser<'a> { @@ -285,6 +295,7 @@ impl<'a> ConstraintCallParser<'a> { } fn parse_call(&mut self) -> Result { + self.expect_str("super::")?; let module = self.parse_ident()?; self.expect_str("::constraint")?; self.skip_ws(); diff --git a/src/commands/generate_constraint/run.rs b/src/commands/generate_constraint/run.rs index 6af8ea4..bbb867e 100644 --- a/src/commands/generate_constraint/run.rs +++ b/src/commands/generate_constraint/run.rs @@ -4,6 +4,7 @@ use std::path::Path; use super::domain::parse_domain; use super::mod_rewriter::rewrite_mod; use super::skeleton::generate_skeleton; +use super::sources::{ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH}; use super::utils::validate_name; use super::wizard::{resolve_pattern_and_hardness, PatternFlags}; use crate::app_spec; @@ -44,7 +45,6 @@ pub fn run( validate_name(name)?; let constraints_dir = Path::new("src/constraints"); - let mod_path = constraints_dir.join("mod.rs"); let new_file = constraints_dir.join(format!("{}.rs", name)); if !constraints_dir.exists() { @@ -52,11 +52,6 @@ pub fn run( missing: "src/constraints/", }); } - if !mod_path.exists() { - return Err(CliError::NotInProject { - missing: "src/constraints/mod.rs", - }); - } if new_file.exists() && !force { return Err(CliError::ResourceExists { kind: "constraint", @@ -64,10 +59,7 @@ pub fn run( }); } - let mod_src = fs::read_to_string(&mod_path).map_err(|e| CliError::IoError { - context: "failed to read src/constraints/mod.rs".to_string(), - source: e, - })?; + let sources = ConstraintSources::read(Path::new("."))?; let domain = parse_domain().map_err(CliError::general)?; let solution_type = domain.solution_type.clone(); @@ -159,11 +151,12 @@ pub fn run( Some(&domain), ); - let new_mod = rewrite_mod(&mod_src, name).map_err(CliError::general)?; + let rewritten = rewrite_mod(&sources, name).map_err(CliError::general)?; if pretend { println!("Would create src/constraints/{}.rs", name); - println!("Would update src/constraints/mod.rs"); + println!("Would update {CONSTRAINTS_MOD_PATH}"); + println!("Would update {CONSTRAINTS_ASSEMBLE_PATH}"); return Ok(()); } @@ -172,18 +165,17 @@ pub fn run( source: e, })?; - fs::write(&mod_path, &new_mod).map_err(|e| CliError::IoError { - context: "failed to write src/constraints/mod.rs".to_string(), - source: e, - })?; + rewritten.write(Path::new("."))?; crate::commands::sf_config::add_constraint(name)?; app_spec::sync_from_project()?; output::print_create(&format!("src/constraints/{}.rs", name)); print_diff_verbose("", &skeleton); - output::print_update("src/constraints/mod.rs"); - print_diff_verbose(&mod_src, &new_mod); + output::print_update(CONSTRAINTS_MOD_PATH); + print_diff_verbose(&sources.module, &rewritten.module); + output::print_update(CONSTRAINTS_ASSEMBLE_PATH); + print_diff_verbose(&sources.assemble, &rewritten.assemble); println!(); if !output::is_quiet() { println!(" Next steps:"); diff --git a/src/commands/generate_constraint/sources.rs b/src/commands/generate_constraint/sources.rs new file mode 100644 index 0000000..ad90a7e --- /dev/null +++ b/src/commands/generate_constraint/sources.rs @@ -0,0 +1,101 @@ +//! The two files that hold a project's managed constraint wiring. +//! +//! `src/constraints/mod.rs` declares the constraint modules inside the +//! `constraint-modules` block and re-exports `create_constraints`; it holds no +//! code. `src/constraints/assemble.rs` defines `create_constraints` and owns +//! the `constraint-calls` block that lists every constraint in evaluation +//! order. + +use std::fs; +use std::path::Path; + +use crate::error::{CliError, CliResult}; + +/// Project-relative path of the constraint module manifest. +pub(crate) const CONSTRAINTS_MOD_PATH: &str = "src/constraints/mod.rs"; +/// Project-relative path of the constraint set assembly. +pub(crate) const CONSTRAINTS_ASSEMBLE_PATH: &str = "src/constraints/assemble.rs"; + +/// Whether `stem` names a constraint module file in `src/constraints/`, as +/// opposed to the module manifest (`mod.rs`) or the assembly (`assemble.rs`). +pub(crate) fn is_constraint_file_stem(stem: &str) -> bool { + stem != "mod" && stem != "assemble" +} + +/// The constraint module manifest and the constraint set assembly, as text. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ConstraintSources { + /// Contents of `src/constraints/mod.rs`. + pub(crate) module: String, + /// Contents of `src/constraints/assemble.rs`. + pub(crate) assemble: String, +} + +impl ConstraintSources { + /// Reads both files relative to `root`. + pub(crate) fn read(root: &Path) -> CliResult { + Ok(Self { + module: read(root, CONSTRAINTS_MOD_PATH)?, + assemble: read(root, CONSTRAINTS_ASSEMBLE_PATH)?, + }) + } + + /// Writes both files relative to `root`. + pub(crate) fn write(&self, root: &Path) -> CliResult { + write(root, CONSTRAINTS_MOD_PATH, &self.module)?; + write(root, CONSTRAINTS_ASSEMBLE_PATH, &self.assemble) + } + + /// Sources of an empty constraint set for `solution_type` scored by + /// `score_type`. + pub(crate) fn empty(solution_type: &str, score_type: &str) -> Self { + Self { + module: EMPTY_CONSTRAINTS_MOD.to_string(), + assemble: format!( + r#"//! Assembles the project's constraint set. + +use crate::domain::{solution_type}; +use solverforge::prelude::*; + +/// Builds the constraint set the solver scores every solution against. +pub fn create_constraints() -> impl ConstraintSet<{solution_type}, {score_type}> {{ + // @solverforge:begin constraint-calls + () + // @solverforge:end constraint-calls +}} +"# + ), + } + } +} + +const EMPTY_CONSTRAINTS_MOD: &str = r#"//! Constraint definitions. +//! +//! Add constraint modules with `solverforge generate constraint ...`. +//! The neutral shell starts with an empty constraint set. + +// @solverforge:begin constraint-modules +// @solverforge:end constraint-modules + +mod assemble; + +pub use assemble::create_constraints; +"#; + +fn read(root: &Path, relative: &'static str) -> CliResult { + let path = root.join(relative); + if !path.exists() { + return Err(CliError::NotInProject { missing: relative }); + } + fs::read_to_string(&path).map_err(|source| CliError::IoError { + context: format!("failed to read {relative}"), + source, + }) +} + +fn write(root: &Path, relative: &str, contents: &str) -> CliResult { + fs::write(root.join(relative), contents).map_err(|source| CliError::IoError { + context: format!("failed to write {relative}"), + source, + }) +} diff --git a/src/commands/generate_constraint/tests.rs b/src/commands/generate_constraint/tests.rs index f6e00fc..244b76e 100644 --- a/src/commands/generate_constraint/tests.rs +++ b/src/commands/generate_constraint/tests.rs @@ -3,6 +3,7 @@ use super::{ mod_rewriter::{remove_constraint_from_source, rewrite_mod, validate_constraint_mod_source}, run::run as run_generate_constraint, skeleton::{generate_skeleton, Pattern}, + sources::ConstraintSources, utils::validate_name, }; use crate::scalar_variable_hooks::ScalarVariableHooks; @@ -41,59 +42,32 @@ fn test_validate_name() { assert!(validate_name("").is_err()); } +use crate::test_support::constraint_sources as sources; + #[test] fn test_rewrite_mod_appends_module_and_call_to_managed_blocks() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - all_assigned::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; - let result = rewrite_mod(src, "max_hours").expect("rewrite should succeed"); - assert!(result.contains("mod max_hours;")); - assert!(result.contains("max_hours::constraint(),")); + let src = sources( + "mod all_assigned;\n", + " (\n super::all_assigned::constraint(),\n )\n", + ); + let result = rewrite_mod(&src, "max_hours").expect("rewrite should succeed"); + assert!(result.module.contains("mod max_hours;")); + assert!(result.assemble.contains("super::max_hours::constraint(),")); + assert!(!result.module.contains("constraint()")); } /// `cargo fmt` sorts consecutive `mod` items alphabetically, while the /// constraint calls keep their insertion order. Both shapes must stay valid. -const RUSTFMT_SORTED_CONSTRAINTS_MOD: &str = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -mod max_hours; -mod zone_cover; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - zone_cover::constraint(), - all_assigned::constraint(), - max_hours::constraint(), - ) - // @solverforge:end constraint-calls - } +fn rustfmt_sorted_sources() -> ConstraintSources { + sources( + "mod all_assigned;\nmod max_hours;\nmod zone_cover;\n", + " (\n super::zone_cover::constraint(),\n super::all_assigned::constraint(),\n super::max_hours::constraint(),\n )\n", + ) } -"#; #[test] fn test_validate_constraint_mod_source_accepts_rustfmt_sorted_modules() { - let modules = validate_constraint_mod_source(RUSTFMT_SORTED_CONSTRAINTS_MOD) + let modules = validate_constraint_mod_source(&rustfmt_sorted_sources()) .expect("rustfmt-sorted module list should validate"); assert_eq!(modules, ["all_assigned", "max_hours", "zone_cover"]); } @@ -101,142 +75,84 @@ fn test_validate_constraint_mod_source_accepts_rustfmt_sorted_modules() { #[test] fn test_rewrite_mod_keeps_modules_sorted_and_calls_in_insertion_order() { let result = - rewrite_mod(RUSTFMT_SORTED_CONSTRAINTS_MOD, "capacity").expect("rewrite should succeed"); + rewrite_mod(&rustfmt_sorted_sources(), "capacity").expect("rewrite should succeed"); assert!( - result.contains( + result.module.contains( "// @solverforge:begin constraint-modules\nmod all_assigned;\nmod capacity;\nmod max_hours;\nmod zone_cover;\n// @solverforge:end constraint-modules" ), - "modules must stay in rustfmt's sorted order:\n{result}" + "modules must stay in rustfmt's sorted order:\n{}", + result.module ); assert!( - result.contains( - " zone_cover::constraint(),\n all_assigned::constraint(),\n max_hours::constraint(),\n capacity::constraint(),\n" + result.assemble.contains( + " super::zone_cover::constraint(),\n super::all_assigned::constraint(),\n super::max_hours::constraint(),\n super::capacity::constraint(),\n" ), - "calls must keep insertion order with the new call appended:\n{result}" + "calls must keep insertion order with the new call appended:\n{}", + result.assemble ); let removed = remove_constraint_from_source(&result, "all_assigned").expect("remove should succeed"); - assert!(removed.contains("mod capacity;\nmod max_hours;\nmod zone_cover;\n")); - assert!(removed.contains( - " zone_cover::constraint(),\n max_hours::constraint(),\n capacity::constraint(),\n" + assert!(removed + .module + .contains("mod capacity;\nmod max_hours;\nmod zone_cover;\n")); + assert!(removed.assemble.contains( + " super::zone_cover::constraint(),\n super::max_hours::constraint(),\n super::capacity::constraint(),\n" )); } #[test] fn test_remove_constraint_from_source_preserves_empty_tuple_shape() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - all_assigned::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; - let result = remove_constraint_from_source(src, "all_assigned").expect("remove should succeed"); - assert!(!result.contains("mod all_assigned;")); - assert!(result.contains(" ()")); + let src = sources( + "mod all_assigned;\n", + " (\n super::all_assigned::constraint(),\n )\n", + ); + let result = + remove_constraint_from_source(&src, "all_assigned").expect("remove should succeed"); + assert!(!result.module.contains("mod all_assigned;")); + assert!(result + .assemble + .contains(" // @solverforge:begin constraint-calls\n ()\n")); } #[test] fn test_rewrite_mod_flattens_nested_constraint_tuples() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -mod extra; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; + let src = sources( + "mod all_assigned;\nmod extra;\n", + " (\n (\n super::all_assigned::constraint(),\n super::extra::constraint(),\n ),\n )\n", + ); - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - ( - all_assigned::constraint(), - extra::constraint(), - ), - ) - // @solverforge:end constraint-calls + let result = rewrite_mod(&src, "capacity").expect("rewrite should succeed"); + assert!(result.module.contains("mod capacity;")); + for call in ["capacity", "all_assigned", "extra"] { + assert!(result + .assemble + .contains(&format!("super::{call}::constraint(),"))); } } -"#; - - let result = rewrite_mod(src, "capacity").expect("rewrite should succeed"); - assert!(result.contains("mod capacity;")); - assert!(result.contains("capacity::constraint(),")); - assert!(result.contains("all_assigned::constraint(),")); - assert!(result.contains("extra::constraint(),")); -} #[test] fn test_remove_constraint_from_source_handles_nested_tuple_calls() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod one; -mod two; -mod three; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - ( - one::constraint(), - two::constraint(), - ), - three::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; + let src = sources( + "mod one;\nmod two;\nmod three;\n", + " (\n (\n super::one::constraint(),\n super::two::constraint(),\n ),\n super::three::constraint(),\n )\n", + ); - let result = remove_constraint_from_source(src, "two").expect("remove should succeed"); - assert!(!result.contains("mod two;")); - assert!(!result.contains("two::constraint(),")); - assert!(result.contains("one::constraint(),")); - assert!(result.contains("three::constraint(),")); + let result = remove_constraint_from_source(&src, "two").expect("remove should succeed"); + assert!(!result.module.contains("mod two;")); + assert!(!result.assemble.contains("two::constraint()")); + assert!(result.assemble.contains("super::one::constraint(),")); + assert!(result.assemble.contains("super::three::constraint(),")); } #[test] fn test_rewrite_mod_requires_every_declared_constraint_to_be_wired() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -mod max_hours; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - all_assigned::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; + let src = sources( + "mod all_assigned;\nmod max_hours;\n", + " (\n super::all_assigned::constraint(),\n )\n", + ); - let err = rewrite_mod(src, "capacity").expect_err("missing constraint wiring should fail"); + let err = rewrite_mod(&src, "capacity").expect_err("missing constraint wiring should fail"); assert_eq!( err, @@ -246,28 +162,13 @@ mod assemble { #[test] fn test_validate_constraint_mod_source_rejects_undeclared_call() { - let src = r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -// @solverforge:end constraint-modules - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - ( - all_assigned::constraint(), - extra::constraint(), - ) - // @solverforge:end constraint-calls - } -} -"#; + let src = sources( + "mod all_assigned;\n", + " (\n super::all_assigned::constraint(),\n super::extra::constraint(),\n )\n", + ); let err = - validate_constraint_mod_source(src).expect_err("undeclared constraint call should fail"); + validate_constraint_mod_source(&src).expect_err("undeclared constraint call should fail"); assert_eq!( err, @@ -275,6 +176,33 @@ mod assemble { ); } +#[test] +fn test_validate_constraint_mod_source_rejects_code_in_mod_rs() { + let mut src = sources( + "mod all_assigned;\n", + " super::all_assigned::constraint()\n", + ); + src.module + .push_str("\npub fn create_extra() -> usize {\n 1\n}\n"); + + let err = validate_constraint_mod_source(&src).expect_err("code in mod.rs should fail"); + + assert!(err.contains("src/constraints/mod.rs"), "{err}"); + assert!(err.contains("fn `create_extra`"), "{err}"); +} + +#[test] +fn test_validate_constraint_mod_source_rejects_calls_without_super_prefix() { + let src = sources( + "mod all_assigned;\n", + " (\n all_assigned::constraint(),\n )\n", + ); + + let err = validate_constraint_mod_source(&src).expect_err("calls must name super::"); + + assert!(err.contains("unsupported constraint call"), "{err}"); +} + #[test] fn test_run_preflights_managed_constraint_mod_before_creating_file() { let _cwd_guard = test_support::lock_cwd(); @@ -282,17 +210,12 @@ fn test_run_preflights_managed_constraint_mod_before_creating_file() { let _dir_guard = CwdGuard::enter(tmp.path()); write_minimal_domain_project(); fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); - let invalid_mod = r#"pub use self::assemble::create_constraints; - -mod assemble { - use super::*; - - pub fn create_constraints() -> impl ConstraintSet { - () - } -} -"#; - fs::write("src/constraints/mod.rs", invalid_mod).expect("failed to write constraints mod"); + let invalid_mod = "mod assemble;\n\npub use assemble::create_constraints;\n"; + let mut sources = ConstraintSources::empty("Plan", "HardSoftScore"); + sources.module = invalid_mod.to_string(); + sources + .write(Path::new(".")) + .expect("failed to write constraint sources"); let err = run_generate_constraint( "capacity", false, true, false, false, false, false, false, false, false, false, false, @@ -449,28 +372,9 @@ fn rewrite_plan_score(score_type: &str) { fn write_minimal_constraints_mod(score_type: &str) { fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); - fs::write( - "src/constraints/mod.rs", - format!( - r#"pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -// @solverforge:end constraint-modules - -mod assemble {{ - use solverforge::prelude::*; - use crate::domain::Plan; - - pub fn create_constraints() -> impl ConstraintSet {{ - // @solverforge:begin constraint-calls - () - // @solverforge:end constraint-calls - }} -}} -"# - ), - ) - .expect("failed to write constraints mod"); + ConstraintSources::empty("Plan", score_type) + .write(Path::new(".")) + .expect("failed to write constraint sources"); } #[test] diff --git a/src/commands/generate_domain/run.rs b/src/commands/generate_domain/run.rs index c4c4014..cb32bc9 100644 --- a/src/commands/generate_domain/run.rs +++ b/src/commands/generate_domain/run.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use crate::app_spec; use crate::commands::generate_constraint::validate_name; use crate::commands::generate_constraint::{domain::DomainModel, parse_domain}; +use crate::commands::generate_constraint::{ConstraintSources, CONSTRAINTS_MOD_PATH}; use crate::error::{CliError, CliResult}; use crate::list_variable_metadata::ListVariableMetadata; use crate::output; @@ -44,7 +45,7 @@ struct CollectionRewritePlan { struct NeutralReplacementPlan { neutral_plan_path: PathBuf, - rewritten_constraints_mod: Option, + rewritten_constraints_mod: Option, contract_rewrites: Vec<(PathBuf, String)>, } @@ -668,17 +669,13 @@ fn neutral_domain_exports() -> CliResult { } fn neutral_constraint_blocks() -> CliResult { - let constraints_mod = Path::new("src/constraints/mod.rs"); - if !constraints_mod.exists() { + if !Path::new(CONSTRAINTS_MOD_PATH).exists() { return Ok(false); } - let src = fs::read_to_string(constraints_mod).map_err(|e| CliError::IoError { - context: "failed to read src/constraints/mod.rs".to_string(), - source: e, - })?; - let modules = crate::managed_block::read_block(&src, CONSTRAINT_MODULES_BLOCK) + let sources = ConstraintSources::read(Path::new("."))?; + let modules = crate::managed_block::read_block(&sources.module, CONSTRAINT_MODULES_BLOCK) .map_err(CliError::general)?; - let calls = crate::managed_block::read_block(&src, CONSTRAINT_CALLS_BLOCK) + let calls = crate::managed_block::read_block(&sources.assemble, CONSTRAINT_CALLS_BLOCK) .map_err(CliError::general)?; Ok(modules.trim().is_empty() && calls.trim() == "()") @@ -689,9 +686,8 @@ fn precompute_neutral_replacement( score_type: &str, ) -> CliResult { let neutral_plan_path = Path::new("src/domain").join(format!("{NEUTRAL_SOLUTION_MODULE}.rs")); - let constraints_mod = Path::new("src/constraints/mod.rs"); - let rewritten_constraints_mod = if constraints_mod.exists() { - Some(empty_constraints_mod_source(solution_type, score_type)) + let rewritten_constraints_mod = if Path::new(CONSTRAINTS_MOD_PATH).exists() { + Some(ConstraintSources::empty(solution_type, score_type)) } else { None }; @@ -753,12 +749,7 @@ fn apply_neutral_replacement(plan: NeutralReplacementPlan) -> CliResult { } if let Some(rewritten_constraints_mod) = plan.rewritten_constraints_mod { - fs::write("src/constraints/mod.rs", rewritten_constraints_mod).map_err(|e| { - CliError::IoError { - context: "failed to update constraints/mod.rs".to_string(), - source: e, - } - })?; + rewritten_constraints_mod.write(Path::new("."))?; } for (path, rewritten) in plan.contract_rewrites { @@ -807,34 +798,6 @@ fn empty_domain_mod_source() -> &'static str { "# } -fn empty_constraints_mod_source(solution_type: &str, score_type: &str) -> String { - format!( - r#"/* Constraint definitions. - -Add constraint modules with `solverforge generate constraint ...`. -The neutral shell starts with an empty constraint set. */ - -use crate::domain::{solution_type}; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -// @solverforge:end constraint-modules - -mod assemble {{ - use super::*; - - pub fn create_constraints() -> impl ConstraintSet<{solution_type}, {score_type}> {{ - // @solverforge:begin constraint-calls - () - // @solverforge:end constraint-calls - }} -}} -"# - ) -} - fn sync_project_metadata() -> CliResult { app_spec::sync_from_project().and_then(|_| sync_demo_data_module()) } diff --git a/src/commands/generate_domain/tests.rs b/src/commands/generate_domain/tests.rs index 47320e4..2a27c6f 100644 --- a/src/commands/generate_domain/tests.rs +++ b/src/commands/generate_domain/tests.rs @@ -854,8 +854,11 @@ pub fn custom_source() -> &'static str { let constraints_mod = std::fs::read_to_string(tmp.path().join("src/constraints/mod.rs")) .expect("failed to read rewritten constraints mod"); assert!(constraints_mod.contains("@solverforge:begin constraint-modules")); - assert!(constraints_mod.contains("use crate::domain::Schedule;")); - assert!(constraints_mod.contains("ConstraintSet")); + assert!(constraints_mod.contains("pub use assemble::create_constraints;")); + let assembly = std::fs::read_to_string(tmp.path().join("src/constraints/assemble.rs")) + .expect("failed to read rewritten constraint assembly"); + assert!(assembly.contains("use crate::domain::Schedule;")); + assert!(assembly.contains("ConstraintSet")); let dto = std::fs::read_to_string(tmp.path().join("src/api/dto.rs")) .expect("failed to read rewritten dto"); @@ -964,31 +967,14 @@ impl Plan {{ ), ) .expect("failed to write plan"); - fs::write( - "src/constraints/mod.rs", - format!( - r#"use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; - -// @solverforge:begin constraint-modules -mod all_assigned; -// @solverforge:end constraint-modules - -mod assemble {{ - use super::*; - - pub fn create_constraints() -> impl ConstraintSet {{ - // @solverforge:begin constraint-calls - all_assigned::constraint() - // @solverforge:end constraint-calls - }} -}} -"# - ), - ) - .expect("failed to write constraints mod"); + let mut sources = test_support::constraint_sources( + "mod all_assigned;\n", + " super::all_assigned::constraint()\n", + ); + sources.assemble = sources.assemble.replace("HardSoftScore", score); + sources + .write(std::path::Path::new(".")) + .expect("failed to write constraint sources"); fs::write( "src/constraints/all_assigned.rs", format!( @@ -1064,7 +1050,7 @@ fn generate_score_rewrites_constraints_and_contract_files() { score_rewrite_assertions( &[ "src/domain/plan.rs", - "src/constraints/mod.rs", + "src/constraints/assemble.rs", "src/constraints/all_assigned.rs", "src/api/dto.rs", "src/solver/service.rs", @@ -1090,12 +1076,11 @@ fn generate_score_is_idempotent_and_failure_safe() { write_score_rewrite_project("HardSoftScore"); super::run_score("HardSoftScore").expect("same-score run should be a no-op"); - let constraints_mod = - fs::read_to_string("src/constraints/mod.rs").expect("failed to read constraints mod"); + let assembly = + fs::read_to_string("src/constraints/assemble.rs").expect("failed to read assembly"); assert!( - constraints_mod.contains("HardSoftScore") - && !constraints_mod.contains("HardMediumSoftScore"), - "same-score run must not mutate constraint files: {constraints_mod}" + assembly.contains("HardSoftScore") && !assembly.contains("HardMediumSoftScore"), + "same-score run must not mutate constraint files: {assembly}" ); fs::write("src/api/dto.rs", "pub struct Broken {").expect("failed to break dto"); @@ -1109,7 +1094,7 @@ fn generate_score_is_idempotent_and_failure_safe() { score_rewrite_assertions( &[ "src/domain/plan.rs", - "src/constraints/mod.rs", + "src/constraints/assemble.rs", "src/constraints/all_assigned.rs", "src/solver/service.rs", "src/lib.rs", @@ -1273,7 +1258,7 @@ fn generate_score_imports_the_bare_name_of_a_generic_score() { // as a hand-edited project would be. for path in [ "src/domain/plan.rs", - "src/constraints/mod.rs", + "src/constraints/assemble.rs", "src/constraints/all_assigned.rs", "src/api/dto.rs", "src/solver/service.rs", diff --git a/src/commands/info.rs b/src/commands/info.rs index 3d842f1..0318141 100644 --- a/src/commands/info.rs +++ b/src/commands/info.rs @@ -1,7 +1,7 @@ use std::fs; use std::path::Path; -use crate::commands::generate_constraint::parse_domain; +use crate::commands::generate_constraint::{domain::list_constraints, parse_domain}; use crate::error::{CliError, CliResult}; use crate::output; @@ -90,20 +90,3 @@ fn read_project_name() -> Option { } None } - -fn list_constraints(dir: &Path) -> Vec { - let mut constraints = Vec::new(); - if let Ok(entries) = fs::read_dir(dir) { - for entry in entries.flatten() { - let path = entry.path(); - if path.extension().and_then(|e| e.to_str()) == Some("rs") { - let name = path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); - if name != "mod" { - constraints.push(name.to_string()); - } - } - } - } - constraints.sort(); - constraints -} diff --git a/src/main.rs b/src/main.rs index ef935aa..a40ca00 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ mod list_variable_metadata; mod managed_block; mod model_contract; mod model_id; +mod module_manifest; mod output; mod rc; mod scaffold_target; diff --git a/src/model_contract.rs b/src/model_contract.rs index df932f8..6bc8925 100644 --- a/src/model_contract.rs +++ b/src/model_contract.rs @@ -6,7 +6,9 @@ use crate::app_spec::{ self, AppSpec, ConflictRepairSpec, ScalarGroupAssignmentHooks, ScalarGroupSpec, }; use crate::commands::generate_constraint::domain::{DomainModel, EntityInfo}; -use crate::commands::generate_constraint::{parse_domain, validate_constraint_mod_source}; +use crate::commands::generate_constraint::{ + parse_domain, validate_constraint_mod_source, ConstraintSources, +}; use crate::error::{CliError, CliResult}; use crate::model_id::ConstraintId; use crate::solver_config::{self, SolverConfigDocument, SolverConfigIndex}; @@ -724,12 +726,8 @@ fn load_constraint_refs(dir: &Path) -> CliResult> { if !mod_path.exists() { return Ok(Vec::new()); } - let mod_src = fs::read_to_string(&mod_path).map_err(|err| CliError::IoError { - context: format!("failed to read {}", mod_path.display()), - source: err, - })?; - let modules = validate_constraint_mod_source(&mod_src) - .map_err(|err| CliError::general(format!("{}: {err}", mod_path.display())))?; + let sources = ConstraintSources::read(Path::new("."))?; + let modules = validate_constraint_mod_source(&sources).map_err(CliError::general)?; let mut refs = Vec::new(); for id in modules { ConstraintId::parse(&id)?; diff --git a/src/module_manifest.rs b/src/module_manifest.rs new file mode 100644 index 0000000..82565f9 --- /dev/null +++ b/src/module_manifest.rs @@ -0,0 +1,90 @@ +//! Enforces that module roots (`mod.rs`, `lib.rs`) hold no code. +//! +//! A module root may declare child modules, re-export or import items, and, +//! for the domain manifest, invoke `solverforge::planning_model!`. Functions, +//! types, impls, constants and inline module bodies belong in their own files. + +use std::fs; +use std::path::{Path, PathBuf}; + +use syn::{Item, Meta}; + +/// Rejects any item in `src` other than a module declaration, a `use`, an +/// `extern crate`, or a `planning_model!` invocation. +pub(crate) fn ensure_declarations_only(src: &str) -> Result<(), String> { + let file = syn::parse_file(src).map_err(|err| format!("failed to parse: {err}"))?; + for attr in &file.attrs { + if !is_documentation(&attr.meta) { + return Err("module roots may not carry inner attributes other than docs".to_string()); + } + } + for item in &file.items { + let allowed = match item { + Item::Mod(module) => module.content.is_none(), + Item::Use(_) | Item::ExternCrate(_) => true, + Item::Macro(invocation) => invocation + .mac + .path + .segments + .last() + .is_some_and(|segment| segment.ident == "planning_model"), + _ => false, + }; + if !allowed { + return Err(format!( + "module roots may hold only module declarations and re-exports; move {} into its own file", + describe(item) + )); + } + } + Ok(()) +} + +/// Every `mod.rs` and `lib.rs` under `src_dir`, sorted. +pub(crate) fn module_roots(src_dir: &Path) -> Vec { + let mut roots = Vec::new(); + collect(src_dir, &mut roots); + roots.sort(); + roots +} + +fn collect(dir: &Path, roots: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + collect(&path, roots); + } else if matches!( + path.file_name().and_then(|name| name.to_str()), + Some("mod.rs" | "lib.rs") + ) { + roots.push(path); + } + } +} + +fn is_documentation(meta: &Meta) -> bool { + meta.path().is_ident("doc") +} + +fn describe(item: &Item) -> String { + match item { + Item::Fn(function) => format!("fn `{}`", function.sig.ident), + Item::Struct(definition) => format!("struct `{}`", definition.ident), + Item::Enum(definition) => format!("enum `{}`", definition.ident), + Item::Trait(definition) => format!("trait `{}`", definition.ident), + Item::Const(definition) => format!("const `{}`", definition.ident), + Item::Static(definition) => format!("static `{}`", definition.ident), + Item::Type(definition) => format!("type `{}`", definition.ident), + Item::Mod(module) => format!("the body of `mod {}`", module.ident), + Item::Impl(_) => "the impl block".to_string(), + Item::Macro(_) => "the macro invocation".to_string(), + _ => "the item".to_string(), + } +} + +#[cfg(test)] +#[path = "module_manifest_tests.rs"] +mod tests; diff --git a/src/module_manifest_tests.rs b/src/module_manifest_tests.rs new file mode 100644 index 0000000..1a42045 --- /dev/null +++ b/src/module_manifest_tests.rs @@ -0,0 +1,52 @@ +use super::ensure_declarations_only; + +#[test] +fn accepts_declarations_reexports_and_the_planning_manifest() { + ensure_declarations_only( + r#"//! Docs. + +// @solverforge:begin constraint-modules +mod all_assigned; +// @solverforge:end constraint-modules +#[cfg(test)] +mod tests; +mod assemble; + +pub use assemble::create_constraints; +use crate::domain::Plan; +"#, + ) + .expect("declarations only"); + ensure_declarations_only( + r#"solverforge::planning_model! { + root = "src/domain"; + mod plan; + pub use plan::Plan; +} +"#, + ) + .expect("the planning manifest is a declaration"); +} + +#[test] +fn rejects_code_in_a_module_root() { + for (src, needle) in [ + ( + "pub fn create_constraints() {}\n", + "fn `create_constraints`", + ), + ( + "mod assemble { pub fn f() {} }\n", + "the body of `mod assemble`", + ), + ("pub struct Plan;\n", "struct `Plan`"), + ("const LIMIT: usize = 4;\n", "const `LIMIT`"), + ("macro_rules! m { () => {} }\n", "the macro invocation"), + ("impl Foo {}\n", "the impl block"), + ] { + let err = ensure_declarations_only(src).expect_err(src); + assert!(err.contains(needle), "{src}: {err}"); + } + let err = ensure_declarations_only("#![allow(dead_code)]\nmod a;\n").expect_err("inner attr"); + assert!(err.contains("inner attributes"), "{err}"); +} diff --git a/src/test_support.rs b/src/test_support.rs index 4673234..2704425 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -1,5 +1,8 @@ +use std::path::Path; use std::sync::{Mutex, MutexGuard, OnceLock}; +use crate::commands::generate_constraint::ConstraintSources; + static CWD_LOCK: OnceLock> = OnceLock::new(); /// Serializes tests that change the process working directory. @@ -14,3 +17,27 @@ pub(crate) fn lock_cwd() -> MutexGuard<'static, ()> { .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()) } + +/// The two managed constraint files: `mod.rs` declares `modules` (one +/// `mod name;` line each) inside the `constraint-modules` block, and +/// `assemble.rs` holds `calls` inside the `constraint-calls` block of +/// `create_constraints`. +pub(crate) fn constraint_sources(modules: &str, calls: &str) -> ConstraintSources { + ConstraintSources { + module: format!( + "// @solverforge:begin constraint-modules\n{modules}// @solverforge:end constraint-modules\n\nmod assemble;\n\npub use assemble::create_constraints;\n" + ), + assemble: format!( + "use crate::domain::Plan;\nuse solverforge::prelude::*;\n\npub fn create_constraints() -> impl ConstraintSet {{\n // @solverforge:begin constraint-calls\n{calls} // @solverforge:end constraint-calls\n}}\n" + ), + } +} + +/// Writes [`constraint_sources`] under `src/constraints/` of the current +/// directory. +pub(crate) fn write_constraint_sources(modules: &str, calls: &str) { + std::fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); + constraint_sources(modules, calls) + .write(Path::new(".")) + .expect("failed to write constraint sources"); +} diff --git a/templates/list/generic/src/constraints/assemble.rs b/templates/list/generic/src/constraints/assemble.rs new file mode 100644 index 0000000..d01894c --- /dev/null +++ b/templates/list/generic/src/constraints/assemble.rs @@ -0,0 +1,14 @@ +//! Assembles the project's constraint set. + +use crate::domain::Plan; +use solverforge::prelude::*; + +/// Builds the constraint set the solver scores every solution against. +pub fn create_constraints() -> impl ConstraintSet { + // @solverforge:begin constraint-calls + ( + super::balanced_load::constraint(), + super::sequence_cohesion::constraint(), + ) + // @solverforge:end constraint-calls +} diff --git a/templates/list/generic/src/constraints/mod.rs b/templates/list/generic/src/constraints/mod.rs index 97e7b9e..e989c2f 100644 --- a/templates/list/generic/src/constraints/mod.rs +++ b/templates/list/generic/src/constraints/mod.rs @@ -1,26 +1,16 @@ -/* Constraint definitions. - -Each constraint is a separate function returning an `IncrementalConstraint`. -Assemble them into a tuple — the solver scores the solution against all of them. - -Add as many constraints as your problem needs. The tuple supports up to 12 elements; -use nested tuples for more. */ - -pub use self::assemble::create_constraints; +//! Constraint definitions. +//! +//! Each constraint is a separate function returning an `IncrementalConstraint`. +//! Assemble them into a tuple — the solver scores the solution against all of them. +//! +//! Add as many constraints as your problem needs. The tuple supports up to 12 elements; +//! use nested tuples for more. // @solverforge:begin constraint-modules mod balanced_load; mod sequence_cohesion; // @solverforge:end constraint-modules -mod assemble { - use super::*; - use crate::domain::Plan; - use solverforge::prelude::*; +mod assemble; - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - (balanced_load::constraint(), sequence_cohesion::constraint()) - // @solverforge:end constraint-calls - } -} +pub use assemble::create_constraints; diff --git a/templates/scalar/generic/src/constraints/assemble.rs b/templates/scalar/generic/src/constraints/assemble.rs new file mode 100644 index 0000000..b79a288 --- /dev/null +++ b/templates/scalar/generic/src/constraints/assemble.rs @@ -0,0 +1,11 @@ +//! Assembles the project's constraint set. + +use crate::domain::Plan; +use solverforge::prelude::*; + +/// Builds the constraint set the solver scores every solution against. +pub fn create_constraints() -> impl ConstraintSet { + // @solverforge:begin constraint-calls + () + // @solverforge:end constraint-calls +} diff --git a/templates/scalar/generic/src/constraints/mod.rs b/templates/scalar/generic/src/constraints/mod.rs index 0ef0a72..98fcfae 100644 --- a/templates/scalar/generic/src/constraints/mod.rs +++ b/templates/scalar/generic/src/constraints/mod.rs @@ -1,22 +1,11 @@ -/* Constraint definitions. - -Add constraint modules with `solverforge generate constraint ...`. -The neutral shell starts with an empty constraint set. */ - -use crate::domain::Plan; -use solverforge::prelude::*; - -pub use self::assemble::create_constraints; +//! Constraint definitions. +//! +//! Add constraint modules with `solverforge generate constraint ...`. +//! The neutral shell starts with an empty constraint set. // @solverforge:begin constraint-modules // @solverforge:end constraint-modules -mod assemble { - use super::*; +mod assemble; - pub fn create_constraints() -> impl ConstraintSet { - // @solverforge:begin constraint-calls - () - // @solverforge:end constraint-calls - } -} +pub use assemble::create_constraints; diff --git a/tests/scaffold_test.rs b/tests/scaffold_test.rs index 5673385..1147277 100644 --- a/tests/scaffold_test.rs +++ b/tests/scaffold_test.rs @@ -1533,6 +1533,13 @@ fn test_generate_solution_replaces_neutral_scaffold_and_cargo_check_passes() { let constraints_mod = std::fs::read_to_string(project_dir.join("src").join("constraints").join("mod.rs")) .unwrap(); + let assembly = std::fs::read_to_string( + project_dir + .join("src") + .join("constraints") + .join("assemble.rs"), + ) + .unwrap(); let data_seed = std::fs::read_to_string(project_dir.join("src").join("data").join("data_seed.rs")).unwrap(); let dto_rs = @@ -1561,10 +1568,16 @@ fn test_generate_solution_replaces_neutral_scaffold_and_cargo_check_passes() { domain_mod ); assert!( - constraints_mod.contains("use crate::domain::Schedule;") - && constraints_mod.contains("ConstraintSet") - && !constraints_mod.contains("ConstraintSet") + && !assembly.contains("ConstraintSet impl ConstraintSet { - () - } -} -"#, + "mod assemble;\n\npub use assemble::create_constraints;\n", ) .unwrap();