Print better expectation messages on stable #425
Conversation
|
Looks like it does regress on generic arguments though: |
There was a problem hiding this comment.
Sorry it took me so long to review this. It's just been hard to find a solid block of time. I must say, this autoderef stuff is a big hack. But such a useful hack! It's so very useful, I don't think that Mockall can do without it. My biggest question is, "why bother with Display"? I think that Debug is more appropriate for this usage. If a type implements both, I think that Debug should get higher priority. And types that implement Display but not Debug are rarer than hens' teeth, so I don't think we should worry about them. Can we just eliminate the Display part?
Also, it needs a CHANGELOG entry.
mockall_derive/src/mock_function.rs
Outdated
| let fstr = format!("{}({})", name, fields); | ||
| quote!(std::format!(#fstr, #(::mockall::MaybeDebugger(&#argnames)),*)) | ||
| quote!(std::format!(#fstr, #((&&&::mockall::MaybeDebugger(&#argnames)).debug_string()),*)) |
There was a problem hiding this comment.
debug_string() will allocate space for a string and populate it. Then std::format! will do the same. That's less efficient than the original. Could we change the autoderef magic to create an actual Debug implementation like before, rather than use this debug_string method?
There was a problem hiding this comment.
played around with this some more and got something that wont allocate unnecessarily!
I agree :) it's definitely an abuse of the type system, but very nice to not have to depend on nightly
This was based on my very narrow use case of printing Date which, at the time, had a much better display implementation. Looks like they recently improved this so my point is now moot. time-rs/time#509. Removed the display trait |
jefftt
force-pushed
the
no-nightly-mock-function
branch
from
656e07c
to
699355f
Compare
jefftt
force-pushed
the
no-nightly-mock-function
branch
from
28d209a
to
a91e62c
Compare
jefftt
force-pushed
the
no-nightly-mock-function
branch
from
a91e62c
to
d55a656
Compare
|
sorry for the late reply, i've rebased the branch on latest master |
ah sorry, i totally missed that... |
|
@asomers hey sorry to bother you, but do you plan on including this in a release in the future? would like to use it in my projects without having to do a git override |
|
Yes, I will include it in the next feature release. I haven't published one yet because I'm still not completely happy with the concretize feature. See https://users.rust-lang.org/t/how-to-pass-multiple-items-to-a-derive-macro/91266 for more discussion about that. |
This change allows better messages for "No matching expectation found" on stable. This is a great user experience upgrade as it allows this behavior without requiring running with the nightly toolchain. This MR applies the generalized technique for autoderef specialization described here