diff --git a/crates/engine/src/cmd/forms.rs b/crates/engine/src/cmd/forms.rs index c362e80..e203c14 100644 --- a/crates/engine/src/cmd/forms.rs +++ b/crates/engine/src/cmd/forms.rs @@ -1045,12 +1045,17 @@ mod tests { if fill_refs { fill_required(&s, form, &mut p); } - let missing: Vec = problems(form, &p).into_iter().filter(|p| p.message == "required").map(|p| p.path).collect(); - let r = s.execute(c.id, &p).map_err(|e| match e { - EngineError::Param { .. } => EngineError::Params(e.to_string()), - e => e, - }); - match r { + let missing: Vec = problems(form, &p).into_iter().filter(|p| p.kind == ProblemKind::Required).map(|p| p.path).collect(); + // The engine names a missing nested field by its path, its own key, or the group + // that holds it. + let is_missing = |key: &str| { + missing.iter().any(|m| m == key || m.rsplit('/').next() == Some(key) || m.strip_prefix(key).is_some_and(|r| r.starts_with('/'))) + }; + match s.execute(c.id, &p) { + Err(EngineError::Param { key, .. }) if is_missing(&key) => {} + Err(EngineError::Param { key, problem }) => { + failures.push(format!("{}: {p} refused for `{key}` {problem}, but only {missing:?} were missing", c.id)); + } // State, not parameters: the meshers' default is the open design's bodies, the // bar has no explicit step; activation and probes need one of several fields. Err(EngineError::Params(e)) if STATE.iter().any(|m| e.contains(m)) => {} diff --git a/crates/engine/src/schema.rs b/crates/engine/src/schema.rs index 27bceb7..f500314 100644 --- a/crates/engine/src/schema.rs +++ b/crates/engine/src/schema.rs @@ -450,26 +450,145 @@ pub fn switch_variant(f: &Field, common: &[Field], old: &Map, tag out } -/// A problem with a field's value, for the form to show beside it. -#[derive(Debug, Clone, PartialEq)] +/// A problem with a field's value: the field, where it is, and what is wrong. [`Problem::for_form`] +/// phrases it by the field's label (the form), [`Problem::for_agents`] by its JSON path (agents, +/// the engine's messages). +#[derive(Debug, Clone)] pub struct Problem { - /// The JSON path of the field (`elastic/young`). + /// The JSON path of the field (`elastic/young`, `rows/0/name`); empty for the parameters as a + /// whole. pub path: String, - pub message: String, + /// The field it is about; `None` for the parameters as a whole. For a variant's tag, the + /// [`Kind::Variants`] field. + pub field: Option<&'static Field>, + pub kind: ProblemKind, +} + +/// What is wrong with a value. +#[derive(Debug, Clone, PartialEq)] +pub enum ProblemKind { + /// Left out (or empty text). + Required, + /// Not a finite number. + NotANumber, + /// A number below the field's minimum. + Below(f64), + /// A number above the field's maximum. + Above(f64), + /// Not a whole number in `min..=max`. + NotAWholeNumber { + min: i64, + max: i64, + }, + NotText, + NotABool, + /// Not one of the field's choices (or, for [`Kind::Either`], of its options). + NotAChoice, + /// Not a list of `len` numbers. + NotAVector { + len: usize, + }, + NotNames, + NotAnObject, + /// A variant tag the field does not have (`None`: no tag at all). + UnknownVariant(Option), + /// More rows than `max`, or not a list. + TooManyRows { + max: usize, + }, +} + +impl Problem { + fn new(path: String, field: &'static Field, kind: ProblemKind) -> Self { + Problem { path, field: Some(field), kind } + } + + /// What the value must be, without the field's name ("required", "at least 0"). + pub fn expected(&self) -> String { + let f = self.field; + match &self.kind { + ProblemKind::Required => "required".into(), + ProblemKind::NotANumber => "a number".into(), + ProblemKind::Below(lo) => format!("at least {lo}"), + ProblemKind::Above(hi) => format!("at most {hi}"), + ProblemKind::NotAWholeNumber { min, max } => format!("a whole number in {min}..={max}"), + ProblemKind::NotText => "text".into(), + ProblemKind::NotABool => "true or false".into(), + ProblemKind::NotAChoice => match f.map(|f| f.kind) { + Some(Kind::Choice(options)) => format!("one of {}", options.iter().map(|(o, _)| *o).collect::>().join(", ")), + Some(Kind::Either(options)) => format!("one of {}", options.iter().map(|o| o.key).collect::>().join(", ")), + _ => "one of the choices".into(), + }, + ProblemKind::NotAVector { len } => format!("{len} numbers"), + ProblemKind::NotNames => "a list of names".into(), + ProblemKind::NotAnObject if f.is_none() => "a JSON object".into(), + ProblemKind::NotAnObject => "an object".into(), + ProblemKind::UnknownVariant(Some(t)) => format!("a known {} (not `{t}`)", f.map_or("type".into(), |f| f.label.to_lowercase())), + ProblemKind::UnknownVariant(None) => match f.map(|f| f.kind) { + Some(Kind::Variants { tag, .. }) => format!("an object with a known `{tag}`"), + _ => "an object with a known type".into(), + }, + ProblemKind::TooManyRows { max } => format!("a list of at most {max} rows"), + } + } + + /// The problem as agents read it, naming the field by its JSON path: + /// "`elastic/young` is required", "`bands` must be a whole number in 2..=24". + pub fn for_agents(&self) -> String { + let what = if self.path.is_empty() { "the parameters".to_string() } else { format!("`{}`", self.path) }; + match self.kind { + ProblemKind::Required => format!("{what} is required"), + _ => format!("{what} must be {}", self.expected()), + } + } + + /// The problem as a form shows it, naming the field by its label and saying what to do: + /// "Material: choose a material", "Young's modulus: must be at least 0". + pub fn for_form(&self) -> String { + let Some(f) = self.field else { return format!("The parameters must be {}", self.expected()) }; + let required = self.kind == ProblemKind::Required; + let text = match f.kind { + Kind::Ref(c) if required => format!("choose {}", article(c.label())), + Kind::RefList(c) if required => format!("choose at least one {}", c.label()), + Kind::Either(options) => format!("choose {}", options.iter().map(|o| article(&lower(o.label))).collect::>().join(" or ")), + Kind::Choice(options) => format!("choose one of {}", options.iter().map(|(_, l)| *l).collect::>().join(", ")), + Kind::Variants { .. } => format!("choose {}", article(&lower(f.label))), + Kind::Path { .. } if required => "choose a file".into(), + Kind::Text if required => format!("enter {}", article(&lower(f.label))), + _ if required => "enter a value".into(), + _ => format!("must be {}", self.expected()), + }; + format!("{}: {text}", f.label) + } +} + +/// "a material", "an element". +fn article(noun: &str) -> String { + let an = noun.starts_with(['a', 'e', 'i', 'o', 'u']); + format!("{} {noun}", if an { "an" } else { "a" }) +} + +/// A label lower-cased, but not its acronyms ("Reference point" → "reference point", "RBE2" stays). +fn lower(label: &str) -> String { + let mut chars = label.chars(); + match (chars.next(), chars.next()) { + (Some(c), Some(d)) if d.is_lowercase() => c.to_lowercase().chain(label.chars().skip(1)).collect(), + _ => label.to_string(), + } } /// What is missing or out of range in `params`: required fields left out, numbers outside /// their range, values of the wrong kind. An empty list does not promise the command accepts /// them (it checks more); a problem means it would not. -pub fn problems(fields: &[Field], params: &Value) -> Vec { +pub fn problems(fields: &'static [Field], params: &Value) -> Vec { match params.as_object() { Some(obj) => object_problems(fields, obj), - None => vec![Problem { path: String::new(), message: "the parameters must be a JSON object".into() }], + None => vec![Problem { path: String::new(), field: None, kind: ProblemKind::NotAnObject }], } } /// [`problems`] of an object's fields. -pub fn object_problems(fields: &[Field], obj: &Map) -> Vec { +pub fn object_problems(fields: &'static [Field], obj: &Map) -> Vec { let mut out = Vec::new(); check(fields, obj, "", &mut out, 0); out @@ -486,7 +605,42 @@ pub fn join(prefix: &str, key: &str) -> String { } } -fn check(fields: &[Field], obj: &Map, prefix: &str, out: &mut Vec, depth: usize) { +/// The field at `path` (`elastic/young`, `rows/0/name`), for a key the engine names. A variant's +/// tag gives its [`Kind::Variants`] field; an [`Kind::Either`] option's key gives the option. +pub fn field_at(fields: &'static [Field], path: &str) -> Option<&'static Field> { + let mut segments = path.split('/'); + let mut field = find(fields, segments.next()?)?; + for (depth, segment) in segments.enumerate() { + if depth > MAX_DEPTH { + return None; + } + field = match field.kind { + Kind::Rows { .. } if segment.parse::().is_ok() => field, + Kind::Group(inner) | Kind::Rows { fields: inner, .. } => find(inner, segment)?, + Kind::Variants { tag, .. } if tag == segment => field, + Kind::Variants { variants, .. } => variants.iter().find_map(|v| find(v.fields, segment))?, + Kind::Either(options) => options.iter().find(|o| o.key == segment)?, + _ => return None, + }; + } + Some(field) +} + +/// The field of `fields` with key `key`, looking into flattened variants. +fn find(fields: &'static [Field], key: &str) -> Option<&'static Field> { + fields.iter().find_map(|f| match f.kind { + Kind::Variants { tag, variants } if f.key.is_empty() => { + if tag == key { + Some(f) + } else { + variants.iter().find_map(|v| find(v.fields, key)) + } + } + _ => (f.key == key).then_some(f), + }) +} + +fn check(fields: &'static [Field], obj: &Map, prefix: &str, out: &mut Vec, depth: usize) { if depth > MAX_DEPTH || out.len() > MAX_ROWS { return; } @@ -497,20 +651,20 @@ fn check(fields: &[Field], obj: &Map, prefix: &str, out: &mut Vec match obj.get(tag).and_then(Value::as_str) { Some(t) => match f.kind.variant(t) { Some(v) => check(v.fields, obj, prefix, out, depth + 1), - None => out.push(Problem { path: join(prefix, tag), message: format!("`{t}` is not a {}", f.label.to_lowercase()) }), + None => out.push(Problem::new(join(prefix, tag), f, ProblemKind::UnknownVariant(Some(t.to_string())))), }, - None if f.required => out.push(Problem { path: join(prefix, tag), message: "required".into() }), + None if f.required => out.push(Problem::new(join(prefix, tag), f, ProblemKind::Required)), None => {} } continue; } let path = join(prefix, f.key); match obj.get(f.key) { - None | Some(Value::Null) if f.required => out.push(Problem { path, message: "required".into() }), + None | Some(Value::Null) if f.required => out.push(Problem::new(path, f, ProblemKind::Required)), None | Some(Value::Null) => {} Some(v) => { - if let Some(message) = value_problem(f, v, &path, out, depth) { - out.push(Problem { path, message }); + if let Some(kind) = value_problem(f, v, &path, out, depth) { + out.push(Problem::new(path, f, kind)); } } } @@ -518,39 +672,39 @@ fn check(fields: &[Field], obj: &Map, prefix: &str, out: &mut Vec } /// What is wrong with `v` as the value of `f`; nested problems go to `out`. -fn value_problem(f: &Field, v: &Value, path: &str, out: &mut Vec, depth: usize) -> Option { +fn value_problem(f: &'static Field, v: &Value, path: &str, out: &mut Vec, depth: usize) -> Option { match f.kind { Kind::Number { min, max, .. } => match (v.as_f64().filter(|x| x.is_finite()), min, max) { - (None, ..) => Some("a number".into()), - (Some(x), Some(lo), _) if x < lo => Some(format!("at least {lo}")), - (Some(x), _, Some(hi)) if x > hi => Some(format!("at most {hi}")), + (None, ..) => Some(ProblemKind::NotANumber), + (Some(x), Some(lo), _) if x < lo => Some(ProblemKind::Below(lo)), + (Some(x), _, Some(hi)) if x > hi => Some(ProblemKind::Above(hi)), _ => None, }, Kind::Integer { min, max } => match v.as_i64() { Some(x) if (min..=max).contains(&x) => None, - _ => Some(format!("a whole number in {min}..={max}")), + _ => Some(ProblemKind::NotAWholeNumber { min, max }), }, Kind::Text | Kind::Path { .. } | Kind::Ref(_) => match v.as_str() { - Some(s) if f.required && s.trim().is_empty() => Some("required".into()), + Some(s) if f.required && s.trim().is_empty() => Some(ProblemKind::Required), Some(_) => None, - None => Some("text".into()), + None => Some(ProblemKind::NotText), }, - Kind::Bool => (!v.is_boolean()).then(|| "true or false".into()), + Kind::Bool => (!v.is_boolean()).then_some(ProblemKind::NotABool), Kind::Choice(options) => match v.as_str() { Some(s) if options.iter().any(|(o, _)| *o == s) => None, - _ => Some(format!("one of {}", options.iter().map(|(o, _)| *o).collect::>().join(", "))), + _ => Some(ProblemKind::NotAChoice), }, Kind::Vector { labels, nullable, .. } => match v.as_array() { Some(a) if a.len() == labels.len() && a.iter().all(|x| x.as_f64().is_some_and(f64::is_finite) || (nullable && x.is_null())) => None, - _ => Some(format!("{} numbers", labels.len())), + _ => Some(ProblemKind::NotAVector { len: labels.len() }), }, - Kind::RefList(_) => (!v.as_array().is_some_and(|a| a.iter().all(Value::is_string))).then(|| "a list of names".into()), + Kind::RefList(_) => (!v.as_array().is_some_and(|a| a.iter().all(Value::is_string))).then_some(ProblemKind::NotNames), Kind::Group(fields) => match v.as_object() { Some(o) => { check(fields, o, path, out, depth + 1); None } - None => Some("an object".into()), + None => Some(ProblemKind::NotAnObject), }, Kind::Variants { tag, .. } => match v.as_object() { Some(o) => match o.get(tag).and_then(Value::as_str).and_then(|t| f.kind.variant(t)) { @@ -558,9 +712,9 @@ fn value_problem(f: &Field, v: &Value, path: &str, out: &mut Vec, depth check(var.fields, o, path, out, depth + 1); None } - None => Some(format!("needs a known `{tag}`")), + None => Some(ProblemKind::UnknownVariant(None)), }, - None => Some("an object".into()), + None => Some(ProblemKind::NotAnObject), }, Kind::Either(options) => { let ok = match v { @@ -570,20 +724,21 @@ fn value_problem(f: &Field, v: &Value, path: &str, out: &mut Vec, depth }), _ => false, }; - (!ok).then(|| format!("one of {}", options.iter().map(|o| o.key).collect::>().join(", "))) + (!ok).then_some(ProblemKind::NotAChoice) } Kind::Unit | Kind::Json => None, Kind::Rows { fields, max } => match v.as_array() { Some(a) if a.len() <= max => { for (i, row) in a.iter().enumerate() { + let row_path = format!("{path}/{i}"); match row.as_object() { - Some(o) => check(fields, o, &format!("{path}/{i}"), out, depth + 1), - None => out.push(Problem { path: format!("{path}/{i}"), message: "an object".into() }), + Some(o) => check(fields, o, &row_path, out, depth + 1), + None => out.push(Problem::new(row_path, f, ProblemKind::NotAnObject)), } } None } - _ => Some(format!("a list of at most {max} rows")), + _ => Some(ProblemKind::TooManyRows { max }), }, } } @@ -737,6 +892,24 @@ mod tests { assert!(super::problems(LOADISH, &json!([1])).len() == 1); } + /// One problem, two phrasings: by label for the form, by path for agents. + #[test] + fn problems_carry_their_field_and_kind() { + let p = problems(LOADISH, &json!({"type": "pressure", "magnitude": "x"})); + let kinds: Vec<(&str, &ProblemKind)> = p.iter().map(|p| (p.path.as_str(), &p.kind)).collect(); + assert_eq!(kinds, [("name", &ProblemKind::Required), ("magnitude", &ProblemKind::NotANumber), ("set", &ProblemKind::Required)]); + assert_eq!(p.iter().map(Problem::for_form).collect::>(), ["Name: enter a name", "Magnitude: must be a number", "Set: choose a set"]); + assert_eq!( + p.iter().map(Problem::for_agents).collect::>(), + ["`name` is required", "`magnitude` must be a number", "`set` is required"] + ); + let unknown = problems(LOADISH, &json!({"name": "a", "type": "spin"})); + assert_eq!(unknown.first().map(Problem::for_form).as_deref(), Some("Type: choose a type")); + assert_eq!(unknown.first().map(Problem::for_agents).as_deref(), Some("`type` must be a known type (not `spin`)")); + assert_eq!(problems(LOADISH, &json!([1])).first().map(Problem::for_agents).as_deref(), Some("the parameters must be a JSON object")); + assert!(field_at(LOADISH, "magnitude").is_some_and(|f| f.label == "Magnitude")); + } + #[test] fn json_schema_has_variants_and_requirements() { let s = json_schema(LOADISH); diff --git a/crates/ui-egui/src/form.rs b/crates/ui-egui/src/form.rs index 16caeb4..43985f9 100644 --- a/crates/ui-egui/src/form.rs +++ b/crates/ui-egui/src/form.rs @@ -469,85 +469,17 @@ impl Form<'_> { } /// What is missing or wrong in `params`, in the form's order: (field path, message), each -/// phrased with the field's label as the form shows it ("Material: choose a material"), never -/// its JSON key. +/// phrased with the field's label as the form shows it ([`schema::Problem::for_form`]). pub fn messages(fields: &'static [Field], params: &Map) -> Vec<(String, String)> { schema::object_problems(fields, params) .into_iter() .map(|p| { - let message = field_at(fields, &p.path).map_or_else(|| format!("{}: {}", p.path, p.message), |f| phrase(f, &p.message)); - (p.path, message) + let m = p.for_form(); + (p.path, m) }) .collect() } -/// The field at `path` (`elastic/young`, `rows/0/name`). A variant's tag gives its -/// [`Kind::Variants`] field; an [`Kind::Either`] option's value gives the option. -pub fn field_at(fields: &'static [Field], path: &str) -> Option<&'static Field> { - let mut segments = path.split('/'); - let mut field = find(fields, segments.next()?)?; - for (depth, segment) in segments.enumerate() { - if depth > MAX_DEPTH { - return None; - } - field = match field.kind { - Kind::Rows { .. } if segment.parse::().is_ok() => field, - Kind::Group(inner) | Kind::Rows { fields: inner, .. } => find(inner, segment)?, - Kind::Variants { tag, .. } if tag == segment => field, - Kind::Variants { variants, .. } => variants.iter().find_map(|v| find(v.fields, segment))?, - Kind::Either(options) => options.iter().find(|o| o.key == segment)?, - _ => return None, - }; - } - Some(field) -} - -/// The field of `fields` with key `key`, looking into flattened variants. -fn find(fields: &'static [Field], key: &str) -> Option<&'static Field> { - fields.iter().find_map(|f| match f.kind { - Kind::Variants { tag, variants } if f.key.is_empty() => { - if tag == key { - Some(f) - } else { - variants.iter().find_map(|v| find(v.fields, key)) - } - } - _ => (f.key == key).then_some(f), - }) -} - -/// A schema problem ([`schema::Problem::message`]) of field `f`, as the form says it. -fn phrase(f: &Field, problem: &str) -> String { - let required = problem == "required"; - let text = match f.kind { - Kind::Ref(c) if required => format!("choose {}", article(c.label())), - Kind::RefList(c) if required => format!("choose at least one {}", c.label()), - Kind::Either(options) => format!("choose {}", options.iter().map(|o| article(&lower(o.label))).collect::>().join(" or ")), - Kind::Choice(options) => format!("choose one of {}", options.iter().map(|(_, l)| *l).collect::>().join(", ")), - Kind::Variants { .. } => format!("choose {}", article(&lower(f.label))), - Kind::Path { .. } if required => "choose a file".into(), - Kind::Text if required && f.key.ends_with("name") => "enter a name".into(), - _ if required => "enter a value".into(), - _ => format!("must be {problem}"), - }; - format!("{}: {text}", f.label) -} - -/// "a material", "an element". -fn article(noun: &str) -> String { - let an = noun.starts_with(['a', 'e', 'i', 'o', 'u']); - format!("{} {noun}", if an { "an" } else { "a" }) -} - -/// A label lower-cased, but not its acronyms ("Reference point" → "reference point", "RBE2" stays). -fn lower(label: &str) -> String { - let mut chars = label.chars(); - match (chars.next(), chars.next()) { - (Some(c), Some(d)) if d.is_lowercase() => c.to_lowercase().chain(label.chars().skip(1)).collect(), - _ => label.to_string(), - } -} - /// A collection's label in the plural: "materials", "point masses", "bodies". fn plural(label: &str) -> String { if let Some(stem) = label.strip_suffix('y') { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 33839dc..ba54df1 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -476,7 +476,7 @@ impl ModelApp { let label = command_label(id); let r = self.session.execute(id, ¶ms).map_err(|e| match e { EngineError::Param { key, problem } => { - let message = match fields.and_then(|f| form::field_at(f, &key)) { + let message = match fields.and_then(|f| fieldcraft_engine::schema::field_at(f, &key)) { Some(f) => format!("{}: {problem}", f.label), None => format!("`{key}` {problem}"), };