-
Notifications
You must be signed in to change notification settings - Fork 7
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
Detect non-virtual calls in Received.InOrder method #108
Comments
@dtchepak @zvirja @alexandrnikitin I was wondering how analyze the body of Received.InOrder so as it produces no false positives. From my experience I know that people usually use Received.InOrder basically in the way described in docs, so sth like Received.InOrder(() => {
connection.Open();
connection.Closed();
//or
connection.Closed(Arg.Is<int>());
});
// etc but I've seen usages which does sth like Received.InOrder(() =>
{
// padding first
_atomicLong.VolatileWrite(new IntPtr(tail),
RingBufferDescriptor.MakeHeader(RingBufferDescriptor.HeaderLength,
ManyToOneRingBuffer.PaddingMsgTypeId));
// message then
_atomicLong.VolatileWrite(new IntPtr(0), RingBufferDescriptor.MakeHeader(-recordLength, MessageTypeId));
_buffer.Write(RingBufferDescriptor.EncodedMsgOffset(0), chunk);
_atomicInt.VolatileWrite(new IntPtr(0), recordLength);
}); note that RingBufferDescriptor.MakeHeader(-recordLength, MessageTypeId) is used not for order check but for providing argument value to the function. I assume that people might also do some more fancy stuff inside the lambda. So the question is, how should I distinguish Received.InOrder(() =>
{
var x = RingBufferDescriptor.MakeHeader(RingBufferDescriptor.HeaderLength,
ManyToOneRingBuffer.PaddingMsgTypeId)
_atomicLong.VolatileWrite(new IntPtr(tail), x);
}); so this approach is not bulletproof |
Hi @tpodolak , I think this is a very hard problem. To solve the issue found in nsubstitute/NSubstitute#572, it might be sufficient to just ensure all
I don't think this is possible. :-\ Based on your very good analysis, we could try something like:
I think this might be pretty inaccurate, but would hopefully catch simple misuses. Another option is to demand every call be checked, with the error documentation making it clear to extract all real calls outside of the |
Hi @dtchepak
IMO I think that will be annoying for people who does not create substitutes directly via var substitute = Substitute.For<IFoo> for instance with automocks like AutoFixture or any other library. At this point I decided to implement the solution as per your comment
|
Hi @tpodolak , I finally managed to test this (I have limited access to real .NET projects at the moment). One case that was detected looked like this: Received.InOrder(() => {
...
Sut.ViewModel.Show(recipientViewModels);
}); Here Another case found was: Received.InOrder(() -> {
...
foo.Received().SomeCall();
}); This received call triggered both a NS5001 (Received call in Received.InOrder) and NS1005 (Member Received can not be intercepted). |
Thanks for checking @dtchepak. When it comes to Received.InOrder(() => {
...
Sut.ViewModel.Show(recipientViewModels);
}); It if fixed and will not report diagnostics for method/properties inside the calling chain. Received.InOrder(() -> {
...
foo.Received().SomeCall();
}); will still report |
Thanks @tpodolak 👍 |
Originated from nsubstitute/NSubstitute#572
The text was updated successfully, but these errors were encountered: