From 7ec487af806bef9d0b517ffc6da9d363397fee29 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 22 May 2024 15:48:27 -0400 Subject: [PATCH 1/5] Adds WPContextualOption --- wp_contextual/src/lib.rs | 5 +- wp_contextual/src/wp_contextual.rs | 55 +++++++++++++++---- wp_contextual/tests/all_tests.rs | 2 + .../tests/basic_wp_contextual_option.rs | 14 +++++ ...ntextual_field_and_wp_contextual_option.rs | 13 +++++ ...tual_field_and_wp_contextual_option.stderr | 5 ++ 6 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 wp_contextual/tests/basic_wp_contextual_option.rs create mode 100644 wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.rs create mode 100644 wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.stderr diff --git a/wp_contextual/src/lib.rs b/wp_contextual/src/lib.rs index 465531549..967fdca94 100644 --- a/wp_contextual/src/lib.rs +++ b/wp_contextual/src/lib.rs @@ -320,7 +320,10 @@ mod wp_contextual; /// * Notice that `BazWithEmbedContext` & `BazWithViewContext` types weren't generated since /// they wouldn't have any fields. /// * Notice the type for `qux: Vec` was preserved as this wasn't an `Option` type. -#[proc_macro_derive(WPContextual, attributes(WPContext, WPContextualField))] +#[proc_macro_derive( + WPContextual, + attributes(WPContext, WPContextualField, WPContextualOption) +)] pub fn derive(input: TokenStream) -> TokenStream { wp_contextual::wp_contextual(parse_macro_input!(input)) .unwrap_or_else(|err| err.into_compile_error().into()) diff --git a/wp_contextual/src/wp_contextual.rs b/wp_contextual/src/wp_contextual.rs index 8b3714fd2..b2331297a 100644 --- a/wp_contextual/src/wp_contextual.rs +++ b/wp_contextual/src/wp_contextual.rs @@ -6,7 +6,6 @@ use syn::{spanned::Spanned, DeriveInput, Ident}; const IDENT_PREFIX: &str = "Sparse"; -// TODO: Public documentation pub fn wp_contextual(ast: DeriveInput) -> Result { let original_ident = &ast.ident; let original_ident_name = original_ident.to_string(); @@ -98,6 +97,9 @@ fn parse_fields( if is_wp_contextual_field_ident(segment_ident) { return Ok(WPParsedAttr::ParsedWPContextualField); } + if is_wp_contextual_option_ident(segment_ident) { + return Ok(WPParsedAttr::ParsedWPContextualOption); + } if is_wp_context_ident(segment_ident) { if let syn::Meta::List(meta_list) = &attr.meta { let contexts = parse_contexts_from_tokens(meta_list.tokens.clone())?; @@ -136,6 +138,21 @@ fn parse_fields( return Err(WPContextualParseError::WPContextualFieldWithoutWPContext .into_syn_error(pf.field.span())); }; + // Check if there are any fields that has both #[WPContextualField] & #[WPContextualOption] + // attributes and return an error. These attributes are incompatible with each other because + // #[WPContextualOption] will leave the type as is, whereas #[WPContextualField] will modify it + if let Some(pf) = parsed_fields.iter().find(|pf| { + pf.parsed_attrs + .contains(&WPParsedAttr::ParsedWPContextualField) + && pf + .parsed_attrs + .contains(&WPParsedAttr::ParsedWPContextualOption) + }) { + return Err( + WPContextualParseError::WPContextualBothOptionAndField.into_syn_error(pf.field.span()) + ); + } + Ok(parsed_fields) } @@ -161,16 +178,25 @@ fn generate_context_fields( }) .map(|pf| { let f = &pf.field; - let mut new_type = extract_inner_type_of_option(&f.ty).unwrap_or(f.ty.clone()); - if f.attrs.iter().any(|attr| { - attr.path() - .segments - .iter() - .any(|s| is_wp_contextual_field_ident(&s.ident)) - }) { - // If the field has #[WPContextualField] attr, map it to its contextual field type - new_type = contextual_field_type(&new_type, context)?; - } + + let new_type = if pf + .parsed_attrs + .contains(&WPParsedAttr::ParsedWPContextualOption) + { + f.ty.clone() + } else { + let mut new_type = extract_inner_type_of_option(&f.ty).unwrap_or(f.ty.clone()); + if f.attrs.iter().any(|attr| { + attr.path() + .segments + .iter() + .any(|s| is_wp_contextual_field_ident(&s.ident)) + }) { + // If the field has #[WPContextualField] attr, map it to its contextual field type + new_type = contextual_field_type(&new_type, context)?; + } + new_type + }; Ok::(syn::Field { // Remove the WPContext & WPContextualField attributes from the generated field attrs: pf @@ -383,6 +409,10 @@ fn is_wp_contextual_field_ident(ident: &Ident) -> bool { ident.to_string().eq("WPContextualField") } +fn is_wp_contextual_option_ident(ident: &Ident) -> bool { + ident.to_string().eq("WPContextualOption") +} + // ``` // #[WPContextual] // pub struct SparseFoo { @@ -431,6 +461,7 @@ struct WPParsedField { #[derive(Debug, PartialEq, Eq)] enum WPParsedAttr { ParsedWPContextualField, + ParsedWPContextualOption, ParsedWPContext { contexts: Vec }, ExternalAttr { attr: syn::Attribute }, } @@ -488,6 +519,8 @@ enum WPContextualParseError { "WPContextual didn't generate anything. Did you forget to add #[WPContext] attribute?" )] EmptyResult, + #[error("#[WPContextualField] & #[WPContextualOption] can't be used together")] + WPContextualBothOptionAndField, #[error( "WPContextualField field types need to start with '{}' prefix", IDENT_PREFIX diff --git a/wp_contextual/tests/all_tests.rs b/wp_contextual/tests/all_tests.rs index 8633941b8..076fdd871 100644 --- a/wp_contextual/tests/all_tests.rs +++ b/wp_contextual/tests/all_tests.rs @@ -3,8 +3,10 @@ fn tests() { let t = trybuild::TestCases::new(); t.pass("tests/basic_wp_contextual.rs"); t.pass("tests/basic_wp_contextual_field.rs"); + t.pass("tests/basic_wp_contextual_option.rs"); t.pass("tests/wp_contextual_field_with_multiple_segments.rs"); t.pass("tests/wp_contextual_field_with_inner_type.rs"); + t.compile_fail("tests/error_both_wp_contextual_field_and_wp_contextual_option.rs"); t.compile_fail("tests/error_missing_sparse_prefix_from_wp_contextual.rs"); t.compile_fail("tests/error_missing_sparse_prefix_from_wp_contextual_field.rs"); t.compile_fail("tests/error_empty_result.rs"); diff --git a/wp_contextual/tests/basic_wp_contextual_option.rs b/wp_contextual/tests/basic_wp_contextual_option.rs new file mode 100644 index 000000000..f8d84083c --- /dev/null +++ b/wp_contextual/tests/basic_wp_contextual_option.rs @@ -0,0 +1,14 @@ +use wp_contextual::WPContextual; + +#[derive(WPContextual)] +pub struct SparseFoo { + #[WPContext(edit)] + #[WPContextualOption] + pub bar: Option, +} + +fn main() { + let _ = FooWithEditContext { bar: None }; +} + +uniffi::setup_scaffolding!(); diff --git a/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.rs b/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.rs new file mode 100644 index 000000000..8921c3bdc --- /dev/null +++ b/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.rs @@ -0,0 +1,13 @@ +use wp_contextual::WPContextual; + +#[derive(WPContextual)] +pub struct SparseFoo { + #[WPContext(edit)] + #[WPContextualField] + #[WPContextualOption] + pub bar: Option, +} + +fn main() {} + +uniffi::setup_scaffolding!(); diff --git a/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.stderr b/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.stderr new file mode 100644 index 000000000..e6bd3785e --- /dev/null +++ b/wp_contextual/tests/error_both_wp_contextual_field_and_wp_contextual_option.stderr @@ -0,0 +1,5 @@ +error: #[WPContextualField] & #[WPContextualOption] can't be used together + --> tests/error_both_wp_contextual_field_and_wp_contextual_option.rs:5:5 + | +5 | #[WPContext(edit)] + | ^ From cfa19b2a938ed9b80080d0ae662c557d09de0869 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 22 May 2024 16:11:55 -0400 Subject: [PATCH 2/5] Document WPContextualOption --- wp_contextual/src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/wp_contextual/src/lib.rs b/wp_contextual/src/lib.rs index 967fdca94..b4c5e40e7 100644 --- a/wp_contextual/src/lib.rs +++ b/wp_contextual/src/lib.rs @@ -239,6 +239,33 @@ //! `PostContentWithViewContext` which are the types that we used in our manually typed //! example. //! +//! --- +//! +//! In some instances, we might need a field to be included in contextual types, but still be an +//! `Option`. In these cases, `WPContextualOption` attribute can be used: +//! +//! ``` +//! # use wp_contextual::WPContextual; +//! #[derive(WPContextual)] +//! pub struct SparseUser { +//! #[WPContext(edit)] +//! #[WPContextualOption] +//! pub avatar_urls: Option>, +//! } +//! # // We need these 2 lines for UniFFI +//! # uniffi::setup_scaffolding!(); +//! # fn main() {} +//! ``` +//! +//! This will generate the following: +//! +//! ``` +//! pub struct UserWithEditContext { +//! pub avatar_urls: Option>, +//! } +//! ``` +//! --- +//! //! Please see the documentation for [`WPContextual`] for technical details. use proc_macro::TokenStream; use syn::parse_macro_input; @@ -255,6 +282,7 @@ mod wp_contextual; /// [`WPContextual`] type. This will tell the compiler to replace the given `Option` /// type with the appropriate contextual type: `BazWithEditContext`, `BazWithEmbedContext` or /// `BazWithViewContext`. +/// * `[WPContextualOption]` is used to tell the compiler to keep the field as an `Option`. /// * Generated types will have the following derive macros: /// `#[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)]`. These types are meant /// to be used for the @@ -277,6 +305,9 @@ mod wp_contextual; /// #[WPContext(edit)] /// #[WPContextualField] /// pub baz: Option, +/// #[WPContext(view)] +/// #[WPContextualOption] +/// pub foo_bar: Option, /// } /// /// #[derive(WPContextual)] @@ -306,6 +337,7 @@ mod wp_contextual; /// #[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)] /// pub struct FooWithViewContext { /// pub bar: u32, +/// pub foo_bar: Option /// } /// #[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)] /// pub struct BazWithEditContext { @@ -320,6 +352,8 @@ mod wp_contextual; /// * Notice that `BazWithEmbedContext` & `BazWithViewContext` types weren't generated since /// they wouldn't have any fields. /// * Notice the type for `qux: Vec` was preserved as this wasn't an `Option` type. +/// * Notice the type for `foo_bar: Option` was preserved since it's marked with +/// `#[WPContextualOption]`. #[proc_macro_derive( WPContextual, attributes(WPContext, WPContextualField, WPContextualOption) From bcd1e2f36ba0320a0eda0ec9f52a2fa19d422f43 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 22 May 2024 16:27:17 -0400 Subject: [PATCH 3/5] Ensure #[WPContextualOption] is only used with #[WPContext] --- wp_contextual/src/wp_contextual.rs | 52 +++++++++++++------ wp_contextual/tests/all_tests.rs | 1 + ...wp_contextual_option_without_wp_context.rs | 11 ++++ ...ontextual_option_without_wp_context.stderr | 5 ++ 4 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 wp_contextual/tests/error_wp_contextual_option_without_wp_context.rs create mode 100644 wp_contextual/tests/error_wp_contextual_option_without_wp_context.stderr diff --git a/wp_contextual/src/wp_contextual.rs b/wp_contextual/src/wp_contextual.rs index b2331297a..9bbcdd66d 100644 --- a/wp_contextual/src/wp_contextual.rs +++ b/wp_contextual/src/wp_contextual.rs @@ -71,6 +71,10 @@ fn struct_fields( // contexts. // * `WPContextualParseError::WPContextualFieldWithoutWPContext`: #[WPContextualField] is added to // a field that doesn't have the #[WPContext] attribute. +// * `WPContextualParseError::WPContextualOptionWithoutWPContext`: #[WPContextualOption] is added to +// a field that doesn't have the #[WPContext] attribute. +// * `WPContextualParseError::WPContextualBothOptionAndField`: #[WPContextualField] and +// #[WPContextualOption] was used together. // // It'll also handle incorrectly formatted #[WPContext] attribute through // `parse_contexts_from_tokens` helper. @@ -120,24 +124,38 @@ fn parse_fields( }) .collect::, syn::Error>>()?; + let assert_has_wp_context_attribute_if_it_has_given_attribute = + |attribute_to_check: WPParsedAttr, error_type: WPContextualParseError| { + if let Some(pf) = parsed_fields + .iter() + .filter(|pf| pf.parsed_attrs.contains(&attribute_to_check)) + .find(|pf| { + !pf.parsed_attrs.iter().any(|pf| match pf { + WPParsedAttr::ParsedWPContext { contexts } => !contexts.is_empty(), + _ => false, + }) + }) + { + Err(error_type.into_syn_error(pf.field.span())) + } else { + Ok(()) + } + }; + // Check if there are any fields that has #[WPContextualField] attribute, // but not the #[WPContext] attribute - if let Some(pf) = parsed_fields - .iter() - .filter(|pf| { - pf.parsed_attrs - .contains(&WPParsedAttr::ParsedWPContextualField) - }) - .find(|pf| { - !pf.parsed_attrs.iter().any(|pf| match pf { - WPParsedAttr::ParsedWPContext { contexts } => !contexts.is_empty(), - _ => false, - }) - }) - { - return Err(WPContextualParseError::WPContextualFieldWithoutWPContext - .into_syn_error(pf.field.span())); - }; + assert_has_wp_context_attribute_if_it_has_given_attribute( + WPParsedAttr::ParsedWPContextualField, + WPContextualParseError::WPContextualFieldWithoutWPContext, + )?; + + // Check if there are any fields that has #[WPContextualField] attribute, + // but not the #[WPContext] attribute + assert_has_wp_context_attribute_if_it_has_given_attribute( + WPParsedAttr::ParsedWPContextualOption, + WPContextualParseError::WPContextualOptionWithoutWPContext, + )?; + // Check if there are any fields that has both #[WPContextualField] & #[WPContextualOption] // attributes and return an error. These attributes are incompatible with each other because // #[WPContextualOption] will leave the type as is, whereas #[WPContextualField] will modify it @@ -534,6 +552,8 @@ enum WPContextualParseError { WPContextualMissingSparsePrefix, #[error("#[WPContextual] is only implemented for Structs")] WPContextualNotAStruct, + #[error("#[WPContextualOption] doesn't have any contexts. Did you forget to add #[WPContext] attribute?")] + WPContextualOptionWithoutWPContext, } impl WPContextualParseError { diff --git a/wp_contextual/tests/all_tests.rs b/wp_contextual/tests/all_tests.rs index 076fdd871..4a4a0cd57 100644 --- a/wp_contextual/tests/all_tests.rs +++ b/wp_contextual/tests/all_tests.rs @@ -17,5 +17,6 @@ fn tests() { t.compile_fail("tests/error_unexpected_wp_context_literal.rs"); t.compile_fail("tests/error_unexpected_wp_context_token.rs"); t.compile_fail("tests/error_wp_contextual_field_without_wp_context.rs"); + t.compile_fail("tests/error_wp_contextual_option_without_wp_context.rs"); t.compile_fail("tests/error_wp_contextual_not_a_struct.rs"); } diff --git a/wp_contextual/tests/error_wp_contextual_option_without_wp_context.rs b/wp_contextual/tests/error_wp_contextual_option_without_wp_context.rs new file mode 100644 index 000000000..629f1f074 --- /dev/null +++ b/wp_contextual/tests/error_wp_contextual_option_without_wp_context.rs @@ -0,0 +1,11 @@ +use wp_contextual::WPContextual; + +#[derive(WPContextual)] +pub struct SparseFoo { + #[WPContextualOption] + pub bar: Option, +} + +fn main() {} + +uniffi::setup_scaffolding!(); diff --git a/wp_contextual/tests/error_wp_contextual_option_without_wp_context.stderr b/wp_contextual/tests/error_wp_contextual_option_without_wp_context.stderr new file mode 100644 index 000000000..eea484561 --- /dev/null +++ b/wp_contextual/tests/error_wp_contextual_option_without_wp_context.stderr @@ -0,0 +1,5 @@ +error: #[WPContextualOption] doesn't have any contexts. Did you forget to add #[WPContext] attribute? + --> tests/error_wp_contextual_option_without_wp_context.rs:5:5 + | +5 | #[WPContextualOption] + | ^ From 986d524bd373f91ac6ac98a2c6e7b8b3a2dba8a1 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 22 May 2024 16:29:49 -0400 Subject: [PATCH 4/5] Mark SparseUser.avatar_urls as #[WPContextualOption] --- wp_api/src/users.rs | 3 +++ wp_contextual/src/lib.rs | 2 +- wp_contextual/src/wp_contextual.rs | 5 +++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/wp_api/src/users.rs b/wp_api/src/users.rs index ef2c52571..1ff74941d 100644 --- a/wp_api/src/users.rs +++ b/wp_api/src/users.rs @@ -360,6 +360,9 @@ pub struct SparseUser { #[WPContext(edit)] pub extra_capabilities: Option>, #[WPContext(edit, embed, view)] + // According to our tests, `avatar_urls` is not available for all site types. It's marked with + // `#[WPContextualOption]` which will make it an `Option` in the generated contextual types. + #[WPContextualOption] pub avatar_urls: Option>, // meta field is omitted for now: https://github.com/Automattic/wordpress-rs/issues/57 } diff --git a/wp_contextual/src/lib.rs b/wp_contextual/src/lib.rs index b4c5e40e7..3b423dac1 100644 --- a/wp_contextual/src/lib.rs +++ b/wp_contextual/src/lib.rs @@ -282,7 +282,7 @@ mod wp_contextual; /// [`WPContextual`] type. This will tell the compiler to replace the given `Option` /// type with the appropriate contextual type: `BazWithEditContext`, `BazWithEmbedContext` or /// `BazWithViewContext`. -/// * `[WPContextualOption]` is used to tell the compiler to keep the field as an `Option`. +/// * `[WPContextualOption]` is used to tell the compiler to keep the field's `Option` type. /// * Generated types will have the following derive macros: /// `#[derive(Debug, serde::Serialize, serde::Deserialize, uniffi::Record)]`. These types are meant /// to be used for the diff --git a/wp_contextual/src/wp_contextual.rs b/wp_contextual/src/wp_contextual.rs index 9bbcdd66d..0bccac775 100644 --- a/wp_contextual/src/wp_contextual.rs +++ b/wp_contextual/src/wp_contextual.rs @@ -74,7 +74,7 @@ fn struct_fields( // * `WPContextualParseError::WPContextualOptionWithoutWPContext`: #[WPContextualOption] is added to // a field that doesn't have the #[WPContext] attribute. // * `WPContextualParseError::WPContextualBothOptionAndField`: #[WPContextualField] and -// #[WPContextualOption] was used together. +// #[WPContextualOption] attributes were used together. // // It'll also handle incorrectly formatted #[WPContext] attribute through // `parse_contexts_from_tokens` helper. @@ -158,7 +158,8 @@ fn parse_fields( // Check if there are any fields that has both #[WPContextualField] & #[WPContextualOption] // attributes and return an error. These attributes are incompatible with each other because - // #[WPContextualOption] will leave the type as is, whereas #[WPContextualField] will modify it + // #[WPContextualOption] will leave the type as is, whereas #[WPContextualField] will modify + // it. if let Some(pf) = parsed_fields.iter().find(|pf| { pf.parsed_attrs .contains(&WPParsedAttr::ParsedWPContextualField) From e2b539d85347ef00bd349f06809c74963b3b9fba Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Wed, 22 May 2024 18:16:01 -0400 Subject: [PATCH 5/5] Import HashMap in wp_contextual documentation tests --- wp_contextual/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wp_contextual/src/lib.rs b/wp_contextual/src/lib.rs index 3b423dac1..a9b330b7f 100644 --- a/wp_contextual/src/lib.rs +++ b/wp_contextual/src/lib.rs @@ -246,6 +246,7 @@ //! //! ``` //! # use wp_contextual::WPContextual; +//! # use std::collections::HashMap; //! #[derive(WPContextual)] //! pub struct SparseUser { //! #[WPContext(edit)] @@ -260,6 +261,7 @@ //! This will generate the following: //! //! ``` +//! # use std::collections::HashMap; //! pub struct UserWithEditContext { //! pub avatar_urls: Option>, //! }