mockall icon indicating copy to clipboard operation
mockall copied to clipboard

Allow mocking methods with non-static generic arguments

Open asomers opened this issue 5 years ago • 18 comments

Currently Mockall cannot mock generic methods with non-static generic arguments. The reason is because arguments are downcast to Any and stored in a map. That allows the caller to set different expectations for different types, like this:

#[automock]
trait Foo {
    fn foo<T: Debug>(&self, t: T);
}
let mut mock = MockFoo::new();
mock.expect_foo::<u32>()
    .with(predicate::eq(42))
    .return_const(());
mock.expect_foo::<&'static str>()
    .withf(|t: &'static str| t == "42")
    .return_const(());

However, it also poses some problems. Chiefly, it's impossible to mock methods with non-'static generic arguments, because they can't be downcast. Also, it's difficult to mock arguments with unnameable or difficult-to-name argument types, like closures or futures.

An alternative may be possible: mock methods with generic arguments by turning those arguments into trait objects. That would skip downcasting. Most returning closures would be compatible; the argument's type would just change from concrete like &u32 to a trait object, like &dyn Debug. Also, the expect_foo method would no longer be generic. All expectations would be matched against all invocations.

There are other problems with this approach, however. Chiefly, not every type can be made into a trait object. Also, most trait objects wouldn't be useable with any of the predicate types. i.e., you wouldn't be able to do expect_foo().with(predicate::eq(42)). So I don't think this new technique could be made mandatory. Instead, it would probably have to be optional, opt-in with an attribute. Like this:

#[mockall::automock]
trait Foo {
    #[mockall::expect_trait_object]
    fn foo<T: Debug>(&self, t: T);
}
let mut mock = MockFoo::new();
mock.expect_foo()
    .withf(|t: &dyn Debug| format!("{:?}", t) == "42")
    .return_const()

And of course, none of this applies to generic return values, only arguments.

asomers avatar Oct 02 '20 02:10 asomers

Hello! Any breakthrough on this issue? Not being able to have non-static generic methods is a very stong design constraint. I would like to help, if you can guide me a bit :)

Also, maybe it could be nice to be able to ignore the methods that we are unable to mock, with a #[ignore] attribute:

#[automock]
trait Foo {
    #[ignore]
    fn foo<T: Debug>(&self, t: T);
}

Calls to such trait methods would panic.

MarinPostma avatar Jan 22 '21 18:01 MarinPostma

No, I haven't had any time to work on it. And as for your #[ignore] suggestion, another user made a similar suggestion in #242 .

asomers avatar Jan 22 '21 19:01 asomers

Thanks for the answer!

MarinPostma avatar Jan 22 '21 19:01 MarinPostma

I hit what I think is an instance of this. Here's the minimal example:

#[mockall::automock] // ERROR "lifetime may not live long enough"
pub trait T {
    fn f<'a>(&self, s: &'a str) -> &'a str;
}

Here's a bigger and more motivating example. I'm using arenas, so the lifetime of the returned value is naturally bounded by the lifetime of the arena parameter.

#[mockall::automock]
pub trait T {
    fn f<'a>(&self, arena: &'a bumpalo::Bump) -> &'a str;
}

struct S {}
impl T for S {
    fn f<'a>(&self,arena: &'a bumpalo::Bump) ->  &'a str {
        arena.alloc_str("hello")
    }
}

pub fn main() {
    let arena = bumpalo::Bump::new();
    let s = (S {}).f(&arena);
    println!("{}", s);
}

I wish I could #[ignore], as suggested in earlier in this thread.

ljw1004 avatar Sep 09 '22 21:09 ljw1004

@ljw1004 your problem is not quite the same, because your only generic parameters are lifetimes. In your case, I think you should be able to simply eliminate the lifetime parameter. What happens if you do that? If that's not an option, then an easy workaround is to use mock! instead of #[automock]. That way you can leave out the lifetime parameter for just the mock version.

asomers avatar Sep 18 '22 20:09 asomers

Update: I've made some progress on this feature. I have a branch that allows mocking functions with non-static generic parameters, with some restrictions:

  • You must annotate the function with an attribute.
  • Predicates are useless, so you can only match with .withf and .withf_st.
  • Expectations are non-generic, and their arguments are trait objects.
  • It won't work if the function also has a closure argument.
  • It only works for arguments whose type is identical to the generic parameter, or a reference to it. For example, t: T and t: &t both work, but t: &[T] and t: IntoIterator<Item=T> do not.

The last restriction is the most troubling. I'm not sure it's worth merging this feature if I can't solve it.

Here's an example of how the new feature will work:

#[automock]
trait Foo {
    #[mockall::concrete_expectations]
    fn foo<P: AsRef<Path>>(&self, p: P);
}

#[test]
fn test_foo() {
    let mut foo = MockFoo::new();
    foo.expect_foo()
        .withf(|p| p.as_ref() == Path::new("/tmp"))
        .times(3)
        .return_const(());
    foo.foo(Path::new("/tmp"));
    foo.foo(Path::new("/tmp").to_owned());
    foo.foo("/tmp");
}

asomers avatar Sep 18 '22 20:09 asomers

Hi @asomers , this looks promising. But the solution you provided doesn't work for a generic method with generic return type.

Please, let me know if I'm mistaken or if there is any work around.

i-am-chauhan avatar Feb 09 '23 04:02 i-am-chauhan

Hi @asomers , this looks promising. But the solution you provided doesn't work for a generic method with generic return type.

Please, let me know if I'm mistaken or if there is any work around.

#[concretize] should not be needed for functions with generic return types, as long as those types are 'static or references with the same lifetime as &self. Do you need to mock a function with a non-static return type, or something?

asomers avatar Feb 09 '23 14:02 asomers

Just got bit by this. Wanted to pass a Drain iterator into my mocked function but couldn't because of the required 'static bound. Need to allocate a vec to get around it which is suboptimal for large data sets outside of the mock

TheDan64 avatar Feb 09 '23 17:02 TheDan64

Hi @asomers , this looks promising. But the solution you provided doesn't work for a generic method with generic return type. Please, let me know if I'm mistaken or if there is any work around.

#[concretize] should not be needed for functions with generic return types, as long as those types are 'static or references with the same lifetime as &self. Do you need to mock a function with a non-static return type, or something?

I was trying to return a non-static generic type and it is giving me error. For example

#[automock]
trait Foo {
    #[cfg_attr(not(target_os = "ia64-unknown-multics"), concretize)]
    fn foo<P: AsRef<Path>>(&self, p: P) -> P;
}

for return type it is saying cannot find type P in this scope

i-am-chauhan avatar Feb 09 '23 19:02 i-am-chauhan

Just got bit by this. Wanted to pass a Drain iterator into my mocked function but couldn't because of the required 'static bound. Need to allocate a vec to get around it which is suboptimal for large data sets outside of the mock

That's exactly what concretize should help with. Have you tried it?

asomers avatar Feb 09 '23 19:02 asomers

Hi @asomers , this looks promising. But the solution you provided doesn't work for a generic method with generic return type. Please, let me know if I'm mistaken or if there is any work around.

#[concretize] should not be needed for functions with generic return types, as long as those types are 'static or references with the same lifetime as &self. Do you need to mock a function with a non-static return type, or something?

I was trying to return a non-static generic type and it is giving me error. For example

#[automock]
trait Foo {
    #[cfg_attr(not(target_os = "ia64-unknown-multics"), concretize)]
    fn foo<P: AsRef<Path>>(&self, p: P) -> P;
}

for return type it is saying cannot find type P in this scope

Yeah, concretize doesn't help with return types. You're best option would probably be to add a 'static bound to P.

asomers avatar Feb 09 '23 19:02 asomers

Hi @asomers , that is not the only issue if you wrap the generic type in another Type, it gives the same error. Even in case of arguments.

#[automock]
trait Foo {
    #[cfg_attr(not(target_os = "ia64-unknown-multics"), concretize)]
    fn foo<P: AsRef<Path>>(&self, p: Option<P>);
}

In above example, it is unable to find P when I wrap it inside Option or any other type.

i-am-chauhan avatar Feb 10 '23 05:02 i-am-chauhan

Just got bit by this. Wanted to pass a Drain iterator into my mocked function but couldn't because of the required 'static bound. Need to allocate a vec to get around it which is suboptimal for large data sets outside of the mock

That's exactly what concretize should help with. Have you tried it?

Yeah, it doesn't seem to work:

use async_trait::async_trait;
use serde::DeserializeOwned;

#[async_trait]
pub trait Coll<T>: Send + Sync
where
    T: DeserializeOwned + Send + Sync + Unpin + 'static,
{
    async fn insert_many<D: IntoIterator<Item = T> + Send, O: Into<Option<()>> + Send + 'static>(
        &self,
        _docs: D,
        _options: O,
    ) -> Result<(), ()>;
}

mockall::mock! {
    #[derive(Debug)]
    pub Collection<T: DeserializeOwned + Send + Sync + Unpin + 'static> {}

    #[async_trait]
    impl<T: DeserializeOwned + Send + Sync + Unpin + 'static> Coll<T> for Collection<T> {

        #[mockall::concretize]
        async fn insert_many<
            D: IntoIterator<Item = T> + Send,
            O: Into<Option<()>> + Send + 'static,
        >(
            &self,
            _docs: D,
            _options: O,
        ) -> Result<(), ()>;
    }
}

which errors:

error: proc macro panicked
  --> src/main.rs:63:1
   |
63 | / mockall::mock! {
64 | |     #[derive(Debug)]
65 | |     pub Collection<T: DeserializeOwned + Send + Sync + Unpin + 'static> {}
66 | |
...  |
79 | |     }
80 | | }
   | |_^
   |
   = help: message: Type cannot be made into a trait object.  More information may be available when mockall is built with the "nightly" feature.

At first I thought this was related to your comment:

It only works for arguments whose type is identical to the generic parameter, or a reference to it. For example, t: T and t: &t both work, but t: &[T] and t: IntoIterator<Item=T> do not.

But it sounds different?

Compiles fine when D is +'static, but that's not what I want here

TheDan64 avatar Feb 10 '23 16:02 TheDan64

@TheDan64 sorry I missed this when you posted it. Could you try again, but using mockall's "nightly" feature? That gives much better error messages.

asomers avatar Mar 21 '23 15:03 asomers

Getting the same error as @TheDan64. Seems to be related to + Send. Here's a repro:

Cargo.toml (0d27b444)

mockall = { git = "https://github.com/asomers/mockall", features = ["nightly"] }
// src/main.rs
use mockall::predicate::*;
use mockall::*;

#[automock]
trait MyTrait {
    #[concretize]
    fn foo<K: AsRef<[u8]>>(&self, x: K) -> u32; // works

    #[concretize]
    fn foo2<K: Send + AsRef<[u8]>>(&self, x: K) -> u32; // breaks
}

fn call_with_four(x: &mut MockMyTrait) -> u32 {
    x.foo(vec![1, 2, 3, 4]) + x.foo2(vec![1, 2, 3, 4])
}

fn main() {
    let mut mock = MockMyTrait::new();
    mock.expect_foo()
        .times(1)
        .returning(|x: &dyn AsRef<[u8]>| x.as_ref().len() as u32 + 1);
    assert_eq!(10, call_with_four(&mut mock));
}

Error: error: Type cannot be made into a trait object

~/code/tmp/mocktmp ❯ cargo +nightly run
   Compiling mocktmp v0.1.0 (/Users/maxime/Code/tmp/mocktmp)
error: Type cannot be made into a trait object
  --> src/main.rs:10:16
   |
10 |     fn foo2<K: Send + AsRef<[u8]>>(&self, x: K) -> u32; // breaks
   |                ^^^^^^^^^^^^^^^^^^

error[E0412]: cannot find type `K` in this scope
  --> src/main.rs:10:46
   |
10 |     fn foo2<K: Send + AsRef<[u8]>>(&self, x: K) -> u32; // breaks
   |                                              ^ not found in this scope

For more information about this error, try `rustc --explain E0412`.
error: could not compile `mocktmp` (bin "mocktmp") due to 2 previous errors

punkeel avatar Jun 05 '23 03:06 punkeel

That's because concretize creates a trait object under the hood, and Rust imposes some restrictions on what types can be made into trait objects. Try these variations:

    #[concretize]
    fn foo2<K: AsRef<[u8]> + Send>(&self, x: K) -> u32;

or if that doesn't work,

trait MyTrait2: AsRef<[u8]> + Send {}
...
    #[concretize]
    fn foo2<K: MyTrait2>(&self, x: K) -> u32;

asomers avatar Jun 05 '23 20:06 asomers

(Your first code block is very similar to mine, with AsRef/Send swapped – and fails with the same error. Typo?)

The trait alias seems to work, but from what I understand, it requires implementing MyTrait2 for all the types I want (eg: str, String, &[u8], Vec<u8>, ....)

the trait `MyTrait2` is not implemented for `Vec<{integer}>`

In my case, I didn't actually need the Sync – so I made it work with async_trait(?Sync).. and I can confirm concretize worked great with AsRef<[u8]> 👍

punkeel avatar Jun 06 '23 11:06 punkeel