asomers / mockall

A powerful mock object library for Rust
Apache License 2.0
1.5k stars 62 forks source link

Cannot use T where T has 'static trait bound #477

Closed mckinnsb closed 10 months ago

mckinnsb commented 1 year ago

Currently I'm trying to implement a mock of BoxedSpan from the opentelemetry crate:

mock! {
    pub BoxedSpan {}

    impl Span for BoxedSpan {
        fn set_attribute(&mut self, attribute: KeyValue);

        fn set_status(&mut self, status: Status);

        fn end(&mut self);

        fn add_event_with_timestamp<T>(
            &mut self,
            name: T,
            timestamp: SystemTime,
            attributes: Vec<KeyValue>,
        ) where T: Into<Cow<'static, str>>;

        fn span_context(&self) -> &SpanContext;

        fn is_recording(&self) -> bool;

        fn update_name<T>(
            &mut self,
            new_name: T
        ) where T: Into<Cow<'static, str>>;

        fn end_with_timestamp(&mut self, timestamp: SystemTime);
    }
}

Which gives the following error:

error[E0310]: the parameter type `T` may not live long enough
    --> src/middleware.rs:244:5
     |
244  | /     mock! {
245  | |         pub BoxedSpan {}
246  | |
247  | |         impl Span for BoxedSpan {
...    |
271  | |         }
272  | |     }
     | |     ^
     | |     |
     | |_____...so that the type `T` will meet its required lifetime bounds...
     |       in this macro invocation
     |
    ::: /Users/smckinney/.cargo/registry/src/github.com-1ecc6299db9ec823/mockall_derive-0.11.4/src/lib.rs:1099:1
     |
1099 |   pub fn mock(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
     |   ---------------------------------------------------------------------- in this expansion of `mock!`
     |
note: ...that is required by this bound
    --> /Users/smckinney/.cargo/registry/src/github.com-1ecc6299db9ec823/mockall-0.11.4/src/lib.rs:1354:29
     |
1354 | pub trait AnyExpectations : Any + Send + Sync {}
     |                             ^^^
help: consider adding an explicit lifetime bound...
     |
268  |             ) where T: Into<Cow<'static, str>> + 'static;
     |                                                +++++++++

However, obviously, adding 'static to the trait bound creates another error, where the bound is stricter than the trait implementation.

I know that there are issues with mocking functions that take generics with lifetimes, but in this case, we are mocking a function that takes a generic that satisfies Into<Cow<'static, str>> - in other words, we can take the argument and turn it into a Cow pointer w/ a static lifetime.

Is this the same issue? Is there a way around this?

asomers commented 1 year ago

It's basically impossible to mock a function with a generic type that has an arbitrary lifetime. In this case, the easiest thing to do would probably be to add a + 'static bound to the trait. Is that an option? If not, you can give Mockall a little bit of help, in the form of a helper method, like this:

mock! {
    pub BoxedSpan {
        fn _set_attribute(&mut self, attribute: KeyValue);
        fn _update_name<T>(&mut self, new_name: T) where T: Into<Cow<'static, str>> + 'static;
        ...
    }
}
impl Span for MockBoxedSpan {
        fn set_attribute(&mut self, attribute: KeyValue) {
            self._set_attribute(attribute)
        }
        fn update_name<T>(&mut self, new_name: T) where T: Into<Cow<'static, str>> {
            let static_new_name = unsafe {
                // Unsafely transmute its lifetime
            };
            self._update_name(static_new_name)
        }
}
mckinnsb commented 1 year ago

In this case, the easiest thing to do would probably be to add a + 'static bound to the trait. Is that an option?

Unfortunately not in this case, this is an external trait. I'll give that helper method a try and report back.

mckinnsb commented 1 year ago

So is the idea here to let mockall mock functions w/ an _ that correspond to "safe" mocked functions, and then to implement the trait directly on the mock, passing through to _ most of the time but mutating the lifetime of a string when necessary?

Is there any way that you can think of to cut down on some of the wrapper functions? Trying to think of ways to handle this without creating a wrapper for everything. It might actually be better to implement the trait directly in this case w/ testing functionality, I'm guessing?

