Skip to content

Commit

Permalink
Relax restriction on duplicate derive macro helper attributes
Browse files Browse the repository at this point in the history
Multiple attributes are now allowed; only duplicate options are caught
  • Loading branch information
cyqsimon committed Oct 15, 2024
1 parent 2b45616 commit bba442c
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 62 deletions.
4 changes: 3 additions & 1 deletion documented-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@ proc-macro = true

[dependencies]
convert_case = "0.6.0"
itertools = { version = "0.13.0", optional = true }
optfield = { version = "0.3.0", optional = true }
proc-macro2 = "1.0.86"
quote = "1.0.35"
strum = { version = "0.26.3", features = ["derive"], optional = true }
syn = { version = "2.0.58", features = ["full", "extra-traits"] }

[dev-dependencies]
documented = { path = "../lib" }

[features]
customise = ["dep:optfield"]
customise = ["dep:itertools", "dep:optfield", "dep:strum"]
default = ["customise"]
7 changes: 6 additions & 1 deletion documented-macros/src/config/customise_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ mod kw {
/// accept or reject any of them in their `Parse` implementation.
///
/// Expected parse stream format: `<KW> = <VAL>`.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, strum::EnumDiscriminants)]
#[strum_discriminants(
name(ConfigOptionType),
derive(strum::Display, Hash),
strum(serialize_all = "snake_case")
)]
pub enum ConfigOption {
/// Custom visibility for the generated constant.
///
Expand Down
83 changes: 42 additions & 41 deletions documented-macros/src/config/derive.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
#[cfg(feature = "customise")]
use itertools::Itertools;
#[cfg(feature = "customise")]
use optfield::optfield;
#[cfg(feature = "customise")]
use syn::{
parse::{Parse, ParseStream},
parse2,
punctuated::Punctuated,
spanned::Spanned,
Attribute, Error, Meta, Token,
};
use syn::{punctuated::Punctuated, spanned::Spanned, Attribute, Error, Meta, Token};

#[cfg(feature = "customise")]
use crate::config::customise_core::ConfigOption;
use crate::config::customise_core::{ConfigOption, ConfigOptionType};

/// Configurable options for derive macros via helper attributes.
///
Expand Down Expand Up @@ -41,12 +37,14 @@ impl DeriveConfig {
}
}

