Add message when trying to access a field that is not set (#4399)

Co-authored-by: Laurenz <laurmaedje@gmail.com>
This commit is contained in:
+merlan #flirora 2024-06-22 04:59:52 -04:00 committed by GitHub
parent 3d3489fbae
commit 781eea632f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 114 additions and 54 deletions

View File

@ -88,7 +88,7 @@ fn format(elements: Vec<Content>, command: &QueryCommand) -> StrResult<String> {
let mapped: Vec<_> = elements let mapped: Vec<_> = elements
.into_iter() .into_iter()
.filter_map(|c| match &command.field { .filter_map(|c| match &command.field {
Some(field) => c.get_by_name(field), Some(field) => c.get_by_name(field).ok(),
_ => Some(c.into_value()), _ => Some(c.into_value()),
}) })
.collect(); .collect();

View File

@ -92,7 +92,8 @@ pub fn analyze_labels(document: &Document) -> (Vec<(Label, Option<EcoString>)>,
let Some(label) = elem.label() else { continue }; let Some(label) = elem.label() else { continue };
let details = elem let details = elem
.get_by_name("caption") .get_by_name("caption")
.or_else(|| elem.get_by_name("body")) .or_else(|_| elem.get_by_name("body"))
.ok()
.and_then(|field| match field { .and_then(|field| match field {
Value::Content(content) => Some(content), Value::Content(content) => Some(content),
_ => None, _ => None,

View File

@ -441,7 +441,7 @@ fn field_access_completions(
if let Some((elem, styles)) = func.element().zip(styles.as_ref()) { if let Some((elem, styles)) = func.element().zip(styles.as_ref()) {
for param in elem.params().iter().filter(|param| !param.required) { for param in elem.params().iter().filter(|param| !param.required) {
if let Some(value) = elem.field_id(param.name).and_then(|id| { if let Some(value) = elem.field_id(param.name).and_then(|id| {
elem.field_from_styles(id, StyleChain::new(styles)) elem.field_from_styles(id, StyleChain::new(styles)).ok()
}) { }) {
ctx.value_completion( ctx.value_completion(
Some(param.name.into()), Some(param.name.into()),

View File

@ -390,13 +390,13 @@ fn create_fields_enum(element: &Elem) -> TokenStream {
} }
impl ::std::convert::TryFrom<u8> for Fields { impl ::std::convert::TryFrom<u8> for Fields {
type Error = (); type Error = #foundations::FieldAccessError;
fn try_from(value: u8) -> Result<Self, Self::Error> { fn try_from(value: u8) -> Result<Self, Self::Error> {
#(const #consts: u8 = Fields::#variants as u8;)* #(const #consts: u8 = Fields::#variants as u8;)*
match value { match value {
#(#consts => Ok(Self::#variants),)* #(#consts => Ok(Self::#variants),)*
_ => Err(()), _ => Err(#foundations::FieldAccessError::Internal),
} }
} }
} }
@ -877,9 +877,9 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
let Field { enum_ident, ident, .. } = field; let Field { enum_ident, ident, .. } = field;
let expr = if field.required { let expr = if field.required {
quote! { Some(#into_value(self.#ident.clone())) } quote! { Ok(#into_value(self.#ident.clone())) }
} else { } else {
quote! { self.#ident.clone().map(#into_value) } quote! { self.#ident.clone().map(#into_value).ok_or(#foundations::FieldAccessError::Unset) }
}; };
quote! { Fields::#enum_ident => #expr } quote! { Fields::#enum_ident => #expr }
@ -890,9 +890,9 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
let Field { enum_ident, ident, .. } = field; let Field { enum_ident, ident, .. } = field;
let expr = if field.required { let expr = if field.required {
quote! { Some(#into_value(self.#ident.clone())) } quote! { Ok(#into_value(self.#ident.clone())) }
} else if field.synthesized { } else if field.synthesized {
quote! { self.#ident.clone().map(#into_value) } quote! { self.#ident.clone().map(#into_value).ok_or(#foundations::FieldAccessError::Unset) }
} else { } else {
let value = create_style_chain_access( let value = create_style_chain_access(
field, field,
@ -900,7 +900,7 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
if field.ghost { quote!(None) } else { quote!(self.#ident.as_ref()) }, if field.ghost { quote!(None) } else { quote!(self.#ident.as_ref()) },
); );
quote! { Some(#into_value(#value)) } quote! { Ok(#into_value(#value)) }
}; };
quote! { Fields::#enum_ident => #expr } quote! { Fields::#enum_ident => #expr }
@ -911,10 +911,10 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
let Field { enum_ident, .. } = field; let Field { enum_ident, .. } = field;
let expr = if field.required || field.synthesized { let expr = if field.required || field.synthesized {
quote! { None } quote! { Err(#foundations::FieldAccessError::Unknown) }
} else { } else {
let value = create_style_chain_access(field, false, quote!(None)); let value = create_style_chain_access(field, false, quote!(None));
quote! { Some(#into_value(#value)) } quote! { Ok(#into_value(#value)) }
}; };
quote! { Fields::#enum_ident => #expr } quote! { Fields::#enum_ident => #expr }
@ -962,6 +962,10 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
let Elem { ident, .. } = element; let Elem { ident, .. } = element;
let result = quote! {
Result<#foundations::Value, #foundations::FieldAccessError>
};
quote! { quote! {
impl #foundations::Fields for #ident { impl #foundations::Fields for #ident {
type Enum = Fields; type Enum = Fields;
@ -977,27 +981,33 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
} }
} }
fn field(&self, id: u8) -> Option<#foundations::Value> { fn field(&self, id: u8) -> #result {
let id = Fields::try_from(id).ok()?; let id = Fields::try_from(id)?;
match id { match id {
#(#field_arms,)* #(#field_arms,)*
_ => None, // This arm might be reached if someone tries to access an
// internal field.
_ => Err(#foundations::FieldAccessError::Unknown),
} }
} }
fn field_with_styles(&self, id: u8, styles: #foundations::StyleChain) -> Option<#foundations::Value> { fn field_with_styles(&self, id: u8, styles: #foundations::StyleChain) -> #result {
let id = Fields::try_from(id).ok()?; let id = Fields::try_from(id)?;
match id { match id {
#(#field_with_styles_arms,)* #(#field_with_styles_arms,)*
_ => None, // This arm might be reached if someone tries to access an
// internal field.
_ => Err(#foundations::FieldAccessError::Unknown),
} }
} }
fn field_from_styles(id: u8, styles: #foundations::StyleChain) -> Option<#foundations::Value> { fn field_from_styles(id: u8, styles: #foundations::StyleChain) -> #result {
let id = Fields::try_from(id).ok()?; let id = Fields::try_from(id)?;
match id { match id {
#(#field_from_styles_arms,)* #(#field_from_styles_arms,)*
_ => None, // This arm might be reached if someone tries to access an
// internal field.
_ => Err(#foundations::FieldAccessError::Unknown),
} }
} }

View File

@ -315,7 +315,7 @@ impl Eval for ast::FieldAccess<'_> {
if let Some(element) = func.element(); if let Some(element) = func.element();
if let Some(id) = element.field_id(&field); if let Some(id) = element.field_id(&field);
let styles = vm.context.styles().at(field.span()); let styles = vm.context.styles().at(field.span());
if let Some(value) = element.field_from_styles( if let Ok(value) = element.field_from_styles(
id, id,
styles.as_ref().map(|&s| s).unwrap_or_default(), styles.as_ref().map(|&s| s).unwrap_or_default(),
); );

View File

@ -196,10 +196,14 @@ impl Content {
/// This is the preferred way to access fields. However, you can only use it /// This is the preferred way to access fields. However, you can only use it
/// if you have set the field IDs yourself or are using the field IDs /// if you have set the field IDs yourself or are using the field IDs
/// generated by the `#[elem]` macro. /// generated by the `#[elem]` macro.
pub fn get(&self, id: u8, styles: Option<StyleChain>) -> Option<Value> { pub fn get(
&self,
id: u8,
styles: Option<StyleChain>,
) -> Result<Value, FieldAccessError> {
if id == 255 { if id == 255 {
if let Some(label) = self.label() { if let Some(label) = self.label() {
return Some(label.into_value()); return Ok(label.into_value());
} }
} }
match styles { match styles {
@ -212,13 +216,13 @@ impl Content {
/// ///
/// If you have access to the field IDs of the element, use [`Self::get`] /// If you have access to the field IDs of the element, use [`Self::get`]
/// instead. /// instead.
pub fn get_by_name(&self, name: &str) -> Option<Value> { pub fn get_by_name(&self, name: &str) -> Result<Value, FieldAccessError> {
if name == "label" { if name == "label" {
if let Some(label) = self.label() { if let Some(label) = self.label() {
return Some(label.into_value()); return Ok(label.into_value());
} }
} }
let id = self.elem().field_id(name)?; let id = self.elem().field_id(name).ok_or(FieldAccessError::Unknown)?;
self.get(id, None) self.get(id, None)
} }
@ -229,7 +233,7 @@ impl Content {
/// generated by the `#[elem]` macro. /// generated by the `#[elem]` macro.
pub fn field(&self, id: u8) -> StrResult<Value> { pub fn field(&self, id: u8) -> StrResult<Value> {
self.get(id, None) self.get(id, None)
.ok_or_else(|| missing_field(self.elem().field_name(id).unwrap())) .map_err(|e| e.message(self, self.elem().field_name(id).unwrap()))
} }
/// Get a field by name, returning a missing field error if it does not /// Get a field by name, returning a missing field error if it does not
@ -238,7 +242,7 @@ impl Content {
/// If you have access to the field IDs of the element, use [`Self::field`] /// If you have access to the field IDs of the element, use [`Self::field`]
/// instead. /// instead.
pub fn field_by_name(&self, name: &str) -> StrResult<Value> { pub fn field_by_name(&self, name: &str) -> StrResult<Value> {
self.get_by_name(name).ok_or_else(|| missing_field(name)) self.get_by_name(name).map_err(|e| e.message(self, name))
} }
/// Resolve all fields with the styles and save them in-place. /// Resolve all fields with the styles and save them in-place.
@ -568,8 +572,8 @@ impl Content {
default: Option<Value>, default: Option<Value>,
) -> StrResult<Value> { ) -> StrResult<Value> {
self.get_by_name(&field) self.get_by_name(&field)
.or(default) .or_else(|e| default.ok_or(e))
.ok_or_else(|| missing_field_no_default(&field)) .map_err(|e| e.message_no_default(self, &field))
} }
/// Returns the fields of this content. /// Returns the fields of this content.
@ -968,18 +972,43 @@ pub trait PlainText {
fn plain_text(&self, text: &mut EcoString); fn plain_text(&self, text: &mut EcoString);
} }
/// The missing field access error message. /// An error arising when trying to access a field of content.
#[cold] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
fn missing_field(field: &str) -> EcoString { pub enum FieldAccessError {
eco_format!("content does not contain field {}", field.repr()) Unknown,
Unset,
Internal,
} }
/// The missing field access error message when no default value was given. impl FieldAccessError {
#[cold] /// Formats the error message given the content and the field name.
fn missing_field_no_default(field: &str) -> EcoString { #[cold]
eco_format!( pub fn message(self, content: &Content, field: &str) -> EcoString {
"content does not contain field {} and \ let elem_name = content.elem().name();
no default value was specified", match self {
field.repr() FieldAccessError::Unknown => {
) eco_format!("{elem_name} does not have field {}", field.repr())
}
FieldAccessError::Unset => {
eco_format!(
"field {} in {elem_name} is not known at this point",
field.repr()
)
}
FieldAccessError::Internal => {
eco_format!(
"internal error when accessing field {} in {elem_name} this is a bug",
field.repr()
)
}
}
}
/// Formats the error message for an `at` calls without a default value.
#[cold]
pub fn message_no_default(self, content: &Content, field: &str) -> EcoString {
let mut msg = self.message(content, field);
msg.push_str(" and no default was specified");
msg
}
} }

View File

@ -11,8 +11,8 @@ use smallvec::SmallVec;
use crate::diag::SourceResult; use crate::diag::SourceResult;
use crate::engine::Engine; use crate::engine::Engine;
use crate::foundations::{ use crate::foundations::{
cast, Args, Content, Dict, Func, ParamInfo, Repr, Scope, Selector, StyleChain, cast, Args, Content, Dict, FieldAccessError, Func, ParamInfo, Repr, Scope, Selector,
Styles, Value, StyleChain, Styles, Value,
}; };
use crate::text::{Lang, Region}; use crate::text::{Lang, Region};
use crate::utils::Static; use crate::utils::Static;
@ -123,8 +123,12 @@ impl Element {
(self.0.field_name)(id) (self.0.field_name)(id)
} }
/// Extract the field name for the given field ID. /// Extract the value of the field for the given field ID and style chain.
pub fn field_from_styles(&self, id: u8, styles: StyleChain) -> Option<Value> { pub fn field_from_styles(
&self,
id: u8,
styles: StyleChain,
) -> Result<Value, FieldAccessError> {
(self.0.field_from_styles)(id, styles) (self.0.field_from_styles)(id, styles)
} }
@ -223,13 +227,17 @@ pub trait Fields {
fn has(&self, id: u8) -> bool; fn has(&self, id: u8) -> bool;
/// Get the field with the given field ID. /// Get the field with the given field ID.
fn field(&self, id: u8) -> Option<Value>; fn field(&self, id: u8) -> Result<Value, FieldAccessError>;
/// Get the field with the given ID in the presence of styles. /// Get the field with the given ID in the presence of styles.
fn field_with_styles(&self, id: u8, styles: StyleChain) -> Option<Value>; fn field_with_styles(
&self,
id: u8,
styles: StyleChain,
) -> Result<Value, FieldAccessError>;
/// Get the field with the given ID from the styles. /// Get the field with the given ID from the styles.
fn field_from_styles(id: u8, styles: StyleChain) -> Option<Value> fn field_from_styles(id: u8, styles: StyleChain) -> Result<Value, FieldAccessError>
where where
Self: Sized; Self: Sized;
@ -282,7 +290,7 @@ pub struct NativeElementData {
/// Gets the name of a field by its numeric index. /// Gets the name of a field by its numeric index.
pub field_name: fn(u8) -> Option<&'static str>, pub field_name: fn(u8) -> Option<&'static str>,
/// Get the field with the given ID in the presence of styles (see [`Fields`]). /// Get the field with the given ID in the presence of styles (see [`Fields`]).
pub field_from_styles: fn(u8, StyleChain) -> Option<Value>, pub field_from_styles: fn(u8, StyleChain) -> Result<Value, FieldAccessError>,
/// Gets the localized name for this element (see [`LocalName`][crate::text::LocalName]). /// Gets the localized name for this element (see [`LocalName`][crate::text::LocalName]).
pub local_name: Option<fn(Lang, Option<Region>) -> &'static str>, pub local_name: Option<fn(Lang, Option<Region>) -> &'static str>,
pub scope: Lazy<Scope>, pub scope: Lazy<Scope>,

View File

@ -128,7 +128,7 @@ impl Selector {
Self::Elem(element, dict) => { Self::Elem(element, dict) => {
target.func() == *element target.func() == *element
&& dict.iter().flat_map(|dict| dict.iter()).all(|(id, value)| { && dict.iter().flat_map(|dict| dict.iter()).all(|(id, value)| {
target.get(*id, styles).as_ref() == Some(value) target.get(*id, styles).as_ref().ok() == Some(value)
}) })
} }
Self::Label(label) => target.label() == Some(*label), Self::Label(label) => target.label() == Some(*label),

View File

@ -13,7 +13,7 @@
- C - C
--- content-field-missing --- --- content-field-missing ---
// Error: 25-28 content does not contain field "fun" // Error: 25-28 heading does not have field "fun"
#show heading: it => it.fun #show heading: it => it.fun
= A = A
@ -118,3 +118,15 @@
} }
= Hello, world! <my-label> = Hello, world! <my-label>
--- content-fields-unset ---
// Error: 10-15 field "block" in raw is not known at this point
#raw("").block
--- content-fields-unset-no-default ---
// Error: 2-21 field "block" in raw is not known at this point and no default was specified
#raw("").at("block")
--- content-try-to-access-internal-field ---
// Error: 9-15 hide does not have field "hidden"
#hide[].hidden

View File

@ -78,7 +78,7 @@ Another text.
= Heading = Heading
--- show-unknown-field --- --- show-unknown-field ---
// Error: 25-29 content does not contain field "page" // Error: 25-29 heading does not have field "page"
#show heading: it => it.page #show heading: it => it.page
= Heading = Heading