Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow mocking methods with non-static generic arguments #217

Open
asomers opened this issue Oct 2, 2020 · 18 comments
Open

Allow mocking methods with non-static generic arguments #217

asomers opened this issue Oct 2, 2020 · 18 comments
Labels
enhancement New feature or request

Comments

@asomers
Copy link
Owner

asomers commented Oct 2, 2020

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.

@MarinPostma
Copy link

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.

@asomers
Copy link
Owner Author

asomers commented Jan 22, 2021

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 .

@MarinPostma
Copy link

Thanks for the answer!

@ljw1004
Copy link

ljw1004 commented Sep 9, 2022

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.

@asomers
Copy link
Owner Author

asomers commented Sep 18, 2022

@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
Copy link
Owner Author

asomers commented Sep 18, 2022

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 added a commit that referenced this issue Sep 20, 2022
Add a #[mockall::concretize] attribute.  When set on a function or
method, its generic expectations will be turned into trait objects.

But it only works for function arguments that are pure generic types or
a few basic combinations:
* T
* &T
* &mut T
* &[T]

Issue #217
asomers added a commit that referenced this issue Sep 20, 2022
Add a #[mockall::concretize] attribute.  When set on a function or
method, its generic expectations will be turned into trait objects.

But it only works for function arguments that are pure generic types or
a few basic combinations:
* T
* &T
* &mut T
* &[T]

Issue #217
asomers added a commit that referenced this issue Sep 20, 2022
Add a #[mockall::concretize] attribute.  When set on a function or
method, its generic expectations will be turned into trait objects.

But it only works for function arguments that are pure generic types or
a few basic combinations:
* T
* &T
* &mut T
* &[T]

Issue #217
@i-am-chauhan
Copy link

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.

@asomers
Copy link
Owner Author

asomers commented Feb 9, 2023

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?

@TheDan64
Copy link

TheDan64 commented Feb 9, 2023

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

@i-am-chauhan
Copy link

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

@asomers
Copy link
Owner Author

asomers commented Feb 9, 2023

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
Copy link
Owner Author

asomers commented Feb 9, 2023

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.

@i-am-chauhan
Copy link

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.

@TheDan64
Copy link

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

@asomers
Copy link
Owner Author

asomers commented Mar 21, 2023

@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.

@punkeel
Copy link

punkeel commented Jun 5, 2023

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

Cargo.toml (0d27b44)

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

@asomers
Copy link
Owner Author

asomers commented Jun 5, 2023

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;

@punkeel
Copy link

punkeel commented Jun 6, 2023

(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]> 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants