Fix ActiveRecord STI format lookup and guard __use__ cycles

[eitoball]

What

Resolves comma_formats from the class hierarchy at call time instead of a stale snapshot, and guards Comma::Extractor#__use__ against circular style references.

Why

Closes #161. Object.comma_formats used class_attribute plus an inherited-hook shallow dup, so a subclass's format table was a snapshot frozen at the moment the subclass was defined — any later change to the superclass's comma blocks (e.g. reopening the class) silently failed to reach subclasses, including ActiveRecord STI subclasses. Separately, __use__ re-evaluated named styles with no cycle detection, so a circular :a uses :b, :b uses :a reference would stack-overflow instead of raising a clear error.

Changes

• lib/comma/object.rb: drop class_attribute :comma_formats and the inherited dup hook; store each class's own DSL entries in a private, never-auto-inherited @own_comma_formats ivar, and resolve the effective comma_formats by walking self → superclass → ... at call time, merging so a subclass's own style entries override the same-named parent style while other styles still fall through. Restore an instance-level comma_formats reader (self.class.comma_formats) to preserve the pre-existing public behavior for receivers that are themselves Class objects (see the #94 regression spec).
• lib/comma/extractor.rb: add Comma::CircularStyleReference < StandardError and a @style_stack that __use__ pushes/pops around each nested expansion, raising immediately if a style already on the stack is requested again.
• spec/comma/comma_spec.rb: add a regression spec proving a superclass comma block reopened after a subclass is defined is still picked up (ChildClassNoComma-style, but redefining the parent block post-hoc), and two new specs for a two-style cycle and a direct self-reference, both expecting Comma::CircularStyleReference.
• spec/comma/rails/active_record_spec.rb: remove the stale # FIXME comment on the Cat#to_comma STI example — it now passes.

Verified against the base spec suite plus the active_record-only appraisal gemfiles for 6.0.6, 6.1.7.6, 7.0.8, and 7.1.3 (all green), and rubocop on the changed files (no offenses).

[Copilot]

Pull request overview

This PR fixes two correctness gaps in Comma’s DSL execution: (1) STI subclasses now resolve comma formats from the live class hierarchy at call time (instead of an inherited snapshot), and (2) __use__ style composition is protected against circular references by raising a dedicated Comma::CircularStyleReference error.

Changes:

• Reworked format storage/lookup to merge per-class format definitions across the superclass chain at call time (fixes STI/reopened-superclass cases).
• Added circular __use__ detection with a new exception type and expanded specs to cover both indirect and direct cycles.
• Updated ActiveRecord STI spec by removing the now-stale FIXME comment (test expectation remains the same).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
lib/comma/object.rb	Replaces inherited snapshotting with call-time hierarchy merge for comma_formats.
lib/comma/extractor.rb	Adds CircularStyleReference and a style stack to detect __use__ cycles.
spec/comma/comma_spec.rb	Adds regression tests for reopened superclass formats and circular __use__ chains.
spec/comma/rails/active_record_spec.rb	Removes outdated comment now that STI format lookup is fixed.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lib/comma/extractor.rb:32

• __use__ pushes to @style_stack and then pops after instance_eval, but the pop won’t run if the used style raises (and the caller rescues). That can leave the stack unbalanced and cause subsequent __use__ calls in the same extraction to incorrectly raise CircularStyleReference.

Wrap the instance_eval in an ensure so the stack is always restored.

      format = @formats.fetch(style) { raise "No comma format defined for style #{style}" }

      @style_stack.push(style)

lib/comma/object.rb:16

• Object.comma_formats now rebuilds a merged Hash on every call by walking the superclass chain. Since Comma::Generator calls to_comma once per row, this introduces per-row allocations and extra work compared to the previous O(1) lookup.

Consider memoizing the resolved formats per class (and invalidating when comma is called on any class in the hierarchy) to avoid recomputing/allocating on hot paths.

    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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.