nrxus / faux

Struct mocking library for Rust
https://nrxus.github.io/faux/
MIT License
420 stars 13 forks source link

Adding #[faux::create] breaks trait impls that access fields #24

Closed TedDriggs closed 3 years ago

TedDriggs commented 4 years ago

I was testing the current behavior with traits to understand #20, and I think I found a case where #[faux::create] breaks otherwise valid code.

Repro

#[faux::create]
pub struct Owned {
    foo: usize,
}

impl fmt::Display for Owned {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Original {}", self.foo)
                                   // ^~~ E0609 No field `foo` on type `&Owned`. 
                                   //     Available fields are `0`.
    }
}

Having looked at the expanded output, I understand what's happening here: faux is rewriting the Owned struct to be a newtype struct around the MaybeFaux, but it doesn't touch the trait impl. As a result, it's impossible to have trait impls that directly reference fields while using faux.

Direct field access prevents mocking in that case, but I don't know that people will want to add accessors for all their fields if they want to mock the struct for a particular test case.

The only solution I could think of is a #[faux::real] attribute or something be added to trait impl blocks (and maybe other places) so that they're rewritten to use the new name for that type?

nrxus commented 4 years ago

Yes. As mentioned in the docs for create:

If #[methods] is not used for an impl block, methods inside the impl may not use any of its fields.

The idea of a #[faux::real] for impl blocks that do not need to be mocked but need to access fields is interesting but I think in terms of priority I would put #10 above this, since it would also fix the issue, albeit in a way that would allow the trait implementation to be mocked. Would you find #[faux::real] useful even after allowing the mocking of trait implementations?

fwiw, I think implementing Display in an object to mock is a weird. Display is used to show data, ideally structs that represent data should not be mocked, only structs that represent behavior. (i.e., don't mock a UserData struct, but rather a UserClient). Although this issue exists for any trait implementations right now so that's off-topic.

TedDriggs commented 4 years ago

I think #[faux::real] would still be useful for the case where I don't need that trait mocked but I do still need my code to compile.

nrxus commented 4 years ago

What cases do you think #[faux::real] would cover that #[faux::methods] wouldn't?

I understand that you might not want to mock the trait but I think the end result is the same.

Case 1: There is a fake instance

let mock = MyStruct::faux();
format!("{}", mock);

[faux::real]

This panics because you are trying to call a method on a fake, and this method was marked as not mockable with #[faux::real]

[faux::methods]

This panics because you are trying to call a method on a fake, and the method was not mocked.

The only difference I can see is that #[faux::methods] would allow it to be mockable if the user wants to. But if it isn't mocked, then both cases would panic.

Case 2: There is a real instance

let my_struct = MyStruct::new(/*some data*/);
format!("{}", my_struct);

[faux::real]

This is real instance, so we can call for the trait implementation no problem.

[faux::methods]

This is a real instance so faux proxies to the real implementation.

In either case, the real Display method is called.

I could be missing something though! Comments are a pretty lossy way of communication so my apologies if I missed something obvious.

nrxus commented 3 years ago

Closing this as I think tagging it with #[methods] but not mocking it would be the same as having a #[real]attribute.