RSpec/StubbedMock false positive

[ngouy]

I have a case where I want to play more extensively with the arguments that are passed to a call
I don't stubb the return value at all
Yet it is suggested I use allow over expect which really is out of question given I absolutely want to know if the method was called, with specific args

# I want to test that .some_method is called on MyClass with some arguments
expect(MyClass).to receive(:some_method).with("a", "b") # rubocop is fine

expect(MyClass).to receive(:some_method) do |arg_1, arg_2| do
  # I want to do some complicated stuff. (It's exactly what I do, the point is I cannot use argument matchers)
  row = Table.find_by(id: arg_1)
  expect(arg_1).to_not be nil
end # rubocop tell me I should use `allow(MyClass)`, which is absolutely not the point

[pirj]

Thanks for reporting!

It is pretty legitimate to use it to verify arguments. Even though the doc suggests using allow, I'd say that it's risky, as if the method won't be called at all, argument checks won't even run, and the example would deceivingly pass.

I'd propose ignoring the block form if it captures arguments. Would you like to submit a PR, @ngouy ?

[ngouy]

@pirj Thanks for confirming! (so fast 🏎️ 💨 )

Maybe we could ignore as long as we don't call and_return maybe?

Also I could try, it won't be before 2023. things are getting busy for OEY (as anyone else anyway I guess)
So yes, but not urgent

[pirj]

Even though we haveRSpec/ReturnFromStub that suggests using and_return by default, it has an alternative block style that would fix all stubs to use the block form. This would effectively disable RSpec/StubbedMock if we would just ignore this form.

On the other hand, if the block form captures arguments, we can assume that the block is used to verify arguments, not to stub the response.

[ngouy]

So what is your directive?
Seems to me like the 2 options have pro and cons
And I am not sure to be knowledgable enough to pick one over the other myself

[pirj]

The AST difference is:

receive(:foo) { bar }

(block
  (send nil :receive
    (sym :foo))
  (args)
  (send nil :bar))

receive(:foo) { |x| bar }

(block
  (send nil :receive
    (sym :foo))
  (args
    (procarg0
      (arg :x)))
  (send nil :bar))

The node matcher is:

(block #message_expectation? args _)  # receive(:foo) { 'bar' }

And what we need is to teach it to make a distinction between no block args and present args. I may be mistaken, but it feels that args -> (args) would do, see this doc.

As a last touch, add an example that would show that we ignore message expectations with a block that accepts arguments here.

[ydah]

@ngouy This issue has been corrected #1546 . Please wait for the next release.