Skip to content

Commit 2e1353e

Browse files
committed
Add Rails/ClassDescendants cop
I trust Ben Sheldon regarding that 😉 github/rubocop-github#110
1 parent a6f848a commit 2e1353e

File tree

5 files changed

+70
-0
lines changed

5 files changed

+70
-0
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1544](https://github.com/rubocop/rubocop-rails/pull/1544): Add new `Rails/ClassDescendants` cop to discourage use of `Class#descendants` due to autoloading issues and non-deterministic behavior with Garbage Collection. ([@alexanderadam][])

config/default.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,14 @@ Rails/BulkChangeTable:
298298
Include:
299299
- db/**/*.rb
300300

301+
Rails/ClassDescendants:
302+
Description: >-
303+
Avoid using `Class#descendants` as it may not include classes that have yet
304+
to be autoloaded and is non-deterministic with regards to Garbage Collection.
305+
Enabled: pending
306+
Severity: warning
307+
VersionAdded: '<<next>>'
308+
301309
Rails/CompactBlank:
302310
Description: 'Checks if collection can be blank-compacted with `compact_blank`.'
303311
Enabled: pending
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Checks for the use of `Class#descendants` which has several issues:
7+
#
8+
# 1. It doesn't know about classes that have yet to be autoloaded.
9+
# 2. It's non-deterministic with regards to Garbage Collection of classes.
10+
# If you use `Class.descendants` in tests where there is a pattern to
11+
# dynamically define classes, GC is unpredictable for when those classes
12+
# are cleaned up and removed.
13+
#
14+
# Consider using alternatives like maintaining an explicit registry of subclasses
15+
# or using Rails' `ActiveSupport::DescendantsTracker` with proper autoloading
16+
# considerations.
17+
#
18+
# @example
19+
# # bad
20+
# User.descendants
21+
# ApplicationRecord.descendants.map(&:name)
22+
#
23+
# # bad
24+
# MyClass.descendants.each do |klass|
25+
# klass.do_something
26+
# end
27+
#
28+
# # good - use subclasses if you don't need recursive descendants
29+
# MyClass.subclasses
30+
#
31+
class ClassDescendants < Base
32+
MSG = 'Avoid using `%<method>s` as it may not include classes that have yet to be autoloaded ' \
33+
'and is non-deterministic with regards to Garbage Collection.'
34+
35+
RESTRICT_ON_SEND = %i[descendants].freeze
36+
37+
# @!method descendants_call?(node)
38+
def_node_matcher :descendants_call?, <<~PATTERN
39+
(send _ :descendants ...)
40+
PATTERN
41+
42+
def on_send(node)
43+
return unless descendants_call?(node)
44+
45+
add_offense(node, message: format(MSG, method: node.method_name))
46+
end
47+
end
48+
end
49+
end
50+
end

lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
require_relative 'rails/belongs_to'
3333
require_relative 'rails/blank'
3434
require_relative 'rails/bulk_change_table'
35+
require_relative 'rails/class_descendants'
3536
require_relative 'rails/compact_blank'
3637
require_relative 'rails/content_tag'
3738
require_relative 'rails/create_table_with_timestamps'
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::ClassDescendants, :config do
4+
it 'registers an offense when using `descendants`' do
5+
expect_offense(<<~RUBY)
6+
User.descendants
7+
^^^^^^^^^^^^^^^^ Avoid using `descendants` as it may not include classes that have yet to be autoloaded and is non-deterministic with regards to Garbage Collection.
8+
RUBY
9+
end
10+
end

0 commit comments

Comments
 (0)