Replace virtus with an internal attributes module

[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.

A custom module was chosen 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

• New internal RubyCritic::Attributes module reproducing only the used virtus subset: an attribute macro, type coercion, per-instance duped defaults, and a hash-accepting initializer.
• Smell and AnalysedModule now include RubyCritic::Attributes instead of Virtus.model.
• 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 🤔