asomers
Initial support for trait-variant
This PR aims to fix #639
Feel free to update the readme, as English is not my primary language
@asomers I think the nightly is failing because some sort of new lints unrelated to this PR, would you like me to fix those also here?
@asomers I think the nightly is failing because some sort of new lints unrelated to this PR, would you like me to fix those also here?
It's already fixed on master. You just need to rebase.
I think the tests could really benefit from some kind of assertion to check that they actually implement the additional trait. That might be hard to do with
Send, but what about something like this?trait Foo {} #[automock] #[trait_variant::make(Foo)] pub trait Bar { ... } #[test] fn some_test_method_name() { let mock = MockBar::new(); let p: Box<dyn Foo> = Box::new(mock); // Ensures that MockBar implements Foo ... }Also, with this change, does the
#[trait_variant::make(OtherTraitName: Foo)]syntax work?
I'm not that familiar with the codebase to implement all of those changes.
On your first example, automock would not implement Foo trait for MockBar, because it might have any arbitrary number of unrelated methods, but it could be manually implemented:
trait Foo {}
#[automock]
#[trait_variant::make(Foo)]
pub trait Bar {
...
}
impl Foo for MockBar {}
The most common use case is to mark your trait (and async method futures) Send, and mocks are already Send so that's covered.
As for the second example, the syntax already works, but we would be mocking the initial trait only (Bar), not the other variant (OtherTraitName). With that syntax you're actually defining two different traits.
The main goal of this PR is to have feature parity with async_trait, other improvements could be added later.
As mentioend on #639 the async_trait annotation can be located before or after the automock annotation, and that means you need to return the actual pinned future or just the future's output on the mocks expectations.
With this PR we can do the same for trait_variant, so that the migration is painless.
@asomers can this PR be merged as is to allow initial basic support for trait_variant?
Other improvements can be added later, when needed
We need to decide what do do about the
#[trait_variant::make(IntFactory: Send)]syntax. It might just work. Have you tried it?
@asomers It was already working but it mocks the original trait (not the variant)
On this example, #[automock] mocks LocalFoo, generating MockLocalFoo:
#[automock]
#[trait_variant::make(Foo: Send)]
pub trait LocalFoo {
async fn foo(&self) -> u32;
async fn bar() -> u32;
}
I've extended automock to allow mocking the variant as well (I've included examples and updated automock documentation)
On this example, #[automock(target = Foo)] mocks Foo, generating MockFoo:
#[automock(target = Foo)]
#[trait_variant::make(Foo: Send)]
pub trait LocalFoo {
async fn foo(&self) -> u32;
async fn bar() -> u32;
}
Note: trait-variant automatically generates a blanket impl for any type implementing Foo to also implement LocalFoo, so the resulting MockFoo can be used anywhere expecting both LocalFoo or Foo
All of the other comments have been resolved, except for the static assertion which is not required. If you still want me to include it let me know.
@asomers There's already a changelog entry, let me know if you would like another sentence.
@asomers There's already a changelog entry, let me know if you would like another sentence.
Of course you did. But yesterday I was only viewing "changes since your last review" so I didn't see it. My bad.