diff --git a/changelog/new_active_record_calculation_cop.md b/changelog/new_active_record_calculation_cop.md new file mode 100644 index 0000000000..ca0ac72ded --- /dev/null +++ b/changelog/new_active_record_calculation_cop.md @@ -0,0 +1 @@ +* [#1205](https://github.com/rubocop/rubocop-rails/issues/1205): Add new `Rails/ActiveRecordCalculation` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index b59db897c5..764326c1e1 100644 --- a/config/default.yml +++ b/config/default.yml @@ -130,6 +130,11 @@ Rails/ActiveRecordAliases: VersionAdded: '0.53' SafeAutoCorrect: false +Rails/ActiveRecordCalculation: + Description: 'Use ActiveRecord calculation methods instead of `pluck` followed by Enumerable methods.' + Enabled: pending + VersionAdded: '<>' + Rails/ActiveRecordCallbacksOrder: Description: 'Order callback declarations in the order in which they will be executed.' StyleGuide: 'https://rails.rubystyle.guide/#callbacks-order' diff --git a/lib/rubocop/cop/rails/active_record_calculation.rb b/lib/rubocop/cop/rails/active_record_calculation.rb new file mode 100644 index 0000000000..b430a58423 --- /dev/null +++ b/lib/rubocop/cop/rails/active_record_calculation.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Enforces the use of ActiveRecord calculation methods instead of `pluck` + # followed by Enumerable methods. + # + # It avoids loading potentially many values into memory by doing the + # calculations inside the database. + # + # @example + # # bad + # User.pluck(:id).max + # User.pluck(:id).min + # User.pluck(:age).sum + # + # # good + # User.maximum(:id) + # User.minimum(:id) + # User.sum(:age) + # + # # good + # User.pluck(:email).max { |email| email.length } + # User.pluck(:email).max(2) + # User.pluck(:id, :company_id).max + # User.pluck(:age).count + # + class ActiveRecordCalculation < Base + include RangeHelp + extend AutoCorrector + + MSG = 'Use `%s` instead of `pluck.%s`.' + + RESTRICT_ON_SEND = %i[pluck].freeze + OPERATIONS_MAP = { min: :minimum, max: :maximum, sum: :sum }.freeze + + def_node_matcher :pluck_calculation?, <<~PATTERN + (send + (send _ :pluck $_) + ${:#{OPERATIONS_MAP.keys.join(' :')}}) + PATTERN + + def on_send(node) + return unless (parent = node.parent) + return if send_with_block?(parent) + + pluck_calculation?(parent) do |arg_node, calculation| + good_method = OPERATIONS_MAP.fetch(calculation) + message = format(MSG, good_method: good_method, bad_method: calculation) + offense_range = range_between(node.loc.selector.begin_pos, parent.source_range.end_pos) + + add_offense(offense_range, message: message) do |corrector| + corrector.replace(offense_range, "#{good_method}(#{arg_node.source})") + end + end + end + + private + + def send_with_block?(node) + node.parent&.block_type? && node.parent.send_node == node + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index b7eddc4fe4..b72b8656ff 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -14,6 +14,7 @@ require_relative 'rails/action_filter' require_relative 'rails/action_order' require_relative 'rails/active_record_aliases' +require_relative 'rails/active_record_calculation' require_relative 'rails/active_record_callbacks_order' require_relative 'rails/active_record_override' require_relative 'rails/active_support_aliases' diff --git a/spec/rubocop/cop/rails/active_record_calculation_spec.rb b/spec/rubocop/cop/rails/active_record_calculation_spec.rb new file mode 100644 index 0000000000..39fb5556ee --- /dev/null +++ b/spec/rubocop/cop/rails/active_record_calculation_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::ActiveRecordCalculation, :config do + it 'registers an offense and corrects when using `pluck.max`' do + expect_offense(<<~RUBY) + Model.pluck(:column).max + ^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`. + RUBY + + expect_correction(<<~RUBY) + Model.maximum(:column) + RUBY + end + + it 'registers an offense and corrects when using `pluck.min`' do + expect_offense(<<~RUBY) + Model.pluck(:column).min + ^^^^^^^^^^^^^^^^^^ Use `minimum` instead of `pluck.min`. + RUBY + + expect_correction(<<~RUBY) + Model.minimum(:column) + RUBY + end + + it 'registers an offense and corrects when using `pluck.sum`' do + expect_offense(<<~RUBY) + Model.pluck(:column).sum + ^^^^^^^^^^^^^^^^^^ Use `sum` instead of `pluck.sum`. + RUBY + + expect_correction(<<~RUBY) + Model.sum(:column) + RUBY + end + + it 'registers an offense and corrects when using `pluck.max` without receiver' do + expect_offense(<<~RUBY) + pluck(:column).max + ^^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`. + RUBY + + expect_correction(<<~RUBY) + maximum(:column) + RUBY + end + + it 'registers an offense and corrects when using `pluck.max` with non-literal column' do + expect_offense(<<~RUBY) + Model.pluck(column).max + ^^^^^^^^^^^^^^^^^ Use `maximum` instead of `pluck.max`. + RUBY + + expect_correction(<<~RUBY) + Model.maximum(column) + RUBY + end + + it 'does not register an offense when using `pluck.max` with multiple arguments' do + expect_no_offenses(<<~RUBY) + Model.pluck(:column1, :column2).max + RUBY + end + + it 'does not register an offense when using `pluck.max` with block' do + expect_no_offenses(<<~RUBY) + Model.pluck(:column).max { |e| e } + RUBY + end + + it 'does not register an offense when using `pluck.max` and `max` has argument' do + expect_no_offenses(<<~RUBY) + Model.pluck(:column).max(1) + RUBY + end + + it 'does not register an offense when using `maximum`' do + expect_no_offenses(<<~RUBY) + Model.maximum(:column) + RUBY + end +end