From 0902d687ef19c474b525650ef2fa80a32c156de7 Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Tue, 10 Nov 2026 01:20:54 +0100 Subject: [PATCH] fix(constraints): list constraints from the constraint-modules manifest list_constraints treated every .rs file in src/constraints except mod.rs and assemble.rs as a constraint, so helper modules such as shared.rs, support.rs or a tests.rs module showed up as constraints in solverforge.app.toml, the generated ui-model.json, `solverforge info` and destroy's dependent-constraint hints. The constraint ids are now the modules declared in the constraint-modules block of src/constraints/mod.rs, validated the same way check and the model contract read them; a malformed manifest is reported instead of guessed around, and destroy matches dependents against that list. The file-name predicate is gone. The destroy fixtures declare the constraints they write, as generate constraint does. Closes #11 --- src/app_spec.rs | 3 +- src/commands/destroy.rs | 7 ++- src/commands/destroy_tests.rs | 7 ++- src/commands/generate_constraint/domain.rs | 66 ++++++++++++++++----- src/commands/generate_constraint/mod.rs | 4 +- src/commands/generate_constraint/sources.rs | 6 -- src/commands/info.rs | 15 ++--- 7 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/app_spec.rs b/src/app_spec.rs index 122b857..27dbd9c 100644 --- a/src/app_spec.rs +++ b/src/app_spec.rs @@ -335,7 +335,8 @@ pub fn sync_from_project() -> CliResult { Err(err) => return Err(CliError::general(err)), } - spec.constraints = list_constraints(Path::new("src/constraints")) + spec.constraints = list_constraints(Path::new(".")) + .map_err(CliError::general)? .into_iter() .map(|name| ConstraintSpec { module: name.clone(), diff --git a/src/commands/destroy.rs b/src/commands/destroy.rs index c8b8160..7acfbd2 100644 --- a/src/commands/destroy.rs +++ b/src/commands/destroy.rs @@ -1,11 +1,11 @@ use std::fs; use std::path::Path; -use crate::commands::generate_constraint::{domain::DomainModel, parse_domain}; use crate::commands::generate_constraint::{ - is_constraint_file_stem, remove_constraint_from_source, ConstraintSources, + domain::list_constraints, remove_constraint_from_source, ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH, }; +use crate::commands::generate_constraint::{domain::DomainModel, parse_domain}; 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, @@ -367,13 +367,14 @@ fn ensure_type_is_unreferenced_in_source( } let constraints_dir = Path::new("src/constraints"); + let declared_constraints = list_constraints(Path::new(".")).map_err(CliError::general)?; let mut constraint_ids = Vec::new(); let mut other_files = Vec::new(); for file in &dependents { let constraint_id = (file.parent() == Some(constraints_dir)) .then(|| file.file_stem().and_then(|stem| stem.to_str())) .flatten() - .filter(|stem| is_constraint_file_stem(stem)); + .filter(|stem| declared_constraints.iter().any(|id| id == stem)); match constraint_id { Some(id) => constraint_ids.push(id.to_string()), None => other_files.push(slash_path(file)), diff --git a/src/commands/destroy_tests.rs b/src/commands/destroy_tests.rs index ce574a2..e8f05fd 100644 --- a/src/commands/destroy_tests.rs +++ b/src/commands/destroy_tests.rs @@ -331,8 +331,13 @@ impl Task { ); } +/// Writes constraint module `name` and declares it in the managed constraint +/// files, as `solverforge generate constraint` does. fn write_task_constraint(name: &str, body: &str) { - fs::create_dir_all("src/constraints").expect("failed to create constraints dir"); + test_support::write_constraint_sources( + &format!("mod {name};\n"), + &format!(" super::{name}::constraint()\n"), + ); fs::write(format!("src/constraints/{name}.rs"), body).expect("failed to write constraint"); } diff --git a/src/commands/generate_constraint/domain.rs b/src/commands/generate_constraint/domain.rs index bd48fef..d0bb8b9 100644 --- a/src/commands/generate_constraint/domain.rs +++ b/src/commands/generate_constraint/domain.rs @@ -742,21 +742,18 @@ fn type_declares_name(item: &Item, type_name: &str) -> bool { } } -pub(crate) 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 super::sources::is_constraint_file_stem(name) { - constraints.push(name.to_string()); - } - } - } +/// Returns the constraint ids of the project under `root`: the modules declared +/// in the `constraint-modules` block of `src/constraints/mod.rs`, sorted. Helper +/// modules declared outside that block are not constraints. A project without +/// `src/constraints/mod.rs` has none. +pub(crate) fn list_constraints(root: &Path) -> Result, String> { + if !root.join(super::sources::CONSTRAINTS_MOD_PATH).exists() { + return Ok(Vec::new()); } - constraints.sort(); - constraints + let sources = super::sources::ConstraintSources::read(root).map_err(|err| err.to_string())?; + let mut ids = super::mod_rewriter::validate_constraint_mod_source(&sources)?; + ids.sort(); + Ok(ids) } #[cfg(test)] @@ -764,6 +761,47 @@ mod tests { use super::*; use tempfile::tempdir; + #[test] + fn list_constraints_reads_the_manifest_not_file_names() { + let temp = tempdir().unwrap(); + let root = temp.path(); + assert_eq!(list_constraints(root).unwrap(), Vec::::new()); + + fs::create_dir_all(root.join("src/constraints")).unwrap(); + crate::test_support::constraint_sources( + "mod zone_cover;\nmod all_assigned;\n", + " (\n super::zone_cover::constraint(),\n super::all_assigned::constraint(),\n )\n", + ) + .write(root) + .unwrap(); + let mut module = fs::read_to_string(root.join("src/constraints/mod.rs")).unwrap(); + module.push_str("mod shared;\n#[cfg(test)]\nmod tests;\n"); + fs::write(root.join("src/constraints/mod.rs"), module).unwrap(); + for helper in ["shared", "tests", "zone_cover", "all_assigned"] { + fs::write(root.join(format!("src/constraints/{helper}.rs")), "").unwrap(); + } + + assert_eq!( + list_constraints(root).unwrap(), + ["all_assigned", "zone_cover"] + ); + } + + #[test] + fn list_constraints_reports_a_malformed_manifest() { + let temp = tempdir().unwrap(); + fs::create_dir_all(temp.path().join("src/constraints")).unwrap(); + crate::test_support::constraint_sources( + "mod all_assigned;\n", + " (\n super::extra::constraint(),\n )\n", + ) + .write(temp.path()) + .unwrap(); + + let err = list_constraints(temp.path()).expect_err("unwired module should fail"); + assert!(err.contains("constraint-calls"), "{err}"); + } + #[test] fn parses_multiline_attrs_and_manifest_modules() { let _guard = crate::test_support::lock_cwd(); diff --git a/src/commands/generate_constraint/mod.rs b/src/commands/generate_constraint/mod.rs index 6954f28..e5f8780 100644 --- a/src/commands/generate_constraint/mod.rs +++ b/src/commands/generate_constraint/mod.rs @@ -12,7 +12,5 @@ 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 sources::{ConstraintSources, CONSTRAINTS_ASSEMBLE_PATH, CONSTRAINTS_MOD_PATH}; pub(crate) use utils::validate_name; diff --git a/src/commands/generate_constraint/sources.rs b/src/commands/generate_constraint/sources.rs index ad90a7e..41f3adb 100644 --- a/src/commands/generate_constraint/sources.rs +++ b/src/commands/generate_constraint/sources.rs @@ -16,12 +16,6 @@ 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 { diff --git a/src/commands/info.rs b/src/commands/info.rs index 0318141..a589b7b 100644 --- a/src/commands/info.rs +++ b/src/commands/info.rs @@ -60,16 +60,13 @@ pub fn run() -> CliResult { println!(); } - let constraints_dir = Path::new("src/constraints"); - if constraints_dir.exists() { - let constraints = list_constraints(constraints_dir); - if !constraints.is_empty() { - println!(" Constraints:"); - for c in &constraints { - println!(" - {}", c); - } - println!(); + let constraints = list_constraints(Path::new(".")).map_err(CliError::general)?; + if !constraints.is_empty() { + println!(" Constraints:"); + for c in &constraints { + println!(" - {}", c); } + println!(); } if Path::new("solver.toml").exists() {