From b9e40e1247c31b74492d59737e343c32fb6ef8b3 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 14 Dec 2023 18:49:18 +0200 Subject: [PATCH] Update `Rails/PluckInWhere` to check for `.ids` call --- .../change_support_ids_for_pluck_in_where.md | 1 + lib/rubocop/cop/rails/pluck_in_where.rb | 18 ++++-- spec/rubocop/cop/rails/pluck_in_where_spec.rb | 62 +++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 changelog/change_support_ids_for_pluck_in_where.md diff --git a/changelog/change_support_ids_for_pluck_in_where.md b/changelog/change_support_ids_for_pluck_in_where.md new file mode 100644 index 0000000000..91b7641e88 --- /dev/null +++ b/changelog/change_support_ids_for_pluck_in_where.md @@ -0,0 +1 @@ +* [#1213](https://github.com/rubocop/rubocop-rails/issues/1213): Update `Rails/PluckInWhere` to check for `.ids` call. ([@fatkodima][]) diff --git a/lib/rubocop/cop/rails/pluck_in_where.rb b/lib/rubocop/cop/rails/pluck_in_where.rb index f88dde22d0..7aa17edede 100644 --- a/lib/rubocop/cop/rails/pluck_in_where.rb +++ b/lib/rubocop/cop/rails/pluck_in_where.rb @@ -22,6 +22,7 @@ module Rails # @example # # bad # Post.where(user_id: User.active.pluck(:id)) + # Post.where(user_id: User.active.ids) # Post.where.not(user_id: User.active.pluck(:id)) # # # good @@ -42,8 +43,9 @@ class PluckInWhere < Base include ConfigurableEnforcedStyle extend AutoCorrector - MSG = 'Use `select` instead of `pluck` within `where` query method.' - RESTRICT_ON_SEND = %i[pluck].freeze + MSG_SELECT = 'Use `select` instead of `pluck` within `where` query method.' + MSG_IDS = 'Use `select(:id)` instead of `ids` within `where` query method.' + RESTRICT_ON_SEND = %i[pluck ids].freeze def on_send(node) return unless in_where?(node) @@ -51,8 +53,16 @@ def on_send(node) range = node.loc.selector - add_offense(range) do |corrector| - corrector.replace(range, 'select') + if node.method?(:ids) + replacement = 'select(:id)' + message = MSG_IDS + else + replacement = 'select' + message = MSG_SELECT + end + + add_offense(range, message: message) do |corrector| + corrector.replace(range, replacement) end end diff --git a/spec/rubocop/cop/rails/pluck_in_where_spec.rb b/spec/rubocop/cop/rails/pluck_in_where_spec.rb index 0a8836302c..efa93a094c 100644 --- a/spec/rubocop/cop/rails/pluck_in_where_spec.rb +++ b/spec/rubocop/cop/rails/pluck_in_where_spec.rb @@ -17,6 +17,17 @@ RUBY end + it 'registers an offense and corrects when using `ids` in `where` for constant' do + expect_offense(<<~RUBY) + Post.where(user_id: User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where(user_id: User.active.select(:id)) + RUBY + end + it 'registers an offense and corrects when using `pluck` in `where.not` for constant' do expect_offense(<<~RUBY) Post.where.not(user_id: User.active.pluck(:id)) @@ -28,6 +39,17 @@ RUBY end + it 'registers an offense and corrects when using `ids` in `where.not` for constant' do + expect_offense(<<~RUBY) + Post.where.not(user_id: User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where.not(user_id: User.active.select(:id)) + RUBY + end + it 'registers an offense and corrects when using `pluck` in `rewhere` for constant' do expect_offense(<<~RUBY) Post.rewhere('user_id IN (?)', User.active.pluck(:id)) @@ -39,6 +61,17 @@ RUBY end + it 'registers an offense and corrects when using `ids` in `rewhere` for constant' do + expect_offense(<<~RUBY) + Post.rewhere('user_id IN (?)', User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.rewhere('user_id IN (?)', User.active.select(:id)) + RUBY + end + it 'does not register an offense when using `select` in `where`' do expect_no_offenses(<<~RUBY) Post.where(user_id: User.active.select(:id)) @@ -51,11 +84,23 @@ RUBY end + it 'does not register an offense when using `ids` chained with other method calls in `where`' do + expect_no_offenses(<<~RUBY) + Post.where(user_id: User.ids.map(&:to_i)) + RUBY + end + it 'does not register an offense when using `select` in query methods other than `where`' do expect_no_offenses(<<~RUBY) Post.order(columns.pluck(:name)) RUBY end + + it 'does not register an offense when using `ids` in query methods other than `where`' do + expect_no_offenses(<<~RUBY) + Post.order(columns.ids) + RUBY + end end context 'EnforcedStyle: conservative' do @@ -69,6 +114,12 @@ Post.where(user_id: users.active.pluck(:id)) RUBY end + + it 'does not register an offense when using `ids` in `where`' do + expect_no_offenses(<<~RUBY) + Post.where(user_id: users.active.ids) + RUBY + end end end @@ -88,6 +139,17 @@ Post.where(user_id: users.active.select(:id)) RUBY end + + it 'registers and corrects an offense when using `ids` in `where`' do + expect_offense(<<~RUBY) + Post.where(user_id: users.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where(user_id: users.active.select(:id)) + RUBY + end end end end