Skip to content

Commit e340acf

Browse files
Bart de Waterjules2689rafaelfrancaFrancois Chagnonbyroot
authored and
bdewater
committed
Add new Rails/ActiveSupportOnLoad cop.
This cop is extracted from Shopify's internal Rubocop repository. Many thanks to the original authors for their work. Co-authored-by: Julian Nadeau <[email protected]> Co-authored-by: Rafael Mendonça França <[email protected]> Co-authored-by: Francois Chagnon <[email protected]> Co-authored-by: Jean Boussier <[email protected]>
1 parent 931e9b2 commit e340acf

File tree

5 files changed

+177
-0
lines changed

5 files changed

+177
-0
lines changed

‎changelog/new_add_railsactivesupportonload_cop.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#749](https://github.com/rubocop/rubocop-rails/pull/749): Add new `Rails/ActiveSupportOnLoad` cop. ([@bdewater][])

‎config/default.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,15 @@ Rails/ActiveSupportAliases:
129129
Enabled: true
130130
VersionAdded: '0.48'
131131

132+
Rails/ActiveSupportOnLoad:
133+
Description: 'Use `ActiveSupport.on_load(...)` to patch Rails framework classes.'
134+
Enabled: 'pending'
135+
Reference:
136+
- 'https://api.rubyonrails.org/classes/ActiveSupport/LazyLoadHooks.html'
137+
- 'https://guides.rubyonrails.org/engines.html#available-load-hooks'
138+
SafeAutoCorrect: false
139+
VersionAdded: '<<next>>'
140+
132141
Rails/AddColumnIndex:
133142
Description: >-
134143
Rails migrations don't make use of a given `index` key, but also

‎lib/rubocop/cop/rails/active_support_on_load.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Checks for Rails framework classes that are patched directly instead of using Active Support load hooks. Direct
7+
# patching forcibly loads the framework referenced, using hooks defers loading until it's actually needed.
8+
#
9+
# @example
10+
#
11+
# # bad
12+
# ActiveRecord::Base.include(MyClass)
13+
#
14+
# # good
15+
# ActiveSupport.on_load(:active_record) { include MyClass }
16+
#
17+
# @safety
18+
# While using lazy load hooks is recommended, it changes the order in which is code is loaded and may reveal
19+
# load order dependency bugs.
20+
class ActiveSupportOnLoad < Base
21+
extend AutoCorrector
22+
23+
MSG = 'Use `%<prefer>s` instead of `%<current>s`.'
24+
RESTRICT_ON_SEND = %i[prepend include extend].freeze
25+
LOAD_HOOKS = {
26+
'ActionCable' => 'action_cable',
27+
'ActionCable::Channel::Base' => 'action_cable_channel',
28+
'ActionCable::Connection::Base' => 'action_cable_connection',
29+
'ActionCable::Connection::TestCase' => 'action_cable_connection_test_case',
30+
'ActionController::API' => 'action_controller',
31+
'ActionController::Base' => 'action_controller',
32+
'ActionController::TestCase' => 'action_controller_test_case',
33+
'ActionDispatch::IntegrationTest' => 'action_dispatch_integration_test',
34+
'ActionDispatch::Request' => 'action_dispatch_request',
35+
'ActionDispatch::Response' => 'action_dispatch_response',
36+
'ActionDispatch::SystemTestCase' => 'action_dispatch_system_test_case',
37+
'ActionMailbox::Base' => 'action_mailbox',
38+
'ActionMailbox::InboundEmail' => 'action_mailbox_inbound_email',
39+
'ActionMailbox::Record' => 'action_mailbox_record',
40+
'ActionMailbox::TestCase' => 'action_mailbox_test_case',
41+
'ActionMailer::Base' => 'action_mailer',
42+
'ActionMailer::TestCase' => 'action_mailer_test_case',
43+
'ActionText::Content' => 'action_text_content',
44+
'ActionText::Record' => 'action_text_record',
45+
'ActionText::RichText' => 'action_text_rich_text',
46+
'ActionView::Base' => 'action_view',
47+
'ActionView::TestCase' => 'action_view_test_case',
48+
'ActiveJob::Base' => 'active_job',
49+
'ActiveJob::TestCase' => 'active_job_test_case',
50+
'ActiveRecord::Base' => 'active_record',
51+
'ActiveStorage::Attachment' => 'active_storage_attachment',
52+
'ActiveStorage::Blob' => 'active_storage_blob',
53+
'ActiveStorage::Record' => 'active_storage_record',
54+
'ActiveStorage::VariantRecord' => 'active_storage_variant_record',
55+
'ActiveSupport::TestCase' => 'active_support_test_case'
56+
}.freeze
57+
58+
def on_send(node)
59+
receiver, method, arguments = *node # rubocop:disable InternalAffairs/NodeDestructuring
60+
return unless receiver && (hook = LOAD_HOOKS[receiver.const_name])
61+
62+
preferred = "ActiveSupport.on_load(:#{hook}) { #{method} #{arguments.source} }"
63+
add_offense(node, message: format(MSG, prefer: preferred, current: node.source)) do |corrector|
64+
corrector.replace(node, preferred)
65+
end
66+
end
67+
end
68+
end
69+
end
70+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
require_relative 'rails/active_record_callbacks_order'
1616
require_relative 'rails/active_record_override'
1717
require_relative 'rails/active_support_aliases'
18+
require_relative 'rails/active_support_on_load'
1819
require_relative 'rails/add_column_index'
1920
require_relative 'rails/after_commit_override'
2021
require_relative 'rails/application_controller'

‎spec/rubocop/cop/rails/active_support_on_load_spec.rb

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe(RuboCop::Cop::Rails::ActiveSupportOnLoad, :config) do
4+
it 'adds offense and corrects when trying to extend a framework class with include' do
5+
expect_offense(<<~RUBY)
6+
ActiveRecord::Base.include(MyClass)
7+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { include MyClass }` instead of `ActiveRecord::Base.include(MyClass)`.
8+
RUBY
9+
10+
expect_correction(<<~RUBY)
11+
ActiveSupport.on_load(:active_record) { include MyClass }
12+
RUBY
13+
end
14+
15+
it 'adds offense and corrects when trying to extend a framework class with prepend' do
16+
expect_offense(<<~RUBY)
17+
ActiveRecord::Base.prepend(MyClass)
18+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { prepend MyClass }` instead of `ActiveRecord::Base.prepend(MyClass)`.
19+
RUBY
20+
21+
expect_correction(<<~RUBY)
22+
ActiveSupport.on_load(:active_record) { prepend MyClass }
23+
RUBY
24+
end
25+
26+
it 'adds offense and corrects when trying to extend a framework class with extend' do
27+
expect_offense(<<~RUBY)
28+
ActiveRecord::Base.extend(MyClass)
29+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend MyClass }` instead of `ActiveRecord::Base.extend(MyClass)`.
30+
RUBY
31+
32+
expect_correction(<<~RUBY)
33+
ActiveSupport.on_load(:active_record) { extend MyClass }
34+
RUBY
35+
end
36+
37+
it 'adds offense and corrects when trying to extend a framework class with absolute name' do
38+
expect_offense(<<~RUBY)
39+
::ActiveRecord::Base.extend(MyClass)
40+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend MyClass }` instead of `::ActiveRecord::Base.extend(MyClass)`.
41+
RUBY
42+
43+
expect_correction(<<~RUBY)
44+
ActiveSupport.on_load(:active_record) { extend MyClass }
45+
RUBY
46+
end
47+
48+
it 'adds offense and corrects when trying to extend a framework class with a variable' do
49+
expect_offense(<<~RUBY)
50+
ActiveRecord::Base.extend(my_class)
51+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend my_class }` instead of `ActiveRecord::Base.extend(my_class)`.
52+
RUBY
53+
54+
expect_correction(<<~RUBY)
55+
ActiveSupport.on_load(:active_record) { extend my_class }
56+
RUBY
57+
end
58+
59+
it 'does not add offense when extending a variable' do
60+
expect_no_offenses(<<~RUBY)
61+
foo.extend(MyClass)
62+
RUBY
63+
end
64+
65+
it 'does not add offense when extending the framework using on_load and include' do
66+
expect_no_offenses(<<~RUBY)
67+
ActiveSupport.on_load(:active_record) { include MyClass }
68+
RUBY
69+
end
70+
71+
it 'does not add offense when extending the framework using on_load and include in a multi-line block' do
72+
expect_no_offenses(<<~RUBY)
73+
ActiveSupport.on_load(:active_record) do
74+
include MyClass
75+
end
76+
RUBY
77+
end
78+
79+
it 'does not add offense when there is no extension on the supported classes' do
80+
expect_no_offenses(<<~RUBY)
81+
ActiveRecord::Base.include_root_in_json = false
82+
RUBY
83+
end
84+
85+
it 'does not add offense when using include?' do
86+
expect_no_offenses(<<~RUBY)
87+
name.include?('bob')
88+
RUBY
89+
end
90+
91+
it 'does not add offense on unsupported classes' do
92+
expect_no_offenses(<<~RUBY)
93+
MyClass1.prepend(MyClass)
94+
RUBY
95+
end
96+
end

0 commit comments

Comments
 (0)