From 5aaaacbf472fc71295d4e4a26615d941a4d92a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20d=27Herbais=20de=20Thun?= Date: Fri, 17 Nov 2023 10:39:08 +0100 Subject: [PATCH] Allow `elem` synthesized fields to take a default value (#2687) --- crates/typst-library/src/meta/figure.rs | 4 ++ crates/typst-library/src/meta/footnote.rs | 7 ++- crates/typst-library/src/meta/outline.rs | 9 ++- crates/typst-macros/src/elem.rs | 74 +++++++++++++++++------ tests/typ/bugs/subelement-panic.typ | 40 ++++++++++++ 5 files changed, 115 insertions(+), 19 deletions(-) create mode 100644 tests/typ/bugs/subelement-panic.typ diff --git a/crates/typst-library/src/meta/figure.rs b/crates/typst-library/src/meta/figure.rs index 6124e9d5e..8e08b9402 100644 --- a/crates/typst-library/src/meta/figure.rs +++ b/crates/typst-library/src/meta/figure.rs @@ -484,19 +484,23 @@ pub struct FigureCaption { /// The figure's supplement. #[synthesized] + #[default(None)] pub supplement: Option, /// How to number the figure. #[synthesized] + #[default(None)] pub numbering: Option, /// The counter for the figure. #[synthesized] + #[default(None)] pub counter: Option, /// The figure's location. #[internal] #[synthesized] + #[default(None)] pub figure_location: Option, } diff --git a/crates/typst-library/src/meta/footnote.rs b/crates/typst-library/src/meta/footnote.rs index 4306f8333..c7bed9090 100644 --- a/crates/typst-library/src/meta/footnote.rs +++ b/crates/typst-library/src/meta/footnote.rs @@ -270,7 +270,12 @@ impl Show for FootnoteEntry { let default = StyleChain::default(); let numbering = note.numbering(default); let counter = Counter::of(FootnoteElem::elem()); - let loc = note.location().unwrap(); + let Some(loc) = note.location() else { + bail!(error!(self.span(), "footnote entry must have a location").with_hint( + "try using a query or a show rule to customize the footnote instead" + )) + }; + let num = counter.at(vt, loc)?.display(vt, numbering)?; let sup = SuperElem::new(num) .pack() diff --git a/crates/typst-library/src/meta/outline.rs b/crates/typst-library/src/meta/outline.rs index 0cae0de42..b3b97087b 100644 --- a/crates/typst-library/src/meta/outline.rs +++ b/crates/typst-library/src/meta/outline.rs @@ -489,7 +489,14 @@ impl Show for OutlineEntry { // In case a user constructs an outline entry with an arbitrary element. let Some(location) = elem.location() else { - bail!(self.span(), "cannot outline {}", elem.func().name()) + if elem.can::() && elem.can::() { + bail!(error!(self.span(), "{} must have a location", elem.func().name()) + .with_hint( + "try using a query or a show rule to customize the outline.entry instead", + )) + } else { + bail!(self.span(), "cannot outline {}", elem.func().name()) + } }; // The body text remains overridable. diff --git a/crates/typst-macros/src/elem.rs b/crates/typst-macros/src/elem.rs index 512cbc844..46ff55c6f 100644 --- a/crates/typst-macros/src/elem.rs +++ b/crates/typst-macros/src/elem.rs @@ -129,7 +129,7 @@ struct Field { borrowed: bool, forced_variant: Option, parse: Option, - default: syn::Expr, + default: Option, } impl Field { @@ -229,9 +229,7 @@ fn parse_field(field: &syn::Field) -> Result { fold: has_attr(&mut attrs, "fold"), resolve: has_attr(&mut attrs, "resolve"), parse: parse_attr(&mut attrs, "parse")?.flatten(), - default: parse_attr::(&mut attrs, "default")? - .flatten() - .unwrap_or_else(|| parse_quote! { ::std::default::Default::default() }), + default: parse_attr::(&mut attrs, "default")?.flatten(), vis: field.vis.clone(), ident: ident.clone(), ident_in: Ident::new(&format!("{}_in", ident), ident.span()), @@ -344,11 +342,17 @@ fn create(element: &Elem) -> TokenStream { /// Create a field declaration. fn create_field(field: &Field) -> TokenStream { - let Field { ident, ty, docs, required, .. } = field; + let Field { + ident, ty, docs, required, synthesized, default, .. + } = field; - let ty = required - .then(|| quote! { #ty }) - .unwrap_or_else(|| quote! { Option<#ty> }); + let ty = required.then(|| quote! { #ty }).unwrap_or_else(|| { + if *synthesized && default.is_some() { + quote! { #ty } + } else { + quote! { ::std::option::Option<#ty> } + } + }); quote! { #[doc = #docs] #ident: #ty @@ -457,9 +461,18 @@ fn create_new_func(element: &Elem) -> TokenStream { let defaults = element .fields .iter() - .filter(|field| !field.external && (field.synthesized || !field.inherent())) - .map(|Field { ident, .. }| { - quote! { #ident: None } + .filter(|field| !field.external && !field.inherent() && !field.synthesized) + .map(|Field { ident, .. }| quote! { #ident: None }); + let default_synthesized = element + .fields + .iter() + .filter(|field| !field.external && field.synthesized) + .map(|Field { ident, default, .. }| { + if let Some(expr) = default { + quote! { #ident: #expr } + } else { + quote! { #ident: None } + } }); let label_and_location = element.unless_capability("Unlabellable", || { @@ -479,6 +492,7 @@ fn create_new_func(element: &Elem) -> TokenStream { guards: ::std::vec::Vec::with_capacity(0), #(#required,)* #(#defaults,)* + #(#default_synthesized,)* } } } @@ -486,10 +500,19 @@ fn create_new_func(element: &Elem) -> TokenStream { /// Create a builder pattern method for a field. fn create_with_field_method(field: &Field) -> TokenStream { - let Field { vis, ident, with_ident, name, ty, .. } = field; + let Field { + vis, + ident, + with_ident, + name, + ty, + synthesized, + default, + .. + } = field; let doc = format!("Set the [`{}`](Self::{}) field.", name, ident); - let set = if field.inherent() { + let set = if field.inherent() || (*synthesized && default.is_some()) { quote! { self.#ident = #ident; } } else { quote! { self.#ident = Some(#ident); } @@ -505,9 +528,19 @@ fn create_with_field_method(field: &Field) -> TokenStream { /// Create a set-style method for a field. fn create_push_field_method(field: &Field) -> TokenStream { - let Field { vis, ident, push_ident, name, ty, .. } = field; + let Field { + vis, + ident, + push_ident, + name, + ty, + synthesized, + default, + .. + } = field; let doc = format!("Push the [`{}`](Self::{}) field.", name, ident); - let set = if field.inherent() && !field.synthesized { + let set = if (field.inherent() && !synthesized) || (*synthesized && default.is_some()) + { quote! { self.#ident = #ident; } } else { quote! { self.#ident = Some(#ident); } @@ -561,10 +594,11 @@ fn create_field_in_method(element: &Elem, field: &Field) -> TokenStream { /// Create an accessor methods for a field. fn create_field_method(element: &Elem, field: &Field) -> TokenStream { let Field { vis, docs, ident, output, .. } = field; - if field.inherent() && !field.synthesized { + if (field.inherent() && !field.synthesized) + || (field.synthesized && field.default.is_some()) + { quote! { #[doc = #docs] - #[track_caller] #vis fn #ident(&self) -> &#output { &self.#ident } @@ -618,6 +652,9 @@ fn create_style_chain_access( (true, true, _) => quote! { get_resolve_fold }, }; + let default = default + .clone() + .unwrap_or_else(|| parse_quote! { ::std::default::Default::default() }); let (init, default) = field.fold.then(|| (None, quote! { || #default })).unwrap_or_else(|| ( Some(quote! { static DEFAULT: ::once_cell::sync::Lazy<#ty> = ::once_cell::sync::Lazy::new(|| #default); @@ -1141,6 +1178,9 @@ fn create_param_info(field: &Field) -> TokenStream { let settable = field.settable(); let default_ty = if *fold { &output } else { &ty }; let default = quote_option(&settable.then(|| { + let default = default + .clone() + .unwrap_or_else(|| parse_quote! { ::std::default::Default::default() }); quote! { || { let typed: #default_ty = #default; diff --git a/tests/typ/bugs/subelement-panic.typ b/tests/typ/bugs/subelement-panic.typ new file mode 100644 index 000000000..fcad83bc3 --- /dev/null +++ b/tests/typ/bugs/subelement-panic.typ @@ -0,0 +1,40 @@ +// Test that figure captions don't cause panics. +// Ref: false + +--- +// #2530 +#figure(caption: [test])[].caption + +--- +// #2165 +#figure.caption[] + +--- +// #2328 +// Error: 4-43 footnote entry must have a location +// Hint: 4-43 try using a query or a show rule to customize the footnote instead +HI#footnote.entry(clearance: 2.5em)[There] + +--- +// Enum item (pre-emptive) +#enum.item(none)[Hello] +#enum.item(17)[Hello] + +--- +// List item (pre-emptive) +#list.item[Hello] + +--- +// Term item (pre-emptive) +#terms.item[Hello][World!] + +--- +// Outline entry (pre-emptive) +// Error: 2-48 cannot outline text +#outline.entry(1, [Hello], [World!], none, [1]) + +--- +// Outline entry (pre-emptive, improved error) +// Error: 2-55 heading must have a location +// Hint: 2-55 try using a query or a show rule to customize the outline.entry instead +#outline.entry(1, heading[Hello], [World!], none, [1])