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

Co-authored-by: Laurenz <laurmaedje@gmail.com>

10 files changed, 114 insertions, 54 deletions

diff --git a/crates/typst-cli/src/query.rs b/crates/typst-cli/src/query.rs
index a8012777..ef3c7951 100644
--- a/crates/typst-cli/src/query.rs
+++ b/crates/typst-cli/src/query.rs
@@ -88,7 +88,7 @@ fn format(elements: Vec<Content>, command: &QueryCommand) -> StrResult<String> {
     let mapped: Vec<_> = elements
         .into_iter()
         .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()),
         })
         .collect();
diff --git a/crates/typst-ide/src/analyze.rs b/crates/typst-ide/src/analyze.rs
index b43d87cd..ecb73dce 100644
--- a/crates/typst-ide/src/analyze.rs
+++ b/crates/typst-ide/src/analyze.rs
@@ -92,7 +92,8 @@ pub fn analyze_labels(document: &Document) -> (Vec<(Label, Option<EcoString>)>,
         let Some(label) = elem.label() else { continue };
         let details = elem
             .get_by_name("caption")
-            .or_else(|| elem.get_by_name("body"))
+            .or_else(|_| elem.get_by_name("body"))
+            .ok()
             .and_then(|field| match field {
                 Value::Content(content) => Some(content),
                 _ => None,
diff --git a/crates/typst-ide/src/complete.rs b/crates/typst-ide/src/complete.rs
index 608b1473..cdcac956 100644
--- a/crates/typst-ide/src/complete.rs
+++ b/crates/typst-ide/src/complete.rs
@@ -441,7 +441,7 @@ fn field_access_completions(
             if let Some((elem, styles)) = func.element().zip(styles.as_ref()) {
                 for param in elem.params().iter().filter(|param| !param.required) {
                     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(
                             Some(param.name.into()),
diff --git a/crates/typst-macros/src/elem.rs b/crates/typst-macros/src/elem.rs
index 35dc8b75..a3666e1d 100644
--- a/crates/typst-macros/src/elem.rs
+++ b/crates/typst-macros/src/elem.rs
@@ -390,13 +390,13 @@ fn create_fields_enum(element: &Elem) -> TokenStream {
         }
 
         impl ::std::convert::TryFrom<u8> for Fields {
-            type Error = ();
+            type Error = #foundations::FieldAccessError;
 
             fn try_from(value: u8) -> Result<Self, Self::Error> {
                 #(const #consts: u8 = Fields::#variants as u8;)*
                 match value {
                     #(#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 expr = if field.required {
-            quote! { Some(#into_value(self.#ident.clone())) }
+            quote! { Ok(#into_value(self.#ident.clone())) }
         } 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 }
@@ -890,9 +890,9 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
         let Field { enum_ident, ident, .. } = field;
 
         let expr = if field.required {
-            quote! { Some(#into_value(self.#ident.clone())) }
+            quote! { Ok(#into_value(self.#ident.clone())) }
         } else if field.synthesized {
-            quote! { self.#ident.clone().map(#into_value) }
+            quote! { self.#ident.clone().map(#into_value).ok_or(#foundations::FieldAccessError::Unset) }
         } else {
             let value = create_style_chain_access(
                 field,
@@ -900,7 +900,7 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
                 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 }
@@ -911,10 +911,10 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
         let Field { enum_ident, .. } = field;
 
         let expr = if field.required || field.synthesized {
-            quote! { None }
+            quote! { Err(#foundations::FieldAccessError::Unknown) }
         } else {
             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 }
@@ -962,6 +962,10 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
 
     let Elem { ident, .. } = element;
 
+    let result = quote! {
+        Result<#foundations::Value, #foundations::FieldAccessError>
+    };
+
     quote! {
         impl #foundations::Fields for #ident {
             type Enum = Fields;
@@ -977,27 +981,33 @@ fn create_fields_impl(element: &Elem) -> TokenStream {
                 }
             }
 
-            fn field(&self, id: u8) -> Option<#foundations::Value> {
-                let id = Fields::try_from(id).ok()?;
+            fn field(&self, id: u8) -> #result {
+                let id = Fields::try_from(id)?;
                 match id {
                     #(#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> {
-                let id = Fields::try_from(id).ok()?;
+            fn field_with_styles(&self, id: u8, styles: #foundations::StyleChain) -> #result {
+                let id = Fields::try_from(id)?;
                 match id {
                     #(#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> {
-                let id = Fields::try_from(id).ok()?;
+            fn field_from_styles(id: u8, styles: #foundations::StyleChain) -> #result {
+                let id = Fields::try_from(id)?;
                 match id {
                     #(#field_from_styles_arms,)*
-                    _ => None,
+                    // This arm might be reached if someone tries to access an
+                    // internal field.
+                    _ => Err(#foundations::FieldAccessError::Unknown),
                 }
             }
 
diff --git a/crates/typst/src/eval/code.rs b/crates/typst/src/eval/code.rs
index 8542c846..dcbc5b69 100644
--- a/crates/typst/src/eval/code.rs
+++ b/crates/typst/src/eval/code.rs
@@ -315,7 +315,7 @@ impl Eval for ast::FieldAccess<'_> {
             if let Some(element) = func.element();
             if let Some(id) = element.field_id(&field);
             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,
                 styles.as_ref().map(|&s| s).unwrap_or_default(),
             );
diff --git a/crates/typst/src/foundations/content.rs b/crates/typst/src/foundations/content.rs
index 5a22734b..c3ab3851 100644
--- a/crates/typst/src/foundations/content.rs
+++ b/crates/typst/src/foundations/content.rs
@@ -196,10 +196,14 @@ impl Content {
     /// 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
     /// 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 let Some(label) = self.label() {
-                return Some(label.into_value());
+                return Ok(label.into_value());
             }
         }
         match styles {
@@ -212,13 +216,13 @@ impl Content {
     ///
     /// If you have access to the field IDs of the element, use [`Self::get`]
     /// 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 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)
     }
 
@@ -229,7 +233,7 @@ impl Content {
     /// generated by the `#[elem]` macro.
     pub fn field(&self, id: u8) -> StrResult<Value> {
         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
@@ -238,7 +242,7 @@ impl Content {
     /// If you have access to the field IDs of the element, use [`Self::field`]
     /// instead.
     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.
@@ -568,8 +572,8 @@ impl Content {
         default: Option<Value>,
     ) -> StrResult<Value> {
         self.get_by_name(&field)
-            .or(default)
-            .ok_or_else(|| missing_field_no_default(&field))
+            .or_else(|e| default.ok_or(e))
+            .map_err(|e| e.message_no_default(self, &field))
     }
 
     /// Returns the fields of this content.
@@ -968,18 +972,43 @@ pub trait PlainText {
     fn plain_text(&self, text: &mut EcoString);
 }
 
-/// The missing field access error message.
-#[cold]
-fn missing_field(field: &str) -> EcoString {
-    eco_format!("content does not contain field {}", field.repr())
+/// An error arising when trying to access a field of content.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+pub enum FieldAccessError {
+    Unknown,
+    Unset,
+    Internal,
 }
 
-/// The missing field access error message when no default value was given.
-#[cold]
-fn missing_field_no_default(field: &str) -> EcoString {
-    eco_format!(
-        "content does not contain field {} and \
-         no default value was specified",
-        field.repr()
-    )
+impl FieldAccessError {
+    /// Formats the error message given the content and the field name.
+    #[cold]
+    pub fn message(self, content: &Content, field: &str) -> EcoString {
+        let elem_name = content.elem().name();
+        match self {
+            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
+    }
 }
diff --git a/crates/typst/src/foundations/element.rs b/crates/typst/src/foundations/element.rs
index 6ffeadb9..e578d209 100644
--- a/crates/typst/src/foundations/element.rs
+++ b/crates/typst/src/foundations/element.rs
@@ -11,8 +11,8 @@ use smallvec::SmallVec;
 use crate::diag::SourceResult;
 use crate::engine::Engine;
 use crate::foundations::{
-    cast, Args, Content, Dict, Func, ParamInfo, Repr, Scope, Selector, StyleChain,
-    Styles, Value,
+    cast, Args, Content, Dict, FieldAccessError, Func, ParamInfo, Repr, Scope, Selector,
+    StyleChain, Styles, Value,
 };
 use crate::text::{Lang, Region};
 use crate::utils::Static;
@@ -123,8 +123,12 @@ impl Element {
         (self.0.field_name)(id)
     }
 
-    /// Extract the field name for the given field ID.
-    pub fn field_from_styles(&self, id: u8, styles: StyleChain) -> Option<Value> {
+    /// Extract the value of the field for the given field ID and style chain.
+    pub fn field_from_styles(
+        &self,
+        id: u8,
+        styles: StyleChain,
+    ) -> Result<Value, FieldAccessError> {
         (self.0.field_from_styles)(id, styles)
     }
 
@@ -223,13 +227,17 @@ pub trait Fields {
     fn has(&self, id: u8) -> bool;
 
     /// 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.
-    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.
-    fn field_from_styles(id: u8, styles: StyleChain) -> Option<Value>
+    fn field_from_styles(id: u8, styles: StyleChain) -> Result<Value, FieldAccessError>
     where
         Self: Sized;
 
@@ -282,7 +290,7 @@ pub struct NativeElementData {
     /// Gets the name of a field by its numeric index.
     pub field_name: fn(u8) -> Option<&'static str>,
     /// 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]).
     pub local_name: Option<fn(Lang, Option<Region>) -> &'static str>,
     pub scope: Lazy<Scope>,
diff --git a/crates/typst/src/foundations/selector.rs b/crates/typst/src/foundations/selector.rs
index 6802a150..55fd9555 100644
--- a/crates/typst/src/foundations/selector.rs
+++ b/crates/typst/src/foundations/selector.rs
@@ -128,7 +128,7 @@ impl Selector {
             Self::Elem(element, dict) => {
                 target.func() == *element
                     && 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),
diff --git a/tests/suite/foundations/content.typ b/tests/suite/foundations/content.typ
index afecc124..31ef1c54 100644
--- a/tests/suite/foundations/content.typ
+++ b/tests/suite/foundations/content.typ
@@ -13,7 +13,7 @@
 - C
 
 --- 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
 = A
 
@@ -118,3 +118,15 @@
 }
 
 = 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
diff --git a/tests/suite/styling/show.typ b/tests/suite/styling/show.typ
index aa121bff..05f545c6 100644
--- a/tests/suite/styling/show.typ
+++ b/tests/suite/styling/show.typ
@@ -78,7 +78,7 @@ Another text.
 = Heading
 
 --- 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
 = Heading