Skip to content

Commit a642b76

Browse files
pnomolosclaude
andcommitted
chore(deps): replace virtus with internal attributes module
The `virtus` gem is unmaintained and drags in a chain of abandoned transitive dependencies: axiom-types, coercible, descendants_tracker, ice_nine, and 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 tree was not justified. Introduce a small internal `RubyCritic::Attributes` module that reproduces only the used subset: an `attribute` class macro with reader/writer generation, type coercion (Float/Integer/Array/Symbol), per-instance duped mutable defaults, and a hash-accepting initializer. `Smell` and `AnalysedModule` now include it instead of `Virtus.model`. Drop the `virtus` runtime dependency and move `ostruct` to a development dependency (only referenced in tests). Update `.reek.yml` and the changelog accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> refactor: simplify Smell/AnalysedModule and add test-only type checking Replace the internal attributes module (from the previous commit) with plain `attr_accessor` plus a hash-accepting initializer, per PR feedback to go simpler. This keeps the gem free of virtus and its abandoned transitive dependencies without introducing any custom attribute DSL. Add a test-only "micro-Sorbet" helper (test/support/type_check.rb) that wraps the writers of these plain accessors and raises on unexpected types, recovering the lightweight type safety virtus used to provide. It is loaded only through test_helper, never required by production code, and never shipped (the gemspec packages lib only). A spec value may be a single class or a union, nil is always permitted, and late-bound class names (e.g. FakeFS::Pathname) are matched by name. Adjust .reek.yml/.rubocop.yml for the plain accessors (writable-attribute and initializer smells) and remove the now-stale entries that referenced the deleted attributes module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 07f5e50 commit a642b76

9 files changed

Lines changed: 269 additions & 29 deletions

File tree

.reek.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,23 @@ detectors:
9292
- RubyCritic::RakeTask#paths
9393
- RubyCritic::RakeTask#verbose
9494
- RubyCritic::RakeTask#fail_on_error
95+
- RubyCritic::Smell#context
96+
- RubyCritic::Smell#cost
97+
- RubyCritic::Smell#locations
98+
- RubyCritic::Smell#message
99+
- RubyCritic::Smell#score
100+
- RubyCritic::Smell#status
101+
- RubyCritic::Smell#type
102+
- RubyCritic::Smell#analyser
103+
- RubyCritic::AnalysedModule#coverage
104+
- RubyCritic::AnalysedModule#name
105+
- RubyCritic::AnalysedModule#pathname
106+
- RubyCritic::AnalysedModule#smells
107+
- RubyCritic::AnalysedModule#churn
108+
- RubyCritic::AnalysedModule#committed_at
109+
- RubyCritic::AnalysedModule#complexity
110+
- RubyCritic::AnalysedModule#duplication
111+
- RubyCritic::AnalysedModule#methods_count
95112
DuplicateMethodCall:
96113
exclude:
97114
- RubyCritic::Analyser::Churn#run
@@ -113,6 +130,7 @@ detectors:
113130
- RubyCritic::Reporter#self.report_generator_class
114131
- RubyCritic::SourceLocator#deduplicate_symlinks
115132
- RubyCritic::Command::Compare#compare_branches
133+
- RubyCritic::AnalysedModule#initialize
116134
FeatureEnvy:
117135
exclude:
118136
- Parser::AST::Node#module_name
@@ -165,6 +183,7 @@ detectors:
165183
- RubyCritic::SourceLocator#ruby_file?
166184
TooManyInstanceVariables:
167185
exclude:
186+
- RubyCritic::AnalysedModule
168187
- RubyCritic::Generator::Html::CodeFile
169188
- RubyCritic::Generator::Html::Overview
170189
- RubyCritic::Generator::Html::SmellsIndex

.rubocop.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ Metrics/MethodLength:
5454
Exclude:
5555
- 'lib/rubycritic/configuration.rb'
5656
- 'test/integration/integration_test_helper.rb'
57+
- 'test/support/type_check.rb'
58+
59+
Naming/MethodName:
60+
Exclude:
61+
- 'test/support/type_check.rb'
5762

5863
Naming/RescuedExceptionsVariableName:
5964
Exclude:
@@ -78,6 +83,7 @@ Style/OneClassPerFile:
7883
- 'test/analysers_test_helper.rb'
7984
- 'test/fakefs_helper.rb'
8085
- 'test/test_helper.rb'
86+
- 'test/support/type_check.rb'
8187

8288
Style/OpenStructUse:
8389
Exclude:

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* [FEATURE] ...
77

88
* [CHANGE] Replace Aruba with direct API calls in specs (by [@faisal][])
9+
* [CHANGE] Replace the unmaintained `virtus` dependency with plain Ruby accessors, dropping its abandoned transitive dependencies (`axiom-types`, `coercible`, `descendants_tracker`, `ice_nine`, and the deprecated `thread_safe`). Also move `ostruct` to a development dependency since it is only used in tests. (by [@pnomolos][])
910
* [CHANGE] Replace all Cucumber features with Minitest/Spec specs (by [@faisal][])
1011
* [BUGFIX] Add `lang="en"` to the report's `<html>` element, give the menu-toggle anchor an `aria-label`, and make the per-rating summary IDs unique. Fixes 17 WCAG 2.1 AA structural errors on `overview.html`. (by [@MarcusAl][])
1112

@@ -507,3 +508,4 @@
507508
[@exoego]: https://github.com/exoego
508509
[@raff-s]: https://github.com/raff-s
509510
[@MarcusAl]: https://github.com/MarcusAl
511+
[@pnomolos]: https://github.com/pnomolos

lib/rubycritic/core/analysed_module.rb

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,22 @@
11
# frozen_string_literal: true
22

3-
require 'virtus'
43
require 'rubycritic/core/rating'
54

65
module RubyCritic
76
class AnalysedModule
8-
include Virtus.model
9-
107
# Complexity is reduced by a factor of 25 when calculating cost
118
COMPLEXITY_FACTOR = 25.0
129

13-
attribute :coverage, Float, default: 0.0
14-
attribute :name
15-
attribute :smells_count
16-
attribute :file_location
17-
attribute :file_name
18-
attribute :line_count
19-
attribute :pathname
20-
attribute :smells, Array, default: []
21-
attribute :churn
22-
attribute :committed_at
23-
attribute :complexity, Float, default: Float::INFINITY
24-
attribute :duplication, Integer, default: 0
25-
attribute :methods_count
10+
attr_accessor :coverage, :name, :pathname, :smells, :churn, :committed_at, :complexity, :duplication, :methods_count
11+
12+
def initialize(attributes = {})
13+
@coverage = 0.0
14+
@smells = []
15+
@complexity = Float::INFINITY
16+
@duplication = 0
17+
@methods_count = 0
18+
attributes.each { |name, value| public_send("#{name}=", value) }
19+
end
2620

2721
def path
2822
@path ||= pathname.to_s

lib/rubycritic/core/smell.rb

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,16 @@
11
# frozen_string_literal: true
22

3-
require 'virtus'
43
require 'rubycritic/core/location'
54

65
module RubyCritic
76
class Smell
8-
include Virtus.model
9-
10-
attribute :context
11-
attribute :cost
12-
attribute :locations, Array, default: []
13-
attribute :message
14-
attribute :score
15-
attribute :status, Symbol, default: :new
16-
attribute :type
17-
attribute :analyser
7+
attr_accessor :context, :cost, :locations, :message, :score, :status, :type, :analyser
8+
9+
def initialize(attributes = {})
10+
@locations = []
11+
@status = :new
12+
attributes.each { |name, value| public_send("#{name}=", value) }
13+
end
1814

1915
FLAY_DOCS_URL = 'http://docs.seattlerb.org/flay/'.freeze
2016
FLOG_DOCS_URL = 'http://docs.seattlerb.org/flog/'.freeze

rubycritic.gemspec

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ Gem::Specification.new do |spec|
3434
spec.add_dependency 'flay', '~> 2.13'
3535
spec.add_dependency 'flog', '~> 4.7'
3636
spec.add_dependency 'launchy', '>= 2.5.2'
37-
spec.add_dependency 'ostruct'
3837
spec.add_dependency 'parser', '>= 3.3.0.5'
3938
spec.add_dependency 'prism', '>= 1.6.0'
4039
spec.add_dependency 'rainbow', '~> 3.1.1'
@@ -43,7 +42,6 @@ Gem::Specification.new do |spec|
4342
spec.add_dependency 'ruby_parser', '~> 3.21'
4443
spec.add_dependency 'simplecov', '>= 0.22.0'
4544
spec.add_dependency 'tty-which', '~> 0.5.0'
46-
spec.add_dependency 'virtus', '~> 2.0'
4745

4846
spec.add_development_dependency 'bundler', '>= 2.0.0'
4947
if RUBY_PLATFORM == 'java'
@@ -59,6 +57,7 @@ Gem::Specification.new do |spec|
5957
spec.add_development_dependency 'minitest-around', '~> 0.6.0'
6058
spec.add_development_dependency 'minitest-mock'
6159
spec.add_development_dependency 'mocha', '~> 3.0.0'
60+
spec.add_development_dependency 'ostruct'
6261
spec.add_development_dependency 'rake', '~> 13.3.0', '>= 11.0.0'
6362
spec.add_development_dependency 'rdoc'
6463
spec.add_development_dependency 'rexml', '>= 3.2.0'
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
# frozen_string_literal: true
2+
3+
require 'test_helper'
4+
require 'rubycritic/core/smell'
5+
require 'rubycritic/core/analysed_module'
6+
7+
describe 'TypeCheck' do
8+
describe RubyCritic::Smell do
9+
it 'allows a value of the declared type' do
10+
smell = RubyCritic::Smell.new
11+
smell.cost = 42
12+
13+
_(smell.cost).must_equal 42
14+
end
15+
16+
it 'allows a subclass of the declared type (Integer for Numeric)' do
17+
smell = RubyCritic::Smell.new
18+
smell.score = 3
19+
20+
_(smell.score).must_equal 3
21+
end
22+
23+
it 'allows nil for any attribute' do
24+
smell = RubyCritic::Smell.new
25+
smell.message = nil
26+
27+
_(smell.message).must_be_nil
28+
end
29+
30+
it 'accepts either member of a union type (String or Symbol for type)' do
31+
smell = RubyCritic::Smell.new
32+
smell.type = 'DuplicateCode'
33+
34+
_(smell.type).must_equal 'DuplicateCode'
35+
smell.type = :complexity
36+
37+
_(smell.type).must_equal :complexity
38+
end
39+
40+
it 'raises a TypeError when assigning the wrong type' do
41+
smell = RubyCritic::Smell.new
42+
error = _ { smell.cost = 'expensive' }.must_raise TypeError
43+
_(error.message).must_match(/Smell#cost= expected Numeric or nil, got String/)
44+
end
45+
46+
it 'raises when assigning the wrong type to a union attribute' do
47+
smell = RubyCritic::Smell.new
48+
error = _ { smell.type = 123 }.must_raise TypeError
49+
_(error.message).must_match(/Smell#type= expected String \| Symbol or nil, got Integer/)
50+
end
51+
52+
it 'enforces the type through the initializer' do
53+
_ { RubyCritic::Smell.new(locations: 'not-an-array') }.must_raise TypeError
54+
end
55+
56+
it 'constructs with realistic arguments without raising' do
57+
smell = RubyCritic::Smell.new(
58+
context: '#bar',
59+
cost: 16,
60+
locations: [RubyCritic::Location.new('./foo', '42')],
61+
message: 'This smells',
62+
score: 0,
63+
type: 'DuplicateMethodCall',
64+
analyser: 'flog'
65+
)
66+
67+
_(smell.analyser).must_equal 'flog'
68+
end
69+
end
70+
71+
describe RubyCritic::AnalysedModule do
72+
it 'allows an Integer for a Numeric attribute (coverage)' do
73+
mod = RubyCritic::AnalysedModule.new
74+
mod.coverage = 0
75+
76+
_(mod.coverage).must_equal 0
77+
end
78+
79+
it 'allows a Float for a Numeric attribute (coverage)' do
80+
mod = RubyCritic::AnalysedModule.new
81+
mod.coverage = 87.5
82+
83+
_(mod.coverage).must_equal 87.5
84+
end
85+
86+
it 'preserves the Float::INFINITY default' do
87+
_(RubyCritic::AnalysedModule.new.complexity).must_equal Float::INFINITY
88+
end
89+
90+
it 'accumulates duplication through the writer (+=)' do
91+
mod = RubyCritic::AnalysedModule.new
92+
mod.duplication += 18
93+
94+
_(mod.duplication).must_equal 18
95+
end
96+
97+
it 'raises a TypeError when assigning the wrong type to pathname' do
98+
mod = RubyCritic::AnalysedModule.new
99+
error = _ { mod.pathname = 'foo.rb' }.must_raise TypeError
100+
_(error.message).must_match(/AnalysedModule#pathname= expected Pathname \| FakeFS::Pathname or nil, got String/)
101+
end
102+
103+
it 'raises when assigning a non-Array to smells' do
104+
_ { RubyCritic::AnalysedModule.new.smells = 5 }.must_raise TypeError
105+
end
106+
107+
it 'constructs with realistic arguments without raising' do
108+
mod = RubyCritic::AnalysedModule.new(
109+
name: 'Foo',
110+
pathname: Pathname.new('foo.rb'),
111+
smells: [],
112+
churn: 3,
113+
complexity: 12.5,
114+
methods_count: 4
115+
)
116+
117+
_(mod.name).must_equal 'Foo'
118+
end
119+
end
120+
121+
describe 'the TypeCheck error type' do
122+
it 'is a kind of the standard TypeError' do
123+
_(TypeCheckHelper::TypeError.ancestors).must_include TypeError
124+
end
125+
end
126+
end

test/support/type_check.rb

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
# frozen_string_literal: true
2+
3+
require 'pathname'
4+
require 'rubycritic/core/smell'
5+
require 'rubycritic/core/analysed_module'
6+
7+
# Test-only "micro-Sorbet". Wraps attribute writers so that assigning a value
8+
# of an unexpected type raises immediately, catching type regressions in the
9+
# plain-Ruby accessors that replaced virtus. It is loaded only through
10+
# test_helper, is never required by production code, and is never shipped
11+
# (the gemspec packages `lib` only), so it has zero runtime impact.
12+
module TypeCheckHelper
13+
# Raised when a wrapped writer receives a value of an unexpected type.
14+
class TypeError < ::TypeError; end
15+
16+
# Does `value` satisfy any of the `allowed` type entries? An entry may be a
17+
# Module (matched with is_a?) or a String naming a class that might not be
18+
# loaded when the spec is defined (e.g. the FakeFS::Pathname test double),
19+
# matched by name against the value's ancestors.
20+
def self.match?(value, allowed)
21+
allowed.any? do |type|
22+
if type.is_a?(Module)
23+
value.is_a?(type)
24+
else
25+
value.class.ancestors.any? { |ancestor| ancestor.name == type }
26+
end
27+
end
28+
end
29+
30+
# Builds an anonymous module that, when included, prepends type-checking
31+
# wrappers around the named writers of the including class. `prepend` is
32+
# required (not plain include) so the wrapper's `name=` wins over the
33+
# `attr_accessor` writer defined directly on the class, while `super`
34+
# still reaches that original writer.
35+
def self.build(specs)
36+
Module.new do
37+
define_singleton_method(:included) do |base|
38+
wrapper = Module.new do
39+
specs.each do |name, expected|
40+
allowed = Array(expected)
41+
define_method("#{name}=") do |value|
42+
unless value.nil? || TypeCheckHelper.match?(value, allowed)
43+
raise TypeCheckHelper::TypeError,
44+
"#{base}##{name}= expected #{allowed.join(' | ')} or nil, " \
45+
"got #{value.class} (#{value.inspect})"
46+
end
47+
48+
super(value)
49+
end
50+
end
51+
end
52+
53+
base.prepend(wrapper)
54+
end
55+
end
56+
end
57+
end
58+
59+
# Test-only DSL so a class can declare `include TypeCheck(attr: Type, ...)`.
60+
# Defined privately on Kernel so the bare `TypeCheck(...)` call resolves inside
61+
# a class body (where `self` is the class). A nil value is always permitted; a
62+
# spec value may be a single class or an array of classes (a union).
63+
module Kernel
64+
private
65+
66+
def TypeCheck(specs)
67+
TypeCheckHelper.build(specs)
68+
end
69+
end
70+
71+
module RubyCritic
72+
class Smell
73+
include TypeCheck(
74+
cost: Numeric,
75+
score: Numeric,
76+
locations: Array,
77+
status: Symbol,
78+
context: String,
79+
message: String,
80+
type: [String, Symbol],
81+
analyser: String
82+
)
83+
end
84+
85+
class AnalysedModule
86+
include TypeCheck(
87+
coverage: Numeric,
88+
complexity: Numeric,
89+
duplication: Numeric,
90+
churn: Integer,
91+
methods_count: Integer,
92+
name: String,
93+
pathname: [Pathname, 'FakeFS::Pathname'],
94+
smells: Array
95+
)
96+
end
97+
end

test/test_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
require 'mocha/minitest'
1414
require 'ostruct'
1515
require 'diff/lcs'
16+
require 'support/type_check'
1617

1718
def context(...)
1819
describe(...)

0 commit comments

Comments
 (0)