From e3c173fa8d527c4b41e698a7d051993ae1720f8d 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. Julian Nadeau Rafael Mendonça França Francois Chagnon Jean Boussier --- CHANGELOG.md | 2 + config/default.yml | 6 + .../cop/rails/active_support_on_load.rb | 50 ++++++++ .../cop/rails/active_support_on_load_spec.rb | 114 ++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + 5 files changed, 173 insertions(+) create mode 100644 lib/rubocop/cop/rails/active_support_on_load.rb create mode 100644 lib/rubocop/cop/rails/active_support_on_load_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ed792716d6..867a9ea45f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Change log ## master (unreleased) +* [#x](https://github.com/rubocop/rubocop-rails/issues/x): Add new `Rails/ActiveSupportOnLoadac` cop. ([@bdewater][]) ## 2.15.2 (2022-07-07) @@ -644,3 +645,4 @@ [@kkitadate]: https://github.com/kkitadate [@Darhazer]: https://github.com/Darhazer [@kazarin]: https://github.com/kazarin +[@bdewater]: https://github.com/bdewater diff --git a/config/default.yml b/config/default.yml index 9b7cd2d1cc..eaeac30d27 100644 --- a/config/default.yml +++ b/config/default.yml @@ -99,6 +99,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..a596f27238 --- /dev/null +++ b/lib/rubocop/cop/rails/active_support_on_load.rb @@ -0,0 +1,50 @@ +# 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 = + "Don't use \x1b[32m%s\x1b[0m. Use this form instead \x1b[32m%s\x1b[0m" + + def on_send(node) + receiver, method, arguments = *node + 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, preferred: preferred, source: node.source)) do |corrector| + corrector.replace(node, preferred) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails/active_support_on_load_spec.rb b/lib/rubocop/cop/rails/active_support_on_load_spec.rb new file mode 100644 index 0000000000..f0995664d5 --- /dev/null +++ b/lib/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) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use \e[32mActiveRecord::Base.include(MyClass)\e[0m. Use this form instead \e[32mActiveSupport.on_load(:active_record) { include MyClass }\e[0m + RUBY + end + + it 'adds offense when trying to extend a framework class with prepend' do + expect_offense(<<~RUBY) + ActiveRecord::Base.prepend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use \e[32mActiveRecord::Base.prepend(MyClass)\e[0m. Use this form instead \e[32mActiveSupport.on_load(:active_record) { prepend MyClass }\e[0m + RUBY + end + + it 'adds offense when trying to extend a framework class with extend' do + expect_offense(<<~RUBY) + ActiveRecord::Base.extend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use \e[32mActiveRecord::Base.extend(MyClass)\e[0m. Use this form instead \e[32mActiveSupport.on_load(:active_record) { extend MyClass }\e[0m + RUBY + end + + it 'adds offense when trying to extend a framework class with absolute name' do + expect_offense(<<~RUBY) + ::ActiveRecord::Base.extend(MyClass) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use \e[32m::ActiveRecord::Base.extend(MyClass)\e[0m. Use this form instead \e[32mActiveSupport.on_load(:active_record) { extend MyClass }\e[0m + RUBY + end + + it 'adds offense when trying to extend a framework class with a variable' do + expect_offense(<<~RUBY) + ActiveRecord::Base.extend(my_class) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use \e[32mActiveRecord::Base.extend(my_class)\e[0m. Use this form instead \e[32mActiveSupport.on_load(:active_record) { extend my_class }\e[0m + 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 extensio 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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 7ff7ac5e18..cbd4aba5d6 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -14,6 +14,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'