Merge pull request #326 from alpaca-tc/call_self_inherited

Fixed a bug that overwrote existing self.inherited method definitions.

CHANGELOG.md

@@ -14,6 +14,7 @@ _No breaking changes!_
 **Project enhancements:**
 
 - Fixed a bug that overwrote existing self.extended method definitions. [[#324]](https://github.com/panorama-ed/memo_wise/pull/314)
+- Fixed a bug that overwrote existing self.inherited method definitions. [[#325]](https://github.com/panorama-ed/memo_wise/pull/315)
 
 ## [v1.8.0](https://github.com/panorama-ed/memo_wise/compare/v1.7.0...v1.8.0) - 2023-10-25
 

README.md

@@ -118,15 +118,15 @@ Results using Ruby 3.2.2:
 
 |Method arguments|`Dry::Core`\* (1.0.1)|`Memery` (1.5.0)|
 |--|--|--|
-|`()` (none)|0.59x|3.65x|
-|`(a)`|1.49x|8.38x|
-|`(a, b)`|1.21x|6.55x|
-|`(a:)`|1.43x|13.14x|
-|`(a:, b:)`|1.20x|10.32x|
-|`(a, b:)`|1.18x|10.10x|
-|`(a, *args)`|0.79x|1.61x|
-|`(a:, **kwargs)`|0.75x|1.97x|
-|`(a, *args, b:, **kwargs)`|0.67x|1.37x|
+|`()` (none)|0.60x|3.58x|
+|`(a)`|1.37x|7.41x|
+|`(a, b)`|1.20x|6.43x|
+|`(a:)`|1.47x|13.60x|
+|`(a:, b:)`|1.20x|10.55x|
+|`(a, b:)`|1.21x|10.36x|
+|`(a, *args)`|0.79x|1.52x|
+|`(a:, **kwargs)`|0.77x|2.02x|
+|`(a, *args, b:, **kwargs)`|0.69x|1.38x|
 
 \* `Dry::Core`
 [may cause incorrect behavior caused by hash collisions](https://github.com/dry-rb/dry-core/issues/63).
@@ -135,15 +135,15 @@ Results using Ruby 2.7.8 (because these gems raise errors in Ruby 3.x):
 
 |Method arguments|`DDMemoize` (1.0.0)|`Memoist` (0.16.2)|`Memoized` (1.1.1)|`Memoizer` (1.0.3)|
 |--|--|--|--|--|
-|`()` (none)|23.10x|2.28x|23.77x|2.69x|
-|`(a)`|21.57x|14.28x|20.61x|12.05x|
-|`(a, b)`|19.05x|13.55x|17.83x|11.68x|
-|`(a:)`|30.29x|23.54x|25.22x|21.69x|
-|`(a:, b:)`|27.79x|22.83x|23.78x|21.08x|
-|`(a, b:)`|26.61x|21.40x|21.63x|19.81x|
-|`(a, *args)`|3.16x|2.26x|3.08x|1.97x|
-|`(a:, **kwargs)`|2.74x|2.25x|2.47x|2.10x|
-|`(a, *args, b:, **kwargs)`|2.18x|1.84x|1.93x|1.73x|
+|`()` (none)|22.09x|2.35x|23.72x|2.60x|
+|`(a)`|20.98x|14.43x|21.20x|12.20x|
+|`(a, b)`|17.45x|12.94x|17.69x|11.13x|
+|`(a:)`|29.80x|23.38x|25.17x|21.57x|
+|`(a:, b:)`|27.00x|22.26x|23.30x|20.91x|
+|`(a, b:)`|25.91x|21.20x|21.88x|19.51x|
+|`(a, *args)`|3.07x|2.27x|3.17x|1.95x|
+|`(a:, **kwargs)`|2.74x|2.28x|2.51x|2.10x|
+|`(a, *args, b:, **kwargs)`|2.14x|1.84x|1.95x|1.72x|
 
 You can run benchmarks yourself with:
 

lib/memo_wise.rb

@@ -86,6 +86,14 @@ def extended(base)
   end
   private_constant(:CreateMemoWiseStateOnExtended)
 
+  module CreateMemoWiseStateOnInherited
+    def inherited(subclass)
+      MemoWise::InternalAPI.create_memo_wise_state!(subclass)
+      super
+    end
+  end
+  private_constant(:CreateMemoWiseStateOnInherited)
+
   # @private
   #
   # Private setup method, called automatically by `prepend MemoWise` in a class.
@@ -165,12 +173,11 @@ def memo_wise(method_name_or_hash)
 
         # This ensures that a memoized method defined on a parent class can
         # still be used in a child class.
-        klass.module_eval <<~HEREDOC, __FILE__, __LINE__ + 1
-          def inherited(subclass)
-            super
-            MemoWise::InternalAPI.create_memo_wise_state!(subclass)
-          end
-        HEREDOC
+        if klass.is_a?(Class) && !klass.singleton_class?
+          klass.singleton_class.prepend(CreateMemoWiseStateOnInherited)
+        else
+          klass.prepend(CreateMemoWiseStateOnInherited)
+        end
 
         raise ArgumentError, "#{method_name.inspect} must be a Symbol" unless method_name.is_a?(Symbol)
 

spec/memo_wise_spec.rb

@@ -851,5 +851,119 @@ def no_args_counter
         expect(instance.no_args_counter).to eq(1)
       end
     end
+
+    context "with target defined self.inherited" do
+      context "when target is class" do
+        let(:klass) do
+          Class.new do
+            prepend MemoWise
+
+            def self.inherited(subclass)
+              super
+              subclass.instance_variable_set(:@inherited_called, true)
+            end
+
+            def no_args
+              @no_args_counter = no_args_counter + 1
+            end
+            memo_wise :no_args
+
+            def no_args_counter
+              @no_args_counter ||= 0
+            end
+          end
+        end
+
+        it "calls defined self.inherited" do
+          inherited = Class.new(klass)
+          expect(inherited.instance_variable_get(:@inherited_called)).to be(true)
+
+          instance = inherited.new
+          expect(Array.new(4) { instance.no_args }).to all(eq(1))
+          expect(instance.no_args_counter).to eq(1)
+        end
+
+        it "doesn't define #inherited" do
+          expect(klass).to be_respond_to(:inherited)
+          expect(klass.new).to_not be_respond_to(:inherited)
+        end
+      end
+
+      context "when target is singleton class" do
+        let(:klass) do
+          Class.new do
+            class << self
+              prepend MemoWise
+
+              def inherited(subclass)
+                super
+                subclass.instance_variable_set(:@inherited_called, true)
+              end
+
+              def no_args
+                @no_args_counter = no_args_counter + 1
+              end
+              memo_wise :no_args
+
+              def no_args_counter
+                @no_args_counter ||= 0
+              end
+            end
+          end
+        end
+
+        it "calls defined self.inherited" do
+          inherited = Class.new(klass)
+          expect(inherited.instance_variable_get(:@inherited_called)).to be(true)
+
+          expect(Array.new(4) { inherited.no_args }).to all(eq(1))
+          expect(inherited.no_args_counter).to eq(1)
+        end
+
+        it "doesn't define #inherited" do
+          expect(klass).to be_respond_to(:inherited)
+          expect(klass.new).to_not be_respond_to(:inherited)
+        end
+      end
+
+      context "when target is module" do
+        let(:klass) do
+          mod = Module.new do
+            prepend MemoWise
+
+            def inherited(subclass)
+              super
+              subclass.instance_variable_set(:@inherited_called, true)
+            end
+
+            def no_args
+              @no_args_counter = no_args_counter + 1
+            end
+            memo_wise :no_args
+
+            def no_args_counter
+              @no_args_counter ||= 0
+            end
+          end
+
+          klass = Class.new
+          klass.extend(mod)
+          klass
+        end
+
+        it "calls defined self.inherited" do
+          inherited = Class.new(klass)
+          expect(inherited.instance_variable_get(:@inherited_called)).to be(true)
+
+          expect(Array.new(4) { inherited.no_args }).to all(eq(1))
+          expect(inherited.no_args_counter).to eq(1)
+        end
+
+        it "doesn't define #inherited" do
+          expect(klass).to be_respond_to(:inherited)
+          expect(klass.new).to_not be_respond_to(:inherited)
+        end
+      end
+    end
   end
 end