#[cfg(feature = "customise")]
impl Parse for DeriveCustomisations {
fn parse(input: ParseStream) -> syn::Result<Self> {
use ConfigOption as O;
// This is implemented instead of `syn::parse::Parse` because the options
// can come from multiple attributes and therefore multiple `MetaList`s.
impl TryFrom<Vec<ConfigOption>> for DeriveCustomisations {
type Error = syn::Error;

let args = Punctuated::<ConfigOption, Token![,]>::parse_terminated(input)?;
/// Duplicate option rejection should be handled upstream.
fn try_from(args: Vec<ConfigOption>) -> Result<Self, Self::Error> {
use ConfigOption as O;

let mut config = Self::default();
for arg in args {
Expand All @@ -55,11 +53,6 @@ impl Parse for DeriveCustomisations {
arg.kw_span(),
"This config option is not applicable to derive macros",
))?,

O::Trim(..) if config.trim.is_some() => Err(Error::new(
arg.kw_span(),
"This config option cannot be specified more than once",
))?,
O::Trim(_, val) => {
config.trim.replace(val);
}
Expand All @@ -73,35 +66,43 @@ impl Parse for DeriveCustomisations {
pub fn get_customisations_from_attrs(
attrs: &[Attribute],
attr_name: &str,
) -> syn::Result<Option<DeriveCustomisations>> {
let customise_attrs = attrs
) -> syn::Result<DeriveCustomisations> {
let customisations = attrs
.iter()
// remove irrelevant attributes
.filter(|attr| attr.path().is_ident(attr_name))
// parse options
.map(|attr| match &attr.meta {
Meta::List(attr_inner) => Ok(attr_inner),
Meta::List(attr_inner) => {
attr_inner.parse_args_with(Punctuated::<ConfigOption, Token![,]>::parse_terminated)
}
other_form => Err(Error::new(
other_form.span(),
format!("{attr_name} is not list-like. Expecting `{attr_name}(...)`"),
)),
})
.collect::<Result<Vec<_>, _>>()?;

let customise_attr = match customise_attrs[..] {
[] => return Ok(None),
[attr] => attr.clone(),
[first, ref rest @ ..] => {
let initial_error = Error::new(
first.span(),
format!("{attr_name} can only be declared once"),
);
let final_error = rest.iter().fold(initial_error, |mut err, declaration| {
err.combine(Error::new(declaration.span(), "Duplicate declaration here"));
err
});
Err(final_error)?
}
};

let customisations = parse2(customise_attr.tokens)?;
Ok(Some(customisations))
.collect::<Result<Vec<_>, _>>()?
.into_iter()
.flatten()
// error on duplicates
.into_group_map_by(|opt| ConfigOptionType::from(opt)) // lifetime variance issues
.into_iter()
.map(|(ty, opts)| match &opts[..] {
[] => unreachable!(), // guaranteed by `into_group_map_by`
[opt] => Ok(opt.clone()),
[first, rest @ ..] => {
let initial_error = Error::new(
first.kw_span(),
format!("Option {ty} can only be declaration once"),
);
let final_error = rest.iter().fold(initial_error, |mut err, opt| {
err.combine(Error::new(opt.kw_span(), "Duplicate declaration here"));
err
});
Err(final_error)
}
})
.collect::<Result<Vec<_>, _>>()?
.try_into()?;
Ok(customisations)
}
30 changes: 11 additions & 19 deletions documented-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ pub fn documented(input: TokenStream) -> TokenStream {
let config = DeriveConfig::default();
#[cfg(feature = "customise")]
let config = match get_customisations_from_attrs(&input.attrs, "documented") {
Ok(Some(customisations)) => DeriveConfig::default().with_customisations(customisations),
Ok(None) => DeriveConfig::default(),
Ok(customisations) => DeriveConfig::default().with_customisations(customisations),
Err(err) => return err.into_compile_error().into(),
};

Expand Down Expand Up @@ -188,8 +187,7 @@ pub fn documented_fields(input: TokenStream) -> TokenStream {
let base_config = DeriveConfig::default();
#[cfg(feature = "customise")]
let base_config = match get_customisations_from_attrs(&input.attrs, "documented_fields") {
Ok(Some(customisations)) => DeriveConfig::default().with_customisations(customisations),
Ok(None) => DeriveConfig::default(),
Ok(customisations) => DeriveConfig::default().with_customisations(customisations),
Err(err) => return err.into_compile_error().into(),
};

Expand All @@ -215,12 +213,10 @@ pub fn documented_fields(input: TokenStream) -> TokenStream {
#[cfg(not(feature = "customise"))]
let config = base_config;
#[cfg(feature = "customise")]
let config =
if let Some(c) = get_customisations_from_attrs(&attrs, "documented_fields")? {
base_config.with_customisations(c)
} else {
base_config
};
let config = base_config.with_customisations(get_customisations_from_attrs(
&attrs,
"documented_fields",
)?);
get_docs(&attrs, config.trim).map(|d| (i, d))
})
.collect::<syn::Result<Vec<_>>>()
Expand Down Expand Up @@ -337,8 +333,7 @@ pub fn documented_variants(input: TokenStream) -> TokenStream {
let base_config = DeriveConfig::default();
#[cfg(feature = "customise")]
let base_config = match get_customisations_from_attrs(&input.attrs, "documented_variants") {
Ok(Some(customisations)) => DeriveConfig::default().with_customisations(customisations),
Ok(None) => DeriveConfig::default(),
Ok(customisations) => DeriveConfig::default().with_customisations(customisations),
Err(err) => return err.into_compile_error().into(),
};

Expand All @@ -359,13 +354,10 @@ pub fn documented_variants(input: TokenStream) -> TokenStream {
#[cfg(not(feature = "customise"))]
let config = base_config;
#[cfg(feature = "customise")]
let config = if let Some(c) =
get_customisations_from_attrs(&attrs, "documented_variants")?
{
base_config.with_customisations(c)
} else {
base_config
};
let config = base_config.with_customisations(get_customisations_from_attrs(
&attrs,
"documented_variants",
)?);
get_docs(&attrs, config.trim).map(|d| (i, f, d))
})
.collect::<syn::Result<Vec<_>>>()
Expand Down
18 changes: 18 additions & 0 deletions documented-test/src/derive/documented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,24 @@ doge
assert_eq!(Doge::DOCS, doc_str);
}

#[test]
fn multiple_attrs_works() {
/** Wow
much
doge
*/
#[derive(Documented)]
#[documented()]
#[documented()]
struct Doge;

let doc_str = "Wow
much
doge
";
assert_eq!(Doge::DOCS, doc_str);
}

#[test]
fn trim_false_works() {
/** Wow
Expand Down
16 changes: 16 additions & 0 deletions documented-test/src/derive/documented_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,22 @@ mod test_customise {
assert_eq!(Doge::get_field_docs("coin"), Ok("Wow, much coin"));
}

#[test]
fn multiple_attrs_works() {
#[derive(DocumentedFields)]
#[documented_fields()]
#[documented_fields()]
#[allow(dead_code)]
struct Doge {
/// Wow, much coin
#[documented_fields()]
#[documented_fields()]
coin: usize,
}

assert_eq!(Doge::get_field_docs("coin"), Ok("Wow, much coin"));
}

#[test]
fn container_customise_works() {
#[derive(DocumentedFields)]
Expand Down
19 changes: 19 additions & 0 deletions documented-test/src/derive/documented_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,25 @@ mod test_customise {
assert_eq!(Name::Kabuso.get_variant_docs(), Ok("RIP"));
}

#[test]
fn multiple_attrs_works() {
#[derive(DocumentedVariants)]
#[documented_variants()]
#[documented_variants()]
#[allow(dead_code)]
enum Name {
/// Wow
#[documented_variants()]
#[documented_variants()]
Doge,
/// RIP
Kabuso,
}

assert_eq!(Name::Doge.get_variant_docs(), Ok("Wow"));
assert_eq!(Name::Kabuso.get_variant_docs(), Ok("RIP"));
}

#[test]
fn container_customise_works() {
#[derive(DocumentedVariants)]
Expand Down

0 comments on commit bba442c

Please sign in to comment.