audunhalland / entrait

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

Associated types fail to compile #23

Open austinjones opened 1 year ago

austinjones commented 1 year ago

Hey @audunhalland, another associated type issue here!

#[entrait::entrait]
pub trait Client {
    type DatabaseImpl;

    fn database(&self, name: &str) -> Self::DatabaseImpl;
}

The associated type disappears from the generated trait:

pub trait MongoClient {
    fn database(&self, name: &str) -> Self::DatabaseImpl;
}

It also needs to be bound in the entrait::Impl block as:

type DatabaseImpl = <EntraitT as MongoClient>::DatabaseImpl;
audunhalland commented 1 year ago

Hi, to me this use case looks a lot like Truly inverted dependencies - static dispatch. What makes the trait mockable in that case is that the facade trait has no associated type. The associated type is in the "helper trait" that is only in use with Impl<T>, not when mocked.

The unimock library does not support associated types, and I don't think it's necessary to support it because it's possible to design oneself around it.

The Rust realworld entrait example project uses this pattern a lot, if you need examples.

austinjones commented 1 year ago

Hi, my use case is a mongodb driver abstraction - we are taking a chunk of the mongodb crate and putting it behind traits for testability. Associated types are needed so that internals can be shared (impl Trait in arguments or Box returns hide those details).

It makes sense that unimock can't support associated types, but support in entrait would be really convenient. It seems to be a straightforward binding for Impl<EntraitT> based on <EntraitT as Trait>::AssociatedType. I fell back to manually implementing my traits for ::entrait::Impl<EntraitT>, but it seems like the macro could generate all that code.

I have a workaround, so feel free to close this if you don't want to support this feature.

audunhalland commented 1 year ago

So, to understand better:

Say you have ~one~ two distinct Impl<A> and Impl<B> (two different concrete "application" types). Do you need those to have different DatabaseImpls, like impl Client for A { type DatabaseImpl = ProperMongo; } and impl Client for B { type DatabaseImpl = TestMongo; }?

Those would be two proper non-mocked applications from the entrait point of view. This is of course a plausible design, and entrait should definitely support this.

This indeed looks like a proper bug and I'll look into it unless you feel like making a PR.

audunhalland commented 1 year ago

Need to keep in mind that this type of trait would not be automatically mockable like other traits. The mock implementation has to be written manually, but the macro could tell the user this explicitly.

I also tried to imagine other ways you could maybe achieve the same thing. The fn database(&self, name: &str) -> Self::DatabaseImpl implies that you want to create some owned value in there. I guess returning a borrowed value would not suffice in your case. If you can return a borrowed value, you can use

#[entrait(delegate_by=ref)]
trait GetDatabase: 'static {
    fn get_database(&self) -> &dyn Database;
}

and Database could look something like

trait Database {
    fn open(&self, name: &str) -> Box<dyn DatabaseImpl>;
}

?

edit: or even just one trait (why overcomplicate)

#[entrait(delegate_by=ref)]
trait Client: 'static {
    fn database(&self, name: &str) -> Box<dyn DatabaseImpl>;
}

(assuming you can use boxing and dynamic dispatch)

austinjones commented 1 year ago

Hah, my traits are very far from being object safe. Lots of impl Into<Option<SomeOperationOptions>> for convenient options handling and generics for deserialization types.

In terms of mocking and associated types, mockall has you specify those in the automock invocation. Or you can use mock! and specify them by hand. Though, impl Trait in argument position isn't supported.

audunhalland commented 1 year ago

Another use case for associated types is something like transaction support. I see that a database abstraction is pretty hard to model using only function calls. See also this URLO thread. I'll try to get something like this to work in rust-realworld-entrait.