From b0f0721bc996a486e9fa6e94c35ce0925e342871 Mon Sep 17 00:00:00 2001 From: Eito Katagiri Date: Sun, 16 Aug 2026 17:46:15 +0900 Subject: [PATCH 1/4] fix: resolve comma format from class hierarchy at call time Replace the class_attribute + inherited-hook dup snapshot with a per-class instance variable and a call-time walk up the superclass chain, so a subclass's comma_formats always reflects the current state of its ancestors instead of a stale copy taken when the subclass was first defined. --- lib/comma/object.rb | 35 +++++++++++++++++++++++++++-------- spec/comma/comma_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/lib/comma/object.rb b/lib/comma/object.rb index 0b5884ff..d239e75b 100644 --- a/lib/comma/object.rb +++ b/lib/comma/object.rb @@ -4,19 +4,38 @@ require 'comma/header_extractor' class Object - class_attribute :comma_formats - class << self def comma(style = :default, &block) - (self.comma_formats ||= {})[style] = block + own_comma_formats[style] = block + end + + def comma_formats + classes_with_own_formats.reverse_each.each_with_object({}) do |klass, formats| + formats.merge!(klass.instance_variable_get(:@own_comma_formats)) + end + end + + private + + def own_comma_formats + @own_comma_formats ||= {} end - def inherited(subclass) - super - subclass.comma_formats = self.comma_formats ? self.comma_formats.dup : {} + def classes_with_own_formats + classes = [] + klass = self + while klass + classes << klass if klass.instance_variable_defined?(:@own_comma_formats) + klass = klass.superclass + end + classes end end + def comma_formats + self.class.comma_formats + end + def to_comma(style = :default) extract_with(Comma::DataExtractor, style) end @@ -29,11 +48,11 @@ def to_comma_headers(style = :default) def extract_with(extractor_class, style = :default) raise_unless_style_exists(style) - extractor_class.new(self, style, self.comma_formats).results + extractor_class.new(self, style, comma_formats).results end def raise_unless_style_exists(style) - return if self.comma_formats && self.comma_formats[style] + return if comma_formats[style] raise "No comma format for class #{self.class} defined for style #{style}" end diff --git a/spec/comma/comma_spec.rb b/spec/comma/comma_spec.rb index 5bf1acb3..81b10e4e 100644 --- a/spec/comma/comma_spec.rb +++ b/spec/comma/comma_spec.rb @@ -296,6 +296,29 @@ class ChildClassNoComma < MySuperClass it 'should return and array of data content, as defined in comma block in super class, if not present in child' do expect(@childNoComma.to_comma).to eq(%w[super-content]) end + + it 'should reflect changes to the superclass format made after the subclass was defined' do + class ReopenedSuperClass + attr_accessor :content + comma do; content end + + def initialize(content) + @content = 'super-' + content + end + end + + class ReopenedChildNoComma < ReopenedSuperClass + end + + ReopenedSuperClass.class_eval do + comma do + content { |c| c.upcase } + end + end + + child = ReopenedChildNoComma.new('content') + expect(child.to_comma).to eq(%w[SUPER-CONTENT]) + end end end From 2376a2a1e1d4b5e9ac1d8ba096616bc580799d4d Mon Sep 17 00:00:00 2001 From: Eito Katagiri Date: Sun, 16 Aug 2026 17:46:47 +0900 Subject: [PATCH 2/4] test: drop stale FIXME now that STI format lookup is call-time resolved Cat#to_comma correctly returns Super-Kitty now that comma_formats walks the class hierarchy at call time instead of relying on a snapshot taken when the subclass was defined. --- spec/comma/rails/active_record_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/comma/rails/active_record_spec.rb b/spec/comma/rails/active_record_spec.rb index c6dc9675..f5bdd1d8 100644 --- a/spec/comma/rails/active_record_spec.rb +++ b/spec/comma/rails/active_record_spec.rb @@ -191,7 +191,6 @@ class Cat < Animal expect(@dog.to_comma).to eq %w[Dog-Rex] end - # FIXME: this one is failing - the comma block from Dog is executed instead of the one from the super class it 'should return and array of data content, as defined in comma block in super class, if not present in child' do expect(@cat.to_comma).to eq %w[Super-Kitty] end From 612a125d02411bedfac7d12d95ce15a099d30a46 Mon Sep 17 00:00:00 2001 From: Eito Katagiri Date: Sun, 16 Aug 2026 17:47:31 +0900 Subject: [PATCH 3/4] fix: raise Comma::CircularStyleReference on cyclic __use__ instead of stack overflow Track the chain of styles currently being expanded and raise a dedicated error the moment __use__ is asked to re-enter a style already on the stack, instead of recursing until the interpreter's stack overflows. --- lib/comma/extractor.rb | 11 ++++++++++- spec/comma/comma_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/comma/extractor.rb b/lib/comma/extractor.rb index 125d0b0b..73a0eb2a 100644 --- a/lib/comma/extractor.rb +++ b/lib/comma/extractor.rb @@ -1,12 +1,15 @@ # frozen_string_literal: true module Comma + class CircularStyleReference < StandardError; end + class Extractor def initialize(instance, style, formats) @instance = instance @style = style @formats = formats @results = [] + @style_stack = [style] end def results @@ -19,8 +22,14 @@ def id(*args, &block) end def __use__(style) - # TODO: prevent infinite recursion + if @style_stack.include?(style) + chain = (@style_stack + [style]).join(' -> ') + raise Comma::CircularStyleReference, "Circular __use__ reference detected: #{chain}" + end + + @style_stack.push(style) instance_eval(&@formats[style]) + @style_stack.pop end private diff --git a/spec/comma/comma_spec.rb b/spec/comma/comma_spec.rb index 81b10e4e..98e3ad33 100644 --- a/spec/comma/comma_spec.rb +++ b/spec/comma/comma_spec.rb @@ -345,3 +345,30 @@ class ReopenedChildNoComma < ReopenedSuperClass its(:size) { should eq(3) } it { should eq(['Programming Ruby', 'Foo, Inc.', 'The Pickaxe book']) } end + +describe Comma, '__use__ keyword with a circular reference' do + it 'should raise Comma::CircularStyleReference instead of overflowing the stack' do + obj = Class.new(Struct.new(:id, :title)) do + comma :a do + title + __use__ :b + end + + comma :b do + __use__ :a + end + end.new(1, 'Programming Ruby') + + expect { obj.to_comma(:a) }.to raise_error(Comma::CircularStyleReference, /a -> b -> a/) + end + + it 'should raise Comma::CircularStyleReference for direct self-reference' do + obj = Class.new(Struct.new(:id)) do + comma :a do + __use__ :a + end + end.new(1) + + expect { obj.to_comma(:a) }.to raise_error(Comma::CircularStyleReference, /a -> a/) + end +end From 5d530e9c58f219109b73046b60ea3818619f0402 Mon Sep 17 00:00:00 2001 From: Eito Katagiri Date: Sun, 16 Aug 2026 17:50:25 +0900 Subject: [PATCH 4/4] style: satisfy rubocop for the new STI regression spec Use the symbol-to-proc form for the value transform and mark the STI describe block as an accepted long block, matching the existing convention elsewhere in this file. --- spec/comma/comma_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/comma/comma_spec.rb b/spec/comma/comma_spec.rb index 98e3ad33..f0249bf7 100644 --- a/spec/comma/comma_spec.rb +++ b/spec/comma/comma_spec.rb @@ -263,7 +263,7 @@ def initialize(content, created_at = Time.now, updated_at = Time.now) end end - describe 'on objects using Single Table Inheritance' do + describe 'on objects using Single Table Inheritance' do # rubocop:disable Metrics/BlockLength before do class MySuperClass attr_accessor :content @@ -312,7 +312,7 @@ class ReopenedChildNoComma < ReopenedSuperClass ReopenedSuperClass.class_eval do comma do - content { |c| c.upcase } + content(&:upcase) end end