Replace virtus with regular accessors + testing type-check shim

[pnomolos]

Why

virtus is unmaintained and pulls in a chain of abandoned gems (axiom-types, coercible, descendants_tracker, ice_nine) plus the deprecated thread_safe (superseded by concurrent-ruby). RubyCritic only used a tiny slice of virtus — an attribute DSL on two classes — so carrying that whole dependency tree wasn't justified.

Direct accessor were chosen after the original PR and some deliberation. A custom module was originally over dry-rb deliberately: dry-struct would just trade one transitive tree for another (dry-core, dry-types, dry-logic, zeitwerk...), which runs counter to the goal and to the codebase's recent direction of shedding/leaning out dependencies (e.g. preferring prism over the parser gem, dropping cucumber).

What changed

• Switch Smell and AnalysedModule to plain accessors
• Added a small "micro-Sorbet" type checking DSL that wraps Smell and AnalysedModule only in testing to guarantee types. If the project is interested in adopting Sorbet or Steep this can (and should) be dropped.
• virtus runtime dependency removed; ostruct moved to a development dependency (only used in tests).
• .reek.yml and CHANGELOG.md updated accordingly.

Verification

• Full rake suite is green: tests, reek (0 warnings), and rubocop (no offenses).
• The full virtus dependency chain is confirmed gone from the bundle.
• Behavior verified faithful to virtus: mutable defaults duped per-instance, Float::INFINITY/Symbol defaults preserved, coercion applied on writers, and method overrides intact.

Checklist

• Add an entry to the changelog
• Squash all commits into a single one
• Describe your PR, link issues, etc.

🤖 Generated with Claude Code

[JuanVqz]

Hey @pnomolos, I like the idea of removing a dependency; I'm always all for it. Well, not always, it depends, haha.

I dug into this and noticed only a small set of classes use virtus. Your instinct is right, and your module faithfully keeps virtus's behavior. But what if we go a bit further and just use plain old attr_accessors instead?

The one thing virtus gave us beyond accessors was type coercion. I checked every place these typed attributes get assigned, and the value is always already the right type (complexity/coverage are Floats, duplication += node.mass is integer + integer, and status is always a Symbol).

So coercion never actually converts anything here, which means we can drop it without changing behavior.

Smell would look like:

    attr_accessor :context, :cost, :locations, :message,
                  :score, :status, :type, :analyser

    def initialize(attributes = {})
      @locations = []
      @status = :new
      attributes.each { |name, value| public_send("#{name}=", value) }
    end

And AnalysedModule:

    attr_accessor :coverage, :name, :pathname, :smells, :churn,
                  :committed_at, :complexity, :duplication, :methods_count

    def initialize(attributes = {})
      @coverage    = 0.0
      @smells      = []
      @complexity  = Float::INFINITY
      @duplication = 0
      attributes.each { |name, value| public_send("#{name}=", value) }
    end

Cleanup bonus baked in: smells_count, file_location, file_name, and line_count were declared as attributes and then immediately overridden by real methods. They just vanish from the accessor list, no longer declared twice.

@faisal I'd like to know your opinion on this, if you can.

[pnomolos]

@JuanVqz More than happy to do that - I had considered it but decided to retain behaviour as close to the original as possible. If @faisal agrees I'll rework the PR as suggested 👍

[faisal]

I dug into this and noticed only a small set of classes use virtus. Your instinct is right, and your module faithfully keeps virtus's behavior. But what if we go a bit further and just use plain old attr_accessors instead?

The one thing virtus gave us beyond accessors was type coercion. I checked every place these typed attributes get assigned, and the value is always already the right type (complexity/coverage are Floats, duplication += node.mass is integer + integer, and status is always a Symbol).

I like the thinking here. My only concern is if this is introducing an assumption for the future. What gives us confidence the types will continue to be correct?

Would it make sense add tests that confirm type alignment, so we'd catch a mismatch at development time without adding runtime overhead?

[pnomolos]

I don't know what thoughts are around these parts in regards to Steep or Sorbet, but this screams at me as a prime candidate for that sort of thing.

If you're not interested, though, I can see about adding tests in order to confirm alignment without affecting the runtime 🤔

[JuanVqz]

For now, I think we can keep it simple, and I second adding tests to catch mismatched types.

But feel free to open a small PR for Steep or Sorbet, then we can check what maintainers think about it

btw, I see Claude is coauthoring these changes. I'm not sure if rubycritic's maintainers have an opinion on that

[pnomolos]

I've updated this with plain accessors. For verifying the types, I chose a "micro-Sorbet" approach of a wrapper that wraps the accessors in the test environment and checks every write for the Smell and AnalysedModule classes.

As far as I can tell there's no guidance towards not using an LLM for this repository, but I'm open to rewriting the implementation (it's pretty close to what I would have built, anyway) if there's any pushback :)

[faisal]

That does suggest we should actually have guidelines ... #576