From dccf33a6684fd7663550b74f808f22de374173bb Mon Sep 17 00:00:00 2001 From: Andrew Lazarus Date: Tue, 28 Jun 2022 14:44:30 -0700 Subject: [PATCH] only set static dep on defining class --- CHANGES.md | 26 ++++++++++++++++++++++++++ lib/interjectable.rb | 12 +++++++----- lib/interjectable/version.rb | 2 +- spec/interjectable_spec.rb | 11 +++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index b38702b..811b28f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,31 @@ # Change Log +# v1.2.0 + +- Ruby 3.x made it an error to override a class variable in a parent class. There was a bug with `inject_static` where + if a subclass was the first to call a static dependency, the class variable would only be set on that subclass (and + children of that subsclass). If the injecting class then called the static dependency, it would override the already + set child's class variable, which is now an error. + + ```ruby + class Parent + include Interjectable + inject_static(:boom) { "goats" } + end + + class Child < Parent; end + + Child.boom # => sets Child's @@boom = "goats" + Parent.boom # => sets Parent's @@boom = "goats" and *clear* Child's @@boom. + Child.boom # => Error on Ruby 3.x because you are trying to read an overriden class variable. + ``` + + Fix: always set the class variable on the class that called `inject_static`. + +# v1.1.3 + +- Fix `test_inject` for sub-sub-classes. + # v1.1.2 - Fix visibility issue with `Module.define_method` for Ruby < 2.5.0. diff --git a/lib/interjectable.rb b/lib/interjectable.rb index bbc905b..7342084 100644 --- a/lib/interjectable.rb +++ b/lib/interjectable.rb @@ -64,26 +64,28 @@ def inject_static(dependency, &default_block) raise MethodAlreadyDefined, "#{dependency} is already defined" end + injecting_class = self + cvar_name = :"@@#{dependency}" setter = :"#{dependency}=" define_method(setter) do |value| - self.class.send(setter, value) + injecting_class.send(setter, value) end define_singleton_method(setter) do |value| - class_variable_set(cvar_name, value) + injecting_class.class_variable_set(cvar_name, value) end define_method(dependency) do - self.class.send(dependency) + injecting_class.send(dependency) end define_singleton_method(dependency) do if class_variable_defined?(cvar_name) - class_variable_get(cvar_name) + injecting_class.class_variable_get(cvar_name) else - class_variable_set(cvar_name, instance_eval(&default_block)) + injecting_class.class_variable_set(cvar_name, instance_eval(&default_block)) end end end diff --git a/lib/interjectable/version.rb b/lib/interjectable/version.rb index 3575801..67a8492 100644 --- a/lib/interjectable/version.rb +++ b/lib/interjectable/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Interjectable - VERSION = "1.1.3" + VERSION = "1.2.0" end diff --git a/spec/interjectable_spec.rb b/spec/interjectable_spec.rb index c9d17ca..6a7e0f0 100644 --- a/spec/interjectable_spec.rb +++ b/spec/interjectable_spec.rb @@ -170,6 +170,17 @@ expect(subclass_instance.static_dependency).to eq(:some_other_value) expect(subclass.static_dependency).to eq(:some_other_value) end + + it "does not error when lazily setting the dep from a subclass first" do + expect(subclass.static_dependency).to eq(:some_value) + expect(klass.static_dependency).to eq(:some_value) + expect(subclass.static_dependency).to eq(:some_value) + end + + it "only defines the class variable on the injecting class" do + expect(subclass.static_dependency).to eq(:some_value) + expect(klass.class_variable_get(:@@static_dependency)).to eq(:some_value) + end end end end