From f5f95f5b0368c05eb4beb7b9c5bcdcf6415aaf82 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 4 Feb 2016 09:53:23 -0500 Subject: [PATCH] Restore 'owned' stubbed methods only in ruby 2.3+. Fixes #1057. In b94fe10e34ececd8cda34c480d996f036a58c557, we changed behavior to to restore 'owned' stubbed methods for all rubies. This doesn't work for all rubies because Method#owner for aliased methods is inconsistent [1]. Since, ruby 2.3+ broke our prior backup/restore mechanism, it's safer to change the original behavior for ruby 2.3+ only. A @klass can have methods implemented (see Method#owner) in @klass or inherited from a superclass. In ruby 2.2 and earlier, we can copy a method regardless of the 'owner' and restore it to @klass after because a call to 'super' from @klass's copied method would end up calling the original class's superclass's method. With the commit below [2], available starting in 2.3.0, ruby changed this behavior and a call to 'super' from the method copied to @klass will call @klass's superclass method, which is the original implementer of this method! This leads to very strange errors if @klass's copied method calls 'super', since it would end up calling itself, the original method implemented in @klass's superclass. For ruby 2.3 and above, we need to only restore methods that @klass originally owned. [1] https://gist.github.com/jrafanie/0e5b8a9f4e8b4f84913f [2] https://github.com/ruby/ruby/commit/c8854d2ca4be9ee6946e6d17b0e17d9ef130ee81 --- lib/rspec/mocks/any_instance/recorder.rb | 23 ++++++++++++++++++++++- spec/rspec/mocks/any_instance_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/rspec/mocks/any_instance/recorder.rb b/lib/rspec/mocks/any_instance/recorder.rb index 8651e02ef..38fc16f23 100644 --- a/lib/rspec/mocks/any_instance/recorder.rb +++ b/lib/rspec/mocks/any_instance/recorder.rb @@ -193,7 +193,28 @@ def restore_original_method!(method_name) alias_method_name = build_alias_method_name(method_name) @klass.class_exec(@backed_up_method_owner) do |backed_up_method_owner| remove_method method_name - alias_method method_name, alias_method_name if backed_up_method_owner[method_name.to_sym] == self + + # A @klass can have methods implemented (see Method#owner) in @klass + # or inherited from a superclass. In ruby 2.2 and earlier, we can copy + # a method regardless of the 'owner' and restore it to @klass after + # because a call to 'super' from @klass's copied method would end up + # calling the original class's superclass's method. + # + # With the commit below, available starting in 2.3.0, ruby changed + # this behavior and a call to 'super' from the method copied to @klass + # will call @klass's superclass method, which is the original + # implementer of this method! This leads to very strange errors + # if @klass's copied method calls 'super', since it would end up + # calling itself, the original method implemented in @klass's + # superclass. + # + # For ruby 2.3 and above, we need to only restore methods that + # @klass originally owned. + # + # https://github.com/ruby/ruby/commit/c8854d2ca4be9ee6946e6d17b0e17d9ef130ee81 + if RUBY_VERSION < "2.3" || backed_up_method_owner[method_name.to_sym] == self + alias_method method_name, alias_method_name + end remove_method alias_method_name end end diff --git a/spec/rspec/mocks/any_instance_spec.rb b/spec/rspec/mocks/any_instance_spec.rb index a3b6a7e27..3cfa69e51 100644 --- a/spec/rspec/mocks/any_instance_spec.rb +++ b/spec/rspec/mocks/any_instance_spec.rb @@ -5,12 +5,22 @@ class GrandparentClass def foo(_a) 'bar' end + + def grandparent_method + 1 + end end class ParentClass < GrandparentClass def foo super(:a) end + + def caller_of_parent_aliased_method + parent_aliased_method + end + + alias parent_aliased_method grandparent_method end class ChildClass < ParentClass @@ -174,6 +184,17 @@ def private_method; :private_method_return_value; end end end + context 'aliased methods' do + it 'tracks aliased method calls' do + instance = AnyInstanceSpec::ParentClass.new + expect_any_instance_of(AnyInstanceSpec::ParentClass).to receive(:parent_aliased_method).with(no_args).and_return(2) + expect(instance.caller_of_parent_aliased_method).to eq 2 + + reset_all + expect(instance.caller_of_parent_aliased_method).to eq 1 + end + end + context "with argument matching" do before do allow_any_instance_of(klass).to receive(:foo).with(:param_one, :param_two).and_return(:result_one)