Correctly unstubs methods stubbed with #any_instance

* Instances stubbed with `#any_instance` would not be usable after the
  test finished because `MethodDouble` would stash the implementation of
  the method already overridden by `AnyInstance::Recorder`. When the
  test finished, that implementation would be restored on the object's
  singleton class, and any future calls to it would blow up with a stack
  overflow.
* This fix only stashes methods if they are defined on the object's
  singleton class to begin with; `AnyInstance::Recorder` defines a
  method on the object's class so that method will not be stashed.
* If there is no method on the object's singleton class, RSpec can
  safely define one there without stashing the original implementation.
  At the end of the test, the method is simply removed entirely from the
  singleton class. Any original implementation defined in the object's
  ancestor chain will show through again.
* This issue cannot be fixed on MRI 1.8.6 because it does not support
  `Method#owner`. However, `#any_instance` itself is not supported on
  1.8.6 for the same reason. The fix should not negatively affect 1.8.6,
  though, because the fallback behavior is to stash the method in all
  cases (which was the original behavior).

* This commit also refactors the stashing behavior out into its own
  object. While not explicitly necessary, it helped me reason about the
  fix much easier than when all the responsibility was in `MethodDouble`
  (which also has other responsibilities).

* Fixes #167
* Closes #182

lib/rspec/mocks/framework.rb

@@ -3,6 +3,7 @@
 # object in the system.
 
 require 'rspec/mocks/extensions/instance_exec'
+require 'rspec/mocks/stashed_instance_method'
 require 'rspec/mocks/method_double'
 require 'rspec/mocks/methods'
 require 'rspec/mocks/argument_matchers'

lib/rspec/mocks/method_double.rb

@@ -10,7 +10,9 @@ def initialize(object, method_name, proxy)
         @method_name = method_name
         @object = object
         @proxy = proxy
-        @stashed = false
+
+        @stashed_method = StashedInstanceMethod.new(object_singleton_class, @method_name)
+        @method_is_proxied = false
         store(:expectations, [])
         store(:stubs, [])
       end
@@ -43,73 +45,39 @@ def object_singleton_class
         class << @object; self; end
       end
 
-      # @private
-      def obfuscate(method_name)
-        "obfuscated_by_rspec_mocks__#{method_name}"
-      end
-
-      # @private
-      def stashed_method_name
-        obfuscate(method_name)
-      end
-
-      # @private
-      def object_responds_to?(method_name)
-        if @proxy.already_proxied_respond_to?
-          @object.__send__(obfuscate(:respond_to?), method_name)
-        elsif method_name == :respond_to?
-          @proxy.already_proxied_respond_to
-        else
-          @object.respond_to?(method_name, true)
-        end
-      end
-
       # @private
       def configure_method
         RSpec::Mocks::space.add(@object) if RSpec::Mocks::space
         warn_if_nil_class
-        unless @stashed
-          stash_original_method
-          define_proxy_method
-        end
-      end
-
-      # @private
-      def stash_original_method
-        stashed = stashed_method_name
-        orig = @method_name
-        object_singleton_class.class_eval do
-          alias_method(stashed, orig) if method_defined?(orig) || private_method_defined?(orig)
-        end
-        @stashed = true
+        @stashed_method.stash
+        define_proxy_method
       end
 
       # @private
       def define_proxy_method
-        method_name = @method_name
-        visibility_for_method = "#{visibility} :#{method_name}"
-        object_singleton_class.class_eval(<<-EOF, __FILE__, __LINE__)
-          def #{method_name}(*args, &block)
-            __mock_proxy.message_received :#{method_name}, *args, &block
+        return if @method_is_proxied
+
+        object_singleton_class.class_eval <<-EOF, __FILE__, __LINE__ + 1
+          def #{@method_name}(*args, &block)
+            __mock_proxy.message_received :#{@method_name}, *args, &block
           end
-        #{visibility_for_method}
+          #{visibility_for_method}
         EOF
+        @method_is_proxied = true
+      end
+
+      # @private
+      def visibility_for_method
+        "#{visibility} :#{method_name}"
       end
 
       # @private
       def restore_original_method
-        if @stashed
-          method_name = @method_name
-          stashed_method_name = self.stashed_method_name
-          object_singleton_class.instance_eval do
-            remove_method method_name
-            if method_defined?(stashed_method_name) || private_method_defined?(stashed_method_name)
-              alias_method method_name, stashed_method_name
-              remove_method stashed_method_name
-            end
-          end
-          @stashed = false
-        end
+        return unless @method_is_proxied
+
+        object_singleton_class.__send__(:remove_method, @method_name)
+        @stashed_method.restore
+        @method_is_proxied = false
       end
 
       # @private

lib/rspec/mocks/stashed_instance_method.rb

@@ -0,0 +1,60 @@
+# @private
+class StashedInstanceMethod
+  def initialize(klass, method)
+    @klass = klass
+    @method = method
+
+    @method_is_stashed = false
+  end
+
+  # @private
+  def stash
+    return if !method_defined_directly_on_klass? || @method_is_stashed
+
+    @klass.__send__(:alias_method, stashed_method_name, @method)
+    @method_is_stashed = true
+  end
+
+  private
+
+  # @private
+  def method_defined_directly_on_klass?
+    method_defined_on_klass? && method_owned_by_klass?
+  end
+
+  # @private
+  def method_defined_on_klass?
+    @klass.method_defined?(@method) || @klass.private_method_defined?(@method)
+  end
+
+  if ::UnboundMethod.method_defined?(:owner)
+    # @private
+    def method_owned_by_klass?
+      @klass.instance_method(@method).owner == @klass
+    end
+  else
+    # @private
+    def method_owned_by_klass?
+      # On 1.8.6, which does not support Method#owner, we have no choice but
+      # to assume it's defined on the klass even if it may be defined on
+      # a superclass.
+      true
+    end
+  end
+
+  # @private
+  def stashed_method_name
+    "obfuscated_by_rspec_mocks__#{@method}"
+  end
+
+  public
+
+  # @private
+  def restore
+    return unless @method_is_stashed
+
+    @klass.__send__(:alias_method, @method, stashed_method_name)
+    @klass.__send__(:remove_method, stashed_method_name)
+    @method_is_stashed = false
+  end
+end

spec/rspec/mocks/any_instance_spec.rb

@@ -859,6 +859,19 @@ class RSpec::SampleRspecTestClass;end
           http_request_class.new.existing_method.should == "foo"
         end
       end
+
+      context "when used after the test has finished" do
+        it "restores the original behavior of a stubbed method" do
+          klass.any_instance.stub(:existing_method).and_return(:stubbed_return_value)
+
+          instance = klass.new
+          instance.existing_method.should == :stubbed_return_value
+
+          RSpec::Mocks.verify
+
+          instance.existing_method.should == :existing_method_return_value
+        end
+      end
     end
   end
 end

spec/rspec/mocks/stashed_instance_method_spec.rb

@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe StashedInstanceMethod do
+  class ExampleClass
+    def hello
+      :hello_defined_on_class
+    end
+  end
+
+  def singleton_class_for(obj)
+    class << obj; self; end
+  end
+
+  it "stashes the current implementation of an instance method so it can be temporarily replaced" do
+    obj = Object.new
+    def obj.hello; :hello_defined_on_singleton_class; end;
+
+    stashed_method = StashedInstanceMethod.new(singleton_class_for(obj), :hello)
+    stashed_method.stash
+
+    def obj.hello; :overridden_hello; end
+    expect(obj.hello).to eql :overridden_hello
+
+    stashed_method.restore
+    expect(obj.hello).to eql :hello_defined_on_singleton_class
+  end
+
+  it "stashes private instance methods" do
+    obj = Object.new
+    def obj.hello; :hello_defined_on_singleton_class; end;
+    singleton_class_for(obj).__send__(:private, :hello)
+
+    stashed_method = StashedInstanceMethod.new(singleton_class_for(obj), :hello)
+    stashed_method.stash
+
+    def obj.hello; :overridden_hello; end
+    stashed_method.restore
+    expect(obj.send(:hello)).to eql :hello_defined_on_singleton_class
+  end
+
+  it "only stashes methods directly defined on the given class, not its ancestors" do
+    obj = ExampleClass.new
+
+    stashed_method = StashedInstanceMethod.new(singleton_class_for(obj), :hello)
+    stashed_method.stash
+
+    def obj.hello; :overridden_hello; end;
+    expect(obj.hello).to eql :overridden_hello
+
+    stashed_method.restore
+    expect(obj.hello).to eql :overridden_hello
+  end
+end