diff --git a/crates/typst/src/foundations/cast.rs b/crates/typst/src/foundations/cast.rs index 9c25aa8e9..cb0099129 100644 --- a/crates/typst/src/foundations/cast.rs +++ b/crates/typst/src/foundations/cast.rs @@ -43,11 +43,11 @@ pub trait Reflect { /// dynamic checks instead of optimized machine code for each type). fn castable(value: &Value) -> bool; - /// Produce an error message for an inacceptable value. + /// Produce an error message for an inacceptable value type. /// /// ``` /// assert_eq!( - /// ::error(Value::None), + /// ::error(&Value::None), /// "expected integer, found none", /// ); /// ``` diff --git a/crates/typst/src/foundations/dict.rs b/crates/typst/src/foundations/dict.rs index f072a6193..291b8f60a 100644 --- a/crates/typst/src/foundations/dict.rs +++ b/crates/typst/src/foundations/dict.rs @@ -113,15 +113,44 @@ impl Dict { self.0.iter() } - /// Return an "unexpected key" error if there is any remaining pair. + /// Check if there is any remaining pair, and if so return an + /// "unexpected key" error. pub fn finish(&self, expected: &[&str]) -> StrResult<()> { - if let Some((key, _)) = self.iter().next() { - let parts: Vec<_> = expected.iter().map(|s| eco_format!("\"{s}\"")).collect(); - let mut msg = format!("unexpected key {}, valid keys are ", key.repr()); - msg.push_str(&repr::separated_list(&parts, "and")); - return Err(msg.into()); + let mut iter = self.iter().peekable(); + if iter.peek().is_none() { + return Ok(()); } - Ok(()) + let unexpected: Vec<&str> = iter.map(|kv| kv.0.as_str()).collect(); + + Err(Self::unexpected_keys(unexpected, Some(expected))) + } + + // Return an "unexpected key" error string. + pub fn unexpected_keys( + unexpected: Vec<&str>, + hint_expected: Option<&[&str]>, + ) -> EcoString { + let format_as_list = |arr: &[&str]| { + repr::separated_list( + &arr.iter().map(|s| eco_format!("\"{s}\"")).collect::>(), + "and", + ) + }; + + let mut msg = String::from(match unexpected.len() { + 0 => unreachable!(), + 1 => "unexpected key ", + _ => "unexpected keys ", + }); + + msg.push_str(&format_as_list(&unexpected[..])); + + if let Some(expected) = hint_expected { + msg.push_str(", valid keys are "); + msg.push_str(&format_as_list(expected)); + } + + msg.into() } } diff --git a/crates/typst/src/layout/corners.rs b/crates/typst/src/layout/corners.rs index e014201c0..e674b04d6 100644 --- a/crates/typst/src/layout/corners.rs +++ b/crates/typst/src/layout/corners.rs @@ -164,7 +164,7 @@ where T: FromValue + Clone, { fn from_value(mut value: Value) -> StrResult { - let keys = [ + let expected_keys = [ "top-left", "top-right", "bottom-right", @@ -177,7 +177,9 @@ where ]; if let Value::Dict(dict) = &mut value { - if dict.iter().any(|(key, _)| keys.contains(&key.as_str())) { + if dict.is_empty() { + return Ok(Self::splat(None)); + } else if dict.iter().any(|(key, _)| expected_keys.contains(&key.as_str())) { let mut take = |key| dict.take(key).ok().map(T::from_value).transpose(); let rest = take("rest")?; let left = take("left")?.or_else(|| rest.clone()); @@ -199,13 +201,18 @@ where .or_else(|| left.clone()), }; - dict.finish(&keys)?; + dict.finish(&expected_keys)?; return Ok(corners); } } if T::castable(&value) { Ok(Self::splat(Some(T::from_value(value)?))) + } else if let Value::Dict(dict) = &value { + let keys = dict.iter().map(|kv| kv.0.as_str()).collect(); + // Do not hint at expected_keys, because T may be castable from Dict + // objects with other sets of expected keys. + Err(Dict::unexpected_keys(keys, None)) } else { Err(Self::error(&value)) } diff --git a/crates/typst/src/layout/sides.rs b/crates/typst/src/layout/sides.rs index 8d472c4ed..937919c4b 100644 --- a/crates/typst/src/layout/sides.rs +++ b/crates/typst/src/layout/sides.rs @@ -190,9 +190,11 @@ where T: Default + FromValue + Clone, { fn from_value(mut value: Value) -> StrResult { - let keys = ["left", "top", "right", "bottom", "x", "y", "rest"]; + let expected_keys = ["left", "top", "right", "bottom", "x", "y", "rest"]; if let Value::Dict(dict) = &mut value { - if dict.iter().any(|(key, _)| keys.contains(&key.as_str())) { + if dict.is_empty() { + return Ok(Self::splat(None)); + } else if dict.iter().any(|(key, _)| expected_keys.contains(&key.as_str())) { let mut take = |key| dict.take(key).ok().map(T::from_value).transpose(); let rest = take("rest")?; let x = take("x")?.or_else(|| rest.clone()); @@ -204,13 +206,18 @@ where bottom: take("bottom")?.or_else(|| y.clone()), }; - dict.finish(&keys)?; + dict.finish(&expected_keys)?; return Ok(sides); } } if T::castable(&value) { Ok(Self::splat(Some(T::from_value(value)?))) + } else if let Value::Dict(dict) = &value { + let keys = dict.iter().map(|kv| kv.0.as_str()).collect(); + // Do not hint at expected_keys, because T may be castable from Dict + // objects with other sets of expected keys. + Err(Dict::unexpected_keys(keys, None)) } else { Err(Self::error(&value)) } diff --git a/tests/typ/bugs/3232-dict-wrong-keys.typ b/tests/typ/bugs/3232-dict-wrong-keys.typ new file mode 100644 index 000000000..61d9e8b8a --- /dev/null +++ b/tests/typ/bugs/3232-dict-wrong-keys.typ @@ -0,0 +1,23 @@ +// Issue #3232: Confusing "expected relative length or dictionary, found dictionary" +// https://github.com/typst/typst/issues/3232 +// Ref: false + +--- +// Error: 16-58 unexpected keys "unexpected" and "unexpected-too" +#block(outset: (unexpected: 0.5em, unexpected-too: 0.2em), [Hi]) + +--- +// Error: 14-56 unexpected keys "unexpected" and "unexpected-too" +#box(radius: (unexpected: 0.5em, unexpected-too: 0.5em), [Hi]) + +--- +// Error: 16-49 unexpected key "unexpected", valid keys are "left", "top", "right", "bottom", "x", "y", and "rest" +#block(outset: (unexpected: 0.2em, right: 0.5em), [Hi]) // The 1st key is unexpected + +--- +// Error: 14-50 unexpected key "unexpected", valid keys are "top-left", "top-right", "bottom-right", "bottom-left", "left", "top", "right", "bottom", and "rest" +#box(radius: (top-left: 0.5em, unexpected: 0.5em), [Hi]) // The 2nd key is unexpected + +--- +#block(outset: (:), [Hi]) // Ok +#box(radius: (:), [Hi]) // Ok