From f41cfcb76371b5faa8f2d931d3d7fa22fd98d027 Mon Sep 17 00:00:00 2001
From: Myron Marston <myron.marston@gmail.com>
Date: Mon, 11 Mar 2013 23:32:20 -0700
Subject: [PATCH] Clarify relationship between different method stub actions.

In 2.13.0 only `and_yield` and `and_return` could be combined, since
that was the only combination case that was specified by an example.
This was a regression, as reported by a user in #230.

I tried here to fully specify all of the various combinations of stub
actions. Notes:

* `and_return`, `and_raise` and `and_throw` are "terminal" actions
  in the sense that they terminate the method. They _must_ happen last
  and it is impossible to support more than one of these. Hence, we allow
  only one of these, and allow them to be overridden. We also return `nil`
  from these methods to discourage further stub configuration.
* `and_call_original` is a special case that doesn't make sense to be
  combined with any of the others. Once you've set it up, this causes
  any further instructions to raise an error.
* `and_yield` is treated as an "initial" action. Yielding doesn't exit
  a method the way the terminal actions do. It is the only initial action.
  Calling it multiple times sets up multiple yields.
* Setting a block implementation (possible by passing a block to almost
  any method on the fluent interface) sets the block as the "inner" action.
  It runs between the configured yields (if there are any) and the configured
  terminal action (if there is one). My thinking here is that in many cases,
  users use a block implementation to specify a return value, essentially
  making it like a terminal action (so it should come after the `and_yield`
  actions), but in other cases, the user may just use a block for a side
  effect, and may configure an terminal action as well. Only one block
  implementation is supported and it can be overridden.
---
 Changelog.md                                  |   4 +
 lib/rspec/mocks/message_expectation.rb        | 119 +++++++----
 ...bining_implementation_instructions_spec.rb | 197 ++++++++++++++++++
 3 files changed, 283 insertions(+), 37 deletions(-)
 create mode 100644 spec/rspec/mocks/combining_implementation_instructions_spec.rb

diff --git a/Changelog.md b/Changelog.md
index 300020a45..74b73e86c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -14,6 +14,10 @@ Enhancements:
 Bug Fixes:
 
 * Bypass RSpec::Mocks::Syntax when mass-assigning stubs via double(). (Paul Annesley)
+* Allow a block implementation to be used in combination with
+  `and_yield`, `and_raise`, `and_return` or `and_throw`. This got fixed
+  in 2.13.1 but failed to get merged into master for the 2.14.0.rc1
+  release (Myron Marston).
 
 ### 2.14.0.rc1 / 2013-05-27
 [full changelog](http://github.com/rspec/rspec-mocks/compare/v2.13.0...v2.14.0.rc1)
diff --git a/lib/rspec/mocks/message_expectation.rb b/lib/rspec/mocks/message_expectation.rb
index 95b785d6f..f28ce6dcb 100644
--- a/lib/rspec/mocks/message_expectation.rb
+++ b/lib/rspec/mocks/message_expectation.rb
@@ -10,7 +10,7 @@ class MessageExpectation
 
       # @private
       def initialize(error_generator, expectation_ordering, expected_from, method_double,
-                     expected_received_count=1, opts={}, &implementation)
+                     expected_received_count=1, opts={}, &implementation_block)
         @error_generator = error_generator
         @error_generator.opts = opts
         @expected_from = expected_from
@@ -24,10 +24,9 @@ def initialize(error_generator, expectation_ordering, expected_from, method_doub
         @args_to_yield = []
         @failed_fast = nil
         @eval_context = nil
-        @implementation = implementation
 
-        @initial_implementation_logic = nil
-        @terminal_implementation_logic = nil
+        @implementation = Implementation.new
+        self.inner_implementation_action = implementation_block
       end
 
       # @private
@@ -78,10 +77,12 @@ def and_return(*values, &implementation)
 
         if implementation
           # TODO: deprecate `and_return { value }`
-          @implementation = implementation
+          self.inner_implementation_action = implementation
         else
-          self.terminal_implementation_logic = AndReturnImplementation.new(values)
+          self.terminal_implementation_action = AndReturnImplementation.new(values)
         end
+
+        nil
       end
 
       # Tells the object to delegate to the original unmodified method
@@ -99,7 +100,7 @@ def and_call_original
         if @method_double.object.is_a?(RSpec::Mocks::TestDouble)
           @error_generator.raise_only_valid_on_a_partial_mock(:and_call_original)
         else
-          @implementation = @method_double.original_method
+          @implementation = AndCallOriginalImplementation.new(@method_double.original_method)
         end
       end
 
@@ -129,7 +130,8 @@ def and_raise(exception = RuntimeError, message = nil)
           exception = message ? exception.exception(message) : exception.exception
         end
 
-        @implementation = Proc.new { raise exception }
+        self.terminal_implementation_action = Proc.new { raise exception }
+        nil
       end
 
       # @overload and_throw(symbol)
@@ -143,7 +145,8 @@ def and_raise(exception = RuntimeError, message = nil)
       #   car.stub(:go).and_throw(:out_of_gas)
       #   car.stub(:go).and_throw(:out_of_gas, :level => 0.1)
       def and_throw(*args)
-        @implementation = Proc.new { throw(*args) }
+        self.terminal_implementation_action = Proc.new { throw(*args) }
+        nil
       end
 
       # Tells the object to yield one or more args to a block when the message
@@ -155,7 +158,7 @@ def and_throw(*args)
       def and_yield(*args, &block)
         yield @eval_context = Object.new if block
         @args_to_yield << args
-        self.initial_implementation_logic = AndYieldImplementation.new(@args_to_yield, @eval_context, @error_generator)
+        self.initial_implementation_action = AndYieldImplementation.new(@args_to_yield, @eval_context, @error_generator)
         self
       end
 
@@ -177,8 +180,8 @@ def invoke(parent_stub, *args, &block)
         @order_group.handle_order_constraint self
 
         begin
-          if @implementation
-            @implementation.call(*args, &block)
+          if implementation.present?
+            implementation.call(*args, &block)
           elsif parent_stub
             parent_stub.invoke(nil, *args, &block)
           end
@@ -292,7 +295,7 @@ def raise_out_of_order_error
       #   cart.add(Book.new(:isbn => 1934356379))
       #   # => passes
       def with(*args, &block)
-        @implementation = block if block_given? unless args.empty?
+        self.inner_implementation_action = block if block_given? unless args.empty?
         @argument_list_matcher = ArgumentListMatcher.new(*args, &block)
         self
       end
@@ -304,7 +307,7 @@ def with(*args, &block)
       #
       #   dealer.should_receive(:deal_card).exactly(10).times
       def exactly(n, &block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         set_expected_received_count :exactly, n
         self
       end
@@ -320,7 +323,7 @@ def at_least(n, &block)
           RSpec.deprecate "at_least(0) with should_receive", :replacement => "stub"
         end
 
-        @implementation = block if block
+        self.inner_implementation_action = block
         set_expected_received_count :at_least, n
         self
       end
@@ -332,7 +335,7 @@ def at_least(n, &block)
       #
       #   dealer.should_receive(:deal_card).at_most(10).times
       def at_most(n, &block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         set_expected_received_count :at_most, n
         self
       end
@@ -345,7 +348,7 @@ def at_most(n, &block)
       #   dealer.should_receive(:deal_card).at_least(10).times
       #   dealer.should_receive(:deal_card).at_most(10).times
       def times(&block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         self
       end
 
@@ -353,7 +356,7 @@ def times(&block)
       # Allows an expected message to be received any number of times.
       def any_number_of_times(&block)
         RSpec.deprecate "any_number_of_times", :replacement => "stub"
-        @implementation = block if block
+        self.inner_implementation_action = block
         @expected_received_count = :any
         self
       end
@@ -374,7 +377,7 @@ def never
       #
       #   car.should_receive(:go).once
       def once(&block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         set_expected_received_count :exactly, 1
         self
       end
@@ -385,7 +388,7 @@ def once(&block)
       #
       #   car.should_receive(:go).twice
       def twice(&block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         set_expected_received_count :exactly, 2
         self
       end
@@ -398,7 +401,7 @@ def twice(&block)
       #   api.should_receive(:run).ordered
       #   api.should_receive(:finish).ordered
       def ordered(&block)
-        @implementation = block if block
+        self.inner_implementation_action = block
         @order_group.register(self)
         @ordered = true
         self
@@ -436,21 +439,16 @@ def set_expected_received_count(relativity, n)
                                    end
       end
 
-      def initial_implementation_logic=(logic)
-        @initial_implementation_logic = logic
-        update_implementation
+      def initial_implementation_action=(action)
+        implementation.initial_action = action
       end
 
-      def terminal_implementation_logic=(logic)
-        @terminal_implementation_logic = logic
-        update_implementation
+      def inner_implementation_action=(action)
+        implementation.inner_action = action if action
       end
 
-      def update_implementation
-        @implementation = Implementation.new(
-          @initial_implementation_logic,
-          @terminal_implementation_logic
-        ).method(:call)
+      def terminal_implementation_action=(action)
+        implementation.terminal_action = action
       end
     end
 
@@ -480,7 +478,7 @@ def initialize(args_to_yield, eval_context, error_generator)
         @error_generator = error_generator
       end
 
-      def call(*args_to_ignore, block)
+      def call(*args_to_ignore, &block)
         return if @args_to_yield.empty? && @eval_context.nil?
 
         @error_generator.raise_missing_block_error @args_to_yield unless block
@@ -502,7 +500,7 @@ def initialize(values_to_return)
         @values_to_return = values_to_return
       end
 
-      def call(*args_to_ignore, block)
+      def call(*args_to_ignore, &block)
         if @values_to_return.size > 1
           @values_to_return.shift
         else
@@ -515,12 +513,59 @@ def call(*args_to_ignore, block)
     # any number of sub-implementations.
     # @private
     class Implementation
-      def initialize(*implementations)
-        @implementations = implementations.compact
+      attr_accessor :initial_action, :inner_action, :terminal_action
+
+      def call(*args, &block)
+        actions.map do |action|
+          action.call(*args, &block)
+        end.last
+      end
+
+      def present?
+        actions.any?
+      end
+
+    private
+
+      def actions
+        [initial_action, inner_action, terminal_action].compact
+      end
+    end
+
+    # Represents an `and_call_original` implementation.
+    # @private
+    class AndCallOriginalImplementation
+      def initialize(method)
+        @method = method
+      end
+
+      CannotModifyFurtherError = Class.new(StandardError)
+
+      def initial_action=(value)
+        raise cannot_modify_further_error
+      end
+
+      def inner_action=(value)
+        raise cannot_modify_further_error
+      end
+
+      def terminal_action=(value)
+        raise cannot_modify_further_error
+      end
+
+      def present?
+        true
       end
 
       def call(*args, &block)
-        @implementations.map { |i| i.call(*args, block) }.last
+        @method.call(*args, &block)
+      end
+
+    private
+
+      def cannot_modify_further_error
+        CannotModifyFurtherError.new "This method has already been configured " +
+          "to call the original implementation, and cannot be modified further."
       end
     end
   end
diff --git a/spec/rspec/mocks/combining_implementation_instructions_spec.rb b/spec/rspec/mocks/combining_implementation_instructions_spec.rb
new file mode 100644
index 000000000..7204459c9
--- /dev/null
+++ b/spec/rspec/mocks/combining_implementation_instructions_spec.rb
@@ -0,0 +1,197 @@
+require 'spec_helper'
+
+module RSpec
+  module Mocks
+    describe "Combining implementation instructions" do
+      it 'can combine and_yield and and_return' do
+        dbl = double
+        dbl.stub(:foo).and_yield(5).and_return(3)
+
+        expect { |b|
+          expect(dbl.foo(&b)).to eq(3)
+        }.to yield_with_args(5)
+      end
+
+      describe "combining and_yield, a block implementation and and_return" do
+        def verify_combined_implementation
+          dbl = double
+          (yield dbl).and_yield(5).and_return(3)
+
+          expect { |b|
+            expect(dbl.foo(:arg, &b)).to eq(3)
+          }.to yield_with_args(5)
+
+          expect(@block_called).to be_true
+        end
+
+        it 'works when passing a block to `stub`' do
+          verify_combined_implementation do |dbl|
+            dbl.stub(:foo) { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `with`' do
+          verify_combined_implementation do |dbl|
+            dbl.stub(:foo).with(:arg) { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `exactly`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).exactly(:once) { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `at_least`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).at_least(:once) { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `at_most`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).at_most(:once) { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `times`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).exactly(1).times { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `any_number_of_times`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).any_number_of_times { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `once`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).once { @block_called = true }
+          end
+        end
+
+        it 'works when passing a block to `twice`' do
+          the_double = nil
+
+          verify_combined_implementation do |dbl|
+            the_double = dbl
+            dbl.should_receive(:foo).twice { @block_called = true }
+          end
+
+          the_double.foo { |a| } # to ensure it is called twice
+        end
+
+        it 'works when passing a block to `ordered`' do
+          verify_combined_implementation do |dbl|
+            dbl.should_receive(:foo).ordered { @block_called = true }
+          end
+        end
+      end
+
+      it 'can combine and_yield and and_return with a block' do
+        dbl = double
+        dbl.stub(:foo).and_yield(5).and_return { :return }
+
+        expect { |b|
+          expect(dbl.foo(&b)).to eq(:return)
+        }.to yield_with_args(5)
+      end
+
+      it 'can combine and_yield and and_raise' do
+        dbl = double
+        dbl.stub(:foo).and_yield(5).and_raise("boom")
+
+        expect { |b|
+          expect { dbl.foo(&b) }.to raise_error("boom")
+        }.to yield_with_args(5)
+      end
+
+      it 'can combine and_yield, a block implementation and and_raise' do
+        dbl = double
+        block_called = false
+        dbl.stub(:foo) { block_called = true }.and_yield(5).and_raise("boom")
+
+        expect { |b|
+          expect { dbl.foo(&b) }.to raise_error("boom")
+        }.to yield_with_args(5)
+
+        expect(block_called).to be_true
+      end
+
+      it 'can combine and_yield and and_throw' do
+        dbl = double
+        dbl.stub(:foo).and_yield(5).and_throw(:bar)
+
+        expect { |b|
+          expect { dbl.foo(&b) }.to throw_symbol(:bar)
+        }.to yield_with_args(5)
+      end
+
+      it 'can combine and_yield, a block implementation and and_throw' do
+        dbl = double
+        block_called = false
+        dbl.stub(:foo) { block_called = true }.and_yield(5).and_throw(:bar)
+
+        expect { |b|
+          expect { dbl.foo(&b) }.to throw_symbol(:bar)
+        }.to yield_with_args(5)
+
+        expect(block_called).to be_true
+      end
+
+      it 'returns `nil` from all terminal actions to discourage further configuration' do
+        expect(double.stub(:foo).and_return(1)).to be_nil
+        expect(double.stub(:foo).and_raise("boom")).to be_nil
+        expect(double.stub(:foo).and_throw(:foo)).to be_nil
+      end
+
+      it 'allows the terminal action to be overriden' do
+        dbl = double
+        stubbed_double = dbl.stub(:foo)
+
+        stubbed_double.and_return(1)
+        expect(dbl.foo).to eq(1)
+
+        stubbed_double.and_return(3)
+        expect(dbl.foo).to eq(3)
+
+        stubbed_double.and_raise("boom")
+        expect { dbl.foo }.to raise_error("boom")
+
+        stubbed_double.and_throw(:bar)
+        expect { dbl.foo }.to throw_symbol(:bar)
+      end
+
+      it 'allows the inner implementation block to be overriden' do
+        dbl = double
+        stubbed_double = dbl.stub(:foo)
+
+        stubbed_double.with(:arg) { :with_block }
+        expect(dbl.foo(:arg)).to eq(:with_block)
+
+        stubbed_double.at_least(:once) { :at_least_block }
+        expect(dbl.foo(:arg)).to eq(:at_least_block)
+      end
+
+      it 'raises an error if `and_call_original` is followed by any other instructions' do
+        dbl = [1, 2, 3]
+        stubbed = dbl.stub(:size)
+        stubbed.and_call_original
+
+        msg_fragment = /cannot be modified further/
+
+        expect { stubbed.and_yield }.to raise_error(msg_fragment)
+        expect { stubbed.and_return(1) }.to raise_error(msg_fragment)
+        expect { stubbed.and_raise("a") }.to raise_error(msg_fragment)
+        expect { stubbed.and_throw(:bar) }.to raise_error(msg_fragment)
+
+        expect { stubbed.once { } }.to raise_error(msg_fragment)
+
+        expect(dbl.size).to eq(3)
+      end
+    end
+  end
+end
+