From 6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8 Mon Sep 17 00:00:00 2001
From: Edouard CHIN <edouard.chin@shopify.com>
Date: Tue, 3 Jul 2018 23:15:30 -0400
Subject: Use class_eval or instance_eval when triggering lazy load hooks:

- When lazy load hooks were triggered we were using
  `Object.instance_eval` which evaluates the block in the context of
  the class being passed. Most of the time that class was a
  `Class`. If one wants to define a instance method on the class then
  it wasn't possible.

  ```ruby
    class A; end;
    A.instance_eval do
      def foo
        puts 'bar'
      end
    end
    A.new.foo #> NoMethodError: undefined method `foo`
    A.foo #> bar
  ```
- This PR checks what object is passed when triggering the hooks and
  either call `class_eval` or `instance_eval`. My rational and assumptions being
  that if an instance of a class is passed, then the blocks needs to
  evaluate in the context of that instance (i.e. defining a method
  should only define it on that instance).
  On the other hand, if a Class or Module is passed when triggering
  hooks, then defining a method should define it on the class itself
- #32776 Pushed me to introduce this change
---
 .../lib/active_support/lazy_load_hooks.rb          |  6 ++-
 activesupport/test/lazy_load_hooks_test.rb         | 49 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/activesupport/lib/active_support/lazy_load_hooks.rb b/activesupport/lib/active_support/lazy_load_hooks.rb
index dc8080c469..97fe8a06c1 100644
--- a/activesupport/lib/active_support/lazy_load_hooks.rb
+++ b/activesupport/lib/active_support/lazy_load_hooks.rb
@@ -68,7 +68,11 @@ module ActiveSupport
           if options[:yield]
             block.call(base)
           else
-            base.instance_eval(&block)
+            if base.is_a?(Class) || base.is_a?(Module)
+              base.class_eval(&block)
+            else
+              base.instance_eval(&block)
+            end
           end
         end
       end
diff --git a/activesupport/test/lazy_load_hooks_test.rb b/activesupport/test/lazy_load_hooks_test.rb
index 721d44d0c1..50a703e49f 100644
--- a/activesupport/test/lazy_load_hooks_test.rb
+++ b/activesupport/test/lazy_load_hooks_test.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "abstract_unit"
+require "active_support/core_ext/module/remove_method"
 
 class LazyLoadHooksTest < ActiveSupport::TestCase
   def test_basic_hook
@@ -125,6 +126,54 @@ class LazyLoadHooksTest < ActiveSupport::TestCase
     assert_equal 7, i
   end
 
+  def test_hook_uses_class_eval_when_base_is_a_class
+    ActiveSupport.on_load(:uses_class_eval) do
+      def first_wrestler
+        "John Cena"
+      end
+    end
+
+    ActiveSupport.run_load_hooks(:uses_class_eval, FakeContext)
+    assert_equal "John Cena", FakeContext.new(0).first_wrestler
+  ensure
+    FakeContext.remove_possible_method(:first_wrestler)
+  end
+
+  def test_hook_uses_class_eval_when_base_is_a_module
+    mod = Module.new
+    ActiveSupport.on_load(:uses_class_eval2) do
+      def last_wrestler
+        "Dwayne Johnson"
+      end
+    end
+    ActiveSupport.run_load_hooks(:uses_class_eval2, mod)
+
+    klass = Class.new do
+      include mod
+    end
+
+    assert_equal "Dwayne Johnson", klass.new.last_wrestler
+  end
+
+  def test_hook_uses_instance_eval_when_base_is_an_instance
+    ActiveSupport.on_load(:uses_instance_eval) do
+      def second_wrestler
+        "Hulk Hogan"
+      end
+    end
+
+    context = FakeContext.new(1)
+    ActiveSupport.run_load_hooks(:uses_instance_eval, context)
+
+    assert_raises NoMethodError do
+      FakeContext.new(2).second_wrestler
+    end
+    assert_raises NoMethodError do
+      FakeContext.second_wrestler
+    end
+    assert_equal "Hulk Hogan", context.second_wrestler
+  end
+
 private
 
   def incr_amt
-- 
cgit v1.2.3