rspec/rspec-mocks — pull request #266 by myronmarston (2013-04-05 05:52:16+00:00)
Title: New syntax

This is my first pass stab at the new syntax discussed in #153.

It's not ready to merge yet but I wanted to start the code review conversation. Notes:

* I decided to call the old syntax the `:direct` syntax because you call methods directly on the object...and the new syntax the `:wrapped` syntax because you wrap the object with a method like `expect` or `allow`. What do others think of these names?
* When we introduced the new syntax to rspec-expectations, we decided to default to both syntaxes enabled. This was necessary because we had supported `expect { }.to` for a long time, so it would have been a backwards incompatibility to only enable `should` by default. This time around, I'm thinking of only enabling the existing `:direct` syntax by default. Generally, I think projects should pick one syntax or the other, and only use both syntaxes during a transition period. There's no need to enable both syntaxes for backwards compatibility like there was for rspec-expectations. On the other hand, it might encourage more folks to try out the new syntax if both are enabled. I'm kinda on the fence here.
* For `any_instance` stubbing, we never figured out what to do for the new syntax. For now, I've gone with `expect_any_instance_of(klass).to receive` and `allow_any_instance_of(klass).to receive` but @alindeman [put together a gist](https://gist.github.com/alindeman/89ec035c97d161c89183) of some other possibilities to consider.
* With the current version of this code, the any instance stuff doesn't actually work when you enable only the `:wrapped` syntax (it works fine when both syntaxes are enabled). The problem is that the way that any instance works, it records `should_receive` and `stub` and then plays it back on the instance later...but with the `:direct` syntax disabled, the methods aren't there to playback onto. I'm not sure of a fix yet.
* I didn't realize this, but the old syntax allowed _any_ object to be `as_null_object`. I personally think that only makes sense on a test double, not on a partial mock. Any objections to only supporting it on test doubles for the new syntax?
* `stub_chain`: Do we need to bring this forward to the new syntax? (It's kind a code smell, IMO).
* The old syntax supported `obj.stub(:msg_1 => 1, :msg_2 => 2)` but I can't think of a good way to support that with the new syntax. (We'll still support `double(:msg_1 => 1, :msg_2 => 2)`, of course). Any objects to not bringing that forward to the new syntax?
* Lots of documentation updates are needed.

Please review and give your feedback!