diff --git a/changelog/new_class_descendants_cop.md b/changelog/new_class_descendants_cop.md new file mode 100644 index 0000000000..4e155ba95c --- /dev/null +++ b/changelog/new_class_descendants_cop.md @@ -0,0 +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][]) diff --git a/config/default.yml b/config/default.yml index 2e59f3661a..836d43811f 100644 --- a/config/default.yml +++ b/config/default.yml @@ -298,6 +298,14 @@ Rails/BulkChangeTable: Include: - db/**/*.rb +Rails/ClassDescendants: + Description: >- + Avoid using `Class#descendants` as it may not include classes that have yet + to be autoloaded and is non-deterministic with regards to Garbage Collection. + Enabled: pending + Severity: warning + VersionAdded: '<>' + Rails/CompactBlank: Description: 'Checks if collection can be blank-compacted with `compact_blank`.' Enabled: pending diff --git a/lib/rubocop/cop/rails/class_descendants.rb b/lib/rubocop/cop/rails/class_descendants.rb new file mode 100644 index 0000000000..9103895707 --- /dev/null +++ b/lib/rubocop/cop/rails/class_descendants.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Checks for the use of `Class#descendants` which has several issues: + # + # 1. It doesn't know about classes that have yet to be autoloaded. + # 2. It's non-deterministic with regards to Garbage Collection of classes. + # If you use `Class.descendants` in tests where there is a pattern to + # dynamically define classes, GC is unpredictable for when those classes + # are cleaned up and removed. + # + # + # @example + # # bad + # User.descendants + # ApplicationRecord.descendants.map(&:name) + # + # # bad + # MyClass.descendants.each do |klass| + # klass.do_something + # end + # + class ClassDescendants < Base + MSG = 'Avoid using `%s` as it may not include classes that have yet to be autoloaded ' \ + 'and is non-deterministic with regards to Garbage Collection.' + + RESTRICT_ON_SEND = %i[descendants].freeze + + # @!method descendants_call?(node) + def_node_matcher :descendants_call?, <<~PATTERN + (send _ :descendants ...) + PATTERN + + def on_send(node) + return unless descendants_call?(node) + + add_offense(node, message: format(MSG, method: node.method_name)) + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index e3cea8a378..ec1a7cc2ee 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -32,6 +32,7 @@ require_relative 'rails/belongs_to' require_relative 'rails/blank' require_relative 'rails/bulk_change_table' +require_relative 'rails/class_descendants' require_relative 'rails/compact_blank' require_relative 'rails/content_tag' require_relative 'rails/create_table_with_timestamps' diff --git a/spec/rubocop/cop/rails/class_descendants_spec.rb b/spec/rubocop/cop/rails/class_descendants_spec.rb new file mode 100644 index 0000000000..575a853f45 --- /dev/null +++ b/spec/rubocop/cop/rails/class_descendants_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::ClassDescendants, :config do + it 'registers an offense when using `descendants`' do + expect_offense(<<~RUBY) + User.descendants + ^^^^^^^^^^^^^^^^ Avoid using `descendants` as it may not include classes that have yet to be autoloaded and is non-deterministic with regards to Garbage Collection. + RUBY + end +end