Skip to content

Commit

Permalink
Update Rails/PluckInWhere to check for .ids call
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Dec 14, 2023
1 parent 1431257 commit b9e40e1
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog/change_support_ids_for_pluck_in_where.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1213](https://github.com/rubocop/rubocop-rails/issues/1213): Update `Rails/PluckInWhere` to check for `.ids` call. ([@fatkodima][])
18 changes: 14 additions & 4 deletions lib/rubocop/cop/rails/pluck_in_where.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -42,17 +43,26 @@ 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)
return if style == :conservative && !root_receiver(node)&.const_type?

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

Expand Down
62 changes: 62 additions & 0 deletions spec/rubocop/cop/rails/pluck_in_where_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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

0 comments on commit b9e40e1

Please sign in to comment.