Anyway, thanks for the help - I appreciate it for sure.

asomers commented 1 year ago

So is the idea here to let mockall mock functions w/ an _ that correspond to "safe" mocked functions, and then to implement the trait directly on the mock, passing through to _ most of the time but mutating the lifetime of a string when necessary?

Exactly.

Is there any way that you can think of to cut down on some of the wrapper functions? Trying to think of ways to handle this without creating a wrapper for everything. It might actually be better to implement the trait directly in this case w/ testing functionality, I'm guessing?

You could try the new concretize feature. It's intended for use-cases just like this one. But it isn't in a stable release yet, so to use it you'll have to use mockall's git master branch. Read the docs in that branch for details. To try it out, do this:

mock! {
    pub BoxedSpan {}

    impl Span for BoxedSpan {
        ...
        #[concretize]
        fn update_name<T>(
            &mut self,
            new_name: T
        ) where T: Into<Cow<'static, str>>;

        fn end_with_timestamp(&mut self, timestamp: SystemTime);
    }
}
mckinnsb commented 1 year ago

I think, in this use case, I might be out of luck. I tried out the feature, and I'm now getting the following error:

302 | /     mock! {
303 | |         BoxedSpan {}
304 | |
305 | |         impl Span for BoxedSpan {
...   |
324 | |         }
325 | |     }
    | |_____^ `Into` cannot be made into an object
    |
    = note: the trait cannot be made into an object because it requires `Self: Sized`
    = note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
    = note: this error originates in the macro `mock` (in Nightly builds, run with -Z macro-backtrace for more info)

I'm guessing this is because this trait cannot be sized because it has two functions that take generic arguments?

asomers commented 1 year ago

Yep. It looks like you'll have to use the "helper method" approach, then.

BaxHugh commented 1 year ago

Here's another suggestion. Although this does affect the public API if what your mocking is a public trait.

Toy model of the problem: This won't work with automock because the T needs to be T: 'static in the individual function due to the inner workings of mock! / automock and it's use of Any.

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

The situation is that we want an API that looks like Foo above, with Foo::foo<T>(), but from the development side, for this to work, it will actually looks like, Foo::<T>::foo(), Why? A) so that mockall will work. B) because perhaps we want to implement for only certain T, and maybe we only wan't some specific types of T (rather than trait bounds) to be supported. but still want to provide a public API that has a generic parameter in the method.

/// Backend for implementing Foo with T being a trait level (rather than method level) generic.
mod _foo {
    #[cfg(test)]
    use mockall::automock;

    /// Sealed development side trait.
    /// (Doesn't have to be sealed though)
    /// trait itself needs to be public as it'll leak into the public API.
    /// But can put it in a private module to make it sealed.
    #[cfg_attr(test, automock)]
    pub trait Foo<T> {
        fn foo(&self, t: T) -> T;
    }
}
#[cfg(test)]
pub use _foo::MockFoo;

/// Trait with T being a generic at the method level, this might be the intended public API.
/// automock is currently not possible here, as explained above.
pub trait Foo {
    fn foo<T>(&self, t: T) -> T
    where
        // Using a sealed trait like this may not be what you want.
        Self: _foo::Foo<T>,
    {
        _foo::Foo::<T>::foo(self, t)
    }
}

/// Note: we can't have a generic implementation of Foo for T where T: _foo::_Foo<U>
/// So we have to explicitly implement structs which impl _foo::Foo<T>.
#[cfg(test)]
impl<T> Foo for MockFoo<T> {}

#[cfg(test)]
mod test {
    use super::*;

    #[test]
    fn test() {
        let mut mock = MockFoo::default();
        // Because the method names are the same in the backend and public trait.
        // It's as if we've just done automock on the public trait, we initially wanted.
        mock.expect_foo().returning(|t| t * 2);
        let _ = mock.foo(1);
    }
}
asomers commented 10 months ago

Closing for lack of feedback. Reopen if your question isn't fully answered.