maybe for using dynamic dispatching in a statically (and strongly) typed language?
to be honest, I don't think it's "wrong" to use boxed closures like this. I prefer a (statically and) well-typed solution in general, but I'm not bothered if I use dynamic dispatching, whether forced to or by choice.
that said, if you really really want a more "rusty" style with stronger types, I can see several ways to improve the current code.
you can create a type to represent the concept of "optional iterator of requests", e.g. we can call it WantedMethodSet, in order to alleviate the duplicated Some(request) vs None condition checks, something like:
/// is mathematically a set, can also implement using
// a `HashSet`, or `BTreeSet` if order is to be preserved
// you can use a custom `MethodId` type to replace the `syn::Ident`
// and use various tricks for this "set" type, like bitmaps
#[derive(Default)]
struct WantedMethodSet {
names: Vec<Ident>,
}
impl WantedMethodSet {
// constructor, optionally can also validate method names here
fn from_iter(names: impl Iterator<Item = Ident>) => Self {
Self { names: names.collect() }
}
// alternatively, validate names after construction,
// e.g. returns unsupported names for error reports
fn invalide(&self) -> Vec<Ident> {
self.names.iter().filter(|name| !is_supported(name)).cloned().collect()
}
// encapsulate the `Some(requests)` or `None` condition
fn is_requested(&self, name: &Ident) -> bool {
self.is_empty() || self.contains(name)
}
}
fn is_supported(name: Ident) -> bool {
[METHOD_FOO, METHOD_BAR, METHOD_BAZ, ...].contains(name)
}
this can then be used in case you use a long if conditions similar to this:
let wanted = match attribute.meta {
Path(_) => WantedMethodSet::default(),
List(_) => WantedMethodSet::from_iter(atribute.parse_args_with(...)?.into_iter()),
NameValue(_) => todo!(),
};
if !wanted.invalid().is_empty() {
todo!("error report");
}
if wanted.is_requested(&METHOD_FOO) {
// generate code for method `foo()`
}
if wanted.is_requested(&METHOD_BAR) {
// generate code for method `bar()`
}
if wanted.is_requested(&METHOD_BAZ) {
// generate code for method `baz()`
}
//...
this still has some duplications though, e.g. is_supported() needs to know the list of predefined method names.
that's where dynamic dispatching comes into play. but you can still make some improvement to what you have now. for instance, you can wrap the dispatcher map into a new type, and call it, say, MethodRegistry:
// the lazy code generator for a specific method
type MethodTokens = Box<dyn Fn() -> TokenStream2>;
// if you want to be fancy, make this a custom type
// such as a newtype wrapper, or an enum, in which case
// you can even implement `Parse` if you want to be fancy
// so you can parse it directly into `Puncutated<MethodId, Delim>`
type MethodId = Ident;
#[derive(Default)]
struct MethodRegistry(HashMap<MethodId, MethodTokens>);
impl MethodRegistry {
fn register(&mut self, name: MethodId, tokens: MethodTokens) {
self.0.insert(name, tokens);
}
fn generate_all(&self, accu: &mut TokenStream2) {
for tokens in self.0.values() {
accu.extend(tokens());
}
}
fn generate_requested(&self, requested: impl Iterator<Item = &MethdId>, accu: &mut TokenStream2) {
for name in requested {
match self.0.get(name) {
//...
}
}
}
}
then the dispach looks like this:
// alternatively, use a static `LazyLock` if the registry should be reused multiple times
let registry = todo!();
match attribute.meta {
Path(_) => registry.generate_all(&mut methods),
List(_) =>
registry.generate_requested(
attribue.parse_args_with(Punctuated::...).into_iter(),
&mut methods,
),
NameValue(_) => todo!(),
}
this is just some ideas for you to start playing with.
