From ffaee7a82717a801108d607cc58db57298092e6c 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 | 48 +++++++++++++++++++++++++++++++--------- spec/comma/comma_spec.rb | 23 +++++++++++++++++++ 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/lib/comma/object.rb b/lib/comma/object.rb index 0b5884ff..785f600b 100644 --- a/lib/comma/object.rb +++ b/lib/comma/object.rb @@ -4,19 +4,43 @@ 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 : {} + # `self` here is always a Class: `comma`/`comma_formats` live on Object's + # singleton class, which only plain Class objects mirror, so a bare + # Module can never reach this method to begin with. `Class#superclass` + # in turn only ever yields another Class or nil, so `klass` stays a + # Class for the whole walk. + 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 @@ -28,13 +52,15 @@ def to_comma_headers(style = :default) private def extract_with(extractor_class, style = :default) - raise_unless_style_exists(style) - extractor_class.new(self, style, self.comma_formats).results + formats = comma_formats + raise_unless_style_exists(formats, style) + extractor_class.new(self, style, formats).results end - def raise_unless_style_exists(style) - return if self.comma_formats && self.comma_formats[style] + def raise_unless_style_exists(formats, style) + return if formats[style] - raise "No comma format for class #{self.class} defined for style #{style}" + receiver = is_a?(Module) ? self : self.class + raise "No comma format for class #{receiver} defined for style #{style}" end 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 b5118870921799f824e5a019cd36645871d673da 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 e23f9c4b11f5f0fad64d939bbb1facc518267de6 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 | 18 ++++++++++++++++-- spec/comma/comma_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/lib/comma/extractor.rb b/lib/comma/extractor.rb index 125d0b0b..01a02a78 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,19 @@ def id(*args, &block) end def __use__(style) - # TODO: prevent infinite recursion - instance_eval(&@formats[style]) + if @style_stack.include?(style) + chain = (@style_stack + [style]).join(' -> ') + raise Comma::CircularStyleReference, "Circular __use__ reference detected: #{chain}" + end + + format = @formats.fetch(style) { raise "No comma format defined for style #{style}" } + + @style_stack.push(style) + begin + instance_eval(&format) + ensure + @style_stack.pop + end 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 0d322cf6950f84b70156e3a52157d198c5785882 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