From a4f456221c7a07992ec7bc2995035db19a103ab3 Mon Sep 17 00:00:00 2001 From: Bart de Water Date: Fri, 22 Jul 2022 07:48:59 -0400 Subject: [PATCH] Add new Rails/ActiveSupportOnLoad cop. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This cop is extracted from Shopify's internal Rubocop repository. Many thanks to the original authors for their work. Co-authored-by: Julian Nadeau Co-authored-by: Rafael Mendonça França Co-authored-by: Francois Chagnon Co-authored-by: Jean Boussier --- CHANGELOG.md | 1 + .../new_add_railsactivesupportonload_cop.md | 1 + config/default.yml | 6 + .../cop/rails/active_support_on_load.rb | 47 ++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/active_support_on_load_spec.rb | 114 ++++++++++++++++++ 6 files changed, 170 insertions(+) create mode 100644 changelog/new_add_railsactivesupportonload_cop.md create mode 100644 lib/rubocop/cop/rails/active_support_on_load.rb create mode 100644 spec/rubocop/cop/rails/active_support_on_load_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ed792716d6..45a2293f51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -644,3 +644,4 @@ [@kkitadate]: https://github.com/kkitadate [@Darhazer]: https://github.com/Darhazer [@kazarin]: https://github.com/kazarin +[@bdewater]: https://github.com/bdewater diff --git a/changelog/new_add_railsactivesupportonload_cop.md b/changelog/new_add_railsactivesupportonload_cop.md new file mode 100644 index 0000000000..2d1f6e1f64 --- /dev/null +++ b/changelog/new_add_railsactivesupportonload_cop.md @@ -0,0 +1 @@ +* [#749](https://github.com/rubocop/rubocop-rails/issues/749): Add new `Rails/ActiveSupportOnLoadac` cop. ([@bdewater][]) diff --git a/config/default.yml b/config/default.yml index 286029a31a..1ecc22a023 100644 --- a/config/default.yml +++ b/config/default.yml @@ -129,6 +129,12 @@ Rails/ActiveSupportAliases: Enabled: true VersionAdded: '0.48' +Rails/ActiveSupportOnLoad: + Description: 'Use `ActiveSupport.on_load(...)` to patch Rails core classes.' + StyleGuide: 'https://rails.rubystyle.guide/#hash-conditions' + Enabled: 'pending' + VersionAdded: '<>' + Rails/AddColumnIndex: Description: >- Rails migrations don't make use of a given `index` key, but also diff --git a/lib/rubocop/cop/rails/active_support_on_load.rb b/lib/rubocop/cop/rails/active_support_on_load.rb new file mode 100644 index 0000000000..dda5a949ec --- /dev/null +++ b/lib/rubocop/cop/rails/active_support_on_load.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Use Active Support lazy load hooks to patch Rails core classes, so they are not forcible loaded early. + # + # @example + # + # # bad + # ActiveRecord::Base.include(MyClass) + # + # # good + # ActiveSupport.on_load(:active_record) { include MyClass } + class ActiveSupportOnLoad < Base + extend AutoCorrector + + AUTOCORRECTABLE_CLASSES = { + 'ActiveRecord::Base' => 'active_record', + 'ActionController::Base' => 'action_controller', + 'ActiveJob::Base' => 'active_job', + 'ActionView::Base' => 'action_view', + 'ActionMailer::Base' => 'action_mailer', + 'ActionController::TestCase' => 'action_controller_test_case', + 'ActiveSupport::TestCase' => 'active_support_test_case', + 'ActiveJob::TestCase' => 'active_job_test_case', + 'ActionDispatch::IntegrationTest' => 'action_dispatch_integration_test', + 'ActionMailer::TestCase' => 'action_mailer_test_case' + }.freeze + RESTRICT_ON_SEND = %i[prepend include extend].freeze + MSG = 'Use `%s` instead of `%s`.' + + def on_send(node) + receiver, method, arguments = *node # rubocop:disable InternalAffairs/NodeDestructuring + return unless receiver && AUTOCORRECTABLE_CLASSES.key?(receiver.const_name) + + hook_name = AUTOCORRECTABLE_CLASSES[receiver.const_name] + preferred = "ActiveSupport.on_load(:#{hook_name}) { #{method} #{arguments.source} }" + + add_offense(node, message: format(MSG, prefer: preferred, current: node.source)) do |corrector| + corrector.replace(node, preferred) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index b8c7612879..8e0e555cc4 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -15,6 +15,7 @@ require_relative 'rails/active_record_callbacks_order' require_relative 'rails/active_record_override' require_relative 'rails/active_support_aliases' +require_relative 'rails/active_support_on_load' require_relative 'rails/add_column_index' require_relative 'rails/after_commit_override' require_relative 'rails/application_controller' diff --git a/spec/rubocop/cop/rails/active_support_on_load_spec.rb b/spec/rubocop/cop/rails/active_support_on_load_spec.rb new file mode 100644 index 0000000000..3aaea4f1b8 --- /dev/null +++ b/spec/rubocop/cop/rails/active_support_on_load_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +RSpec.describe(RuboCop::Cop::Rails::ActiveSupportOnLoad, :config) do + it 'adds offense when trying to extend a framework class with include' do + expect_offense(<<~RUBY) + ActiveRecord::Base.include(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { include MyClass }` instead of `ActiveRecord::Base.include(MyClass)`. + RUBY + end + + it 'adds offense when trying to extend a framework class with prepend' do + expect_offense(<<~RUBY) + ActiveRecord::Base.prepend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { prepend MyClass }` instead of `ActiveRecord::Base.prepend(MyClass)`. + RUBY + end + + it 'adds offense when trying to extend a framework class with extend' do + expect_offense(<<~RUBY) + ActiveRecord::Base.extend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend MyClass }` instead of `ActiveRecord::Base.extend(MyClass)`. + RUBY + end + + it 'adds offense when trying to extend a framework class with absolute name' do + expect_offense(<<~RUBY) + ::ActiveRecord::Base.extend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend MyClass }` instead of `::ActiveRecord::Base.extend(MyClass)`. + RUBY + end + + it 'adds offense when trying to extend a framework class with a variable' do + expect_offense(<<~RUBY) + ActiveRecord::Base.extend(my_class) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `ActiveSupport.on_load(:active_record) { extend my_class }` instead of `ActiveRecord::Base.extend(my_class)`. + RUBY + end + + it 'does not add offense when extending a variable' do + expect_no_offenses(<<~RUBY) + foo.extend(MyClass) + RUBY + end + + it 'does not add offense when extending the framework using on_load and include' do + expect_no_offenses(<<~RUBY) + ActiveSupport.on_load(:active_record) { include MyClass } + RUBY + end + + it 'does not add offense when extending the framework using on_load and include in a multi-line block' do + expect_no_offenses(<<~RUBY) + ActiveSupport.on_load(:active_record) do + include MyClass + end + RUBY + end + + it 'does not add offense when not including a class' do + expect_no_offenses(<<~RUBY) + ActiveRecord::Base.include_root_in_json = false + RUBY + end + + it 'does not add offense when using include?' do + expect_no_offenses(<<~RUBY) + name.include?('bob') + RUBY + end + + context 'autocorrect' do + it 'autocorrects extension on supported classes' do + source = <<~RUBY + ActiveRecord::Base.prepend(MyClass) + RUBY + + corrected_source = <<~RUBY + ActiveSupport.on_load(:active_record) { prepend MyClass } + RUBY + + corrected = autocorrect_source(source) + + expect(corrected).to(eq(corrected_source)) + end + + it 'does not autocorrect extension on unsupported classes' do + source = <<~RUBY + MyClass1.prepend(MyClass) + RUBY + + corrected = autocorrect_source(source) + + expect(corrected).to(eq(source)) + + source = <<~RUBY + MyClass1.include(MyClass) + RUBY + + corrected = autocorrect_source(source) + + expect(corrected).to(eq(source)) + end + + it 'does not autocorrect when there is no extension on the supported classes' do + source = <<~RUBY + ActiveRecord::Base.include_root_in_json = false + RUBY + + corrected = autocorrect_source(source) + + expect(corrected).to(eq(source)) + end + end +end