audunhalland / entrait

Loosely coupled Rust application design made easy
86 stars 3 forks source link

Impl expansion incorrectly adding lifetime from method #29

Closed MrJerB closed 1 year ago

MrJerB commented 1 year ago

Hi there,

Thanks for the library as well as the concept! I might have discovered a case of incorrect expansion in a certain scenario. I'll try to explain below:

I have a handwritten trait named Foo in crate A which I then want to implement as MyFoo in crate B. The expansion in crate B is causing errors due to the lifetime specified in the method I prescribed in my handwritten trait.

The DoFoo method in my trait contains a lifetime parameter (my real implementation involves tokenizing some input string, and preferably I would like to avoid clones).

Crate A (no issues) ## Crate A: ```rust #[entrait(FooImpl, delegate_by = FooDelegate)] trait Foo { fn DoFoo<'a>(&self, input: &'a str) -> &'a str; } ``` ## Crate A expanded (no issues): ```rust trait Foo { fn DoFoo<'a>(&self, input: &'a str) -> &'a str; } trait FooImpl: 'static { fn DoFoo<'a>(__impl: &::entrait::Impl, input: &'a str) -> &'a str; } pub trait FooDelegate { type Target: FooImpl; } impl Foo for ::entrait::Impl where EntraitT: FooDelegate + Sync + 'static, { #[inline] fn DoFoo<'a>(&self, input: &'a str) -> &'a str { >::DoFoo(self, input) } } ```
Crate B ## Crate B ```rust struct MyFoo; #[entrait] impl FooImpl for MyFoo { fn DoFoo<'a, D>(deps: &D, input: &'a str) -> &'a str { input } } ``` ## Crate B expanded (issue is here) ```rust impl MyFoo { fn DoFoo<'a, D>(deps: &D, input: &'a str) -> &'a str { input } } // Issue lies here: as the lifetime is added to the impl for the trait and in the wrong order (it is not needed at all) impl FooImpl for MyFoo { #[inline] fn DoFoo<'a>(__impl: &::entrait::Impl, input: &'a str) -> &'a str { Self::DoFoo(__impl, input) } } ```

I am using version 0.5.3 of the crate.

audunhalland commented 1 year ago

Hi, thanks for your interest and for reporting an issue! I'll try to look into it.

Entrait is not something I engage with every day. Whether it's really a viable design pattern for production code is for me an open question, and the answer to that is dependent on people evaluating it, so thanks :)

This (on first glance) seems like a proper bug.

PS: PRs are very welcome!

audunhalland commented 1 year ago

0.6.0 released. The reason for the major bump is that unimock was already bumped some time ago on the main branch. I initially planned some other changes before 0.6.0 but I never got to do them.