Skip to content

Commit

Permalink
bug(di): impl Trait in generics not fixed correctly (#30)
Browse files Browse the repository at this point in the history
It is possible to have a singleton / scoped dependency which has an
`impl Trait` inside some generic. This will cause a compile error as
`impl Trait` cannot be used inside struct type fields.

This PR fixes the issue by correctly replacing the `impl Trait` with the
hinted type.
  • Loading branch information
chesedo authored Sep 23, 2024
1 parent 5a214bd commit 39dcfef
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 2 deletions.
2 changes: 1 addition & 1 deletion despatma-dependency-container/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ proc-macro-error = "1.0.4"
proc-macro2.workspace = true
quote.workspace = true
strsim = "0.11.1"
syn = { workspace = true, features = ["extra-traits", "full"] }
syn = { workspace = true, features = ["extra-traits", "full", "visit-mut"] }

[dev-dependencies]
async-once-cell.workspace = true
Expand Down
6 changes: 5 additions & 1 deletion despatma-dependency-container/src/processing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use crate::input;
use self::visitor::{
AddWildcardLifetime, ErrorVisitorMut, ExtractAsync, ExtractBoxType, ExtractLifetime,
ImplTraitButRegisteredConcrete, ImplTraitFields, LinkDependencies, OwningManagedDependency,
SetHasExplicitLifetime, UnsupportedRegisteredTypes, VisitableMut, WrapBoxType,
ReplaceImplGenericsWithConcrete, SetHasExplicitLifetime, UnsupportedRegisteredTypes,
VisitableMut, WrapBoxType,
};

mod visitor;
Expand Down Expand Up @@ -127,6 +128,9 @@ impl Container {
self.process_visitor::<ExtractLifetime>();
self.process_visitor::<LinkDependencies>();

// Needs field types (lifetimes) to be extracted and dependencies to be linked first
self.process_visitor::<ReplaceImplGenericsWithConcrete>();

// Needs lifetimes to be extracted first
self.process_visitor::<ImplTraitFields>();

Expand Down
2 changes: 2 additions & 0 deletions despatma-dependency-container/src/processing/visitor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub use impl_trait_but_registered_concrete::ImplTraitButRegisteredConcrete;
pub use impl_trait_fields::ImplTraitFields;
pub use link_dependencies::LinkDependencies;
pub use owning_managed_dependency::OwningManagedDependency;
pub use replace_impl_generics_with_concrete::ReplaceImplGenericsWithConcrete;
pub use set_has_explicit_lifetime::SetHasExplicitLifetime;
pub use unsupported_registered_types::UnsupportedRegisteredTypes;
pub use wrap_box_type::WrapBoxType;
Expand All @@ -22,6 +23,7 @@ mod impl_trait_but_registered_concrete;
mod impl_trait_fields;
mod link_dependencies;
mod owning_managed_dependency;
mod replace_impl_generics_with_concrete;
mod set_has_explicit_lifetime;
mod unsupported_registered_types;
mod wrap_box_type;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
use std::collections::HashMap;

use syn::{
visit_mut::{visit_type_mut, VisitMut},
Type,
};

use crate::processing::Dependency;

use super::{ErrorVisitorMut, VisitorMut};

/// Replaces any `impl Trait` in generics with their explicitly hinted concrete types.
///
/// Needs to happen after type hints (lifetimes) are extracted.
/// And after dependencies are linked.
pub struct ReplaceImplGenericsWithConcrete {
replacer: Replacer,
}

impl VisitorMut for ReplaceImplGenericsWithConcrete {
fn visit_dependency_mut(&mut self, dependency: &mut Dependency) {
if let Some(field_ty) = &mut dependency.field_ty {
if self.replacer.to_replace.contains_key(&dependency.ty) {
return;
}

// Replace children first as they might impact this parent type
for dependency in dependency.dependencies.iter_mut() {
self.visit_dependency_mut(&mut dependency.inner.borrow_mut());
}

// Fix `field_ty` first before it is registered for later replacements
self.replacer.visit_type_mut(field_ty);

self.replacer
.to_replace
.insert(dependency.ty.clone(), field_ty.clone());
}
}
}

struct Replacer {
to_replace: HashMap<Type, Type>,
}

impl VisitMut for Replacer {
fn visit_type_mut(&mut self, ty: &mut Type) {
if let Some(concrete_type) = self.to_replace.get(ty) {
*ty = concrete_type.clone();
} else {
// Continue checking for any impl types on inner generics
visit_type_mut(self, ty);
}
}
}

impl ErrorVisitorMut for ReplaceImplGenericsWithConcrete {
fn new() -> Self {
Self {
replacer: Replacer {
to_replace: Default::default(),
},
}
}
}

#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use syn::parse_quote;

use crate::{
input,
processing::{
self,
visitor::{ExtractLifetime, LinkDependencies, VisitableMut},
},
};

use super::*;

#[test]
fn impl_trait_but_registered_concrete() {
let mut container: processing::Container = input::Container::from_item_impl(parse_quote!(
impl Container {
#[Singleton(Sqlite)]
fn db(&self) -> impl DB {
Sqlite
}

#[Singleton]
fn service(&self, db: impl DB) -> Service<impl DB> {
Service(db)
}
}
))
.into();

// Test needs them to be linked and lifetimes to be extracted
container.apply_mut(&mut ExtractLifetime::new());
container.apply_mut(&mut LinkDependencies::new());

assert_eq!(
container.dependencies[0].borrow().field_ty,
Some(parse_quote!(Sqlite))
);
assert_eq!(
container.dependencies[1].borrow().field_ty,
Some(parse_quote!(Service<impl DB>))
);

container.apply_mut(&mut ReplaceImplGenericsWithConcrete::new());

assert_eq!(
container.dependencies[0].borrow().field_ty,
Some(parse_quote!(Sqlite))
);
assert_eq!(
container.dependencies[1].borrow().field_ty,
Some(parse_quote!(Service<Sqlite>))
);
}
}

0 comments on commit 39dcfef

Please sign in to comment.