Skip to content

Commit

Permalink
[Fix rubocop#341] Add EnforcedStyle to Rails/WhereExists to allow whe…
Browse files Browse the repository at this point in the history
…re(...).exists? to be the enforced style
  • Loading branch information
dvandersluis committed Sep 2, 2020
1 parent 820df97 commit 66bb979
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 46 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* [#310](https://github.com/rubocop-hq/rubocop-rails/issues/310): Add `EnforcedStyle` to `Rails/PluckInWhere`. By default, it does not register an offense if `pluck` method's receiver is a variable. ([@koic][])
* [#320](https://github.com/rubocop-hq/rubocop-rails/pull/320): Mark `Rails/UniqBeforePluck` as unsafe auto-correction. ([@kunitoo][])
* [#324](https://github.com/rubocop-hq/rubocop-rails/pull/324): Make `Rails/IndexBy` and `Rails/IndexWith` aware of `to_h` with block. ([@eugeneius][])
* [#341](https://github.com/rubocop-hq/rubocop-rails/pull/341): Make `Rails/WhereExists` configurable to allow `where(...).exists?` to be the prefered style. ([@dvandersluis][])

## 2.7.1 (2020-07-26)

Expand Down Expand Up @@ -273,3 +274,4 @@
[@jaredmoody]: https://github.com/jaredmoody
[@mobilutz]: https://github.com/mobilutz
[@bubaflub]: https://github.com/bubaflub
[@dvandersluis]: https://github.com/dvandersluis
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,12 @@ Rails/Validation:
Rails/WhereExists:
Description: 'Prefer `exists?(...)` over `where(...).exists?`.'
Enabled: 'pending'
EnforcedStyle: exists
SupportedStyles:
- exists
- where
VersionAdded: '2.7'
VersionChanged: '2.8'

Rails/WhereNot:
Description: 'Use `where.not(...)` instead of manually constructing negated SQL in `where`.'
Expand Down
40 changes: 38 additions & 2 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4293,13 +4293,21 @@ validates :foo, uniqueness: true
| Yes
| Yes
| 2.7
| -
| 2.8
|===

This cop enforces the use of `exists?(...)` over `where(...).exists?`.
This cop enforces consistent style when using `exists?`.

Two styles are supported for this cop. When EnforcedStyle is 'exists'
then the cop enforces `exists?(...)` over `where(...).exists?`.

When EnforcedStyle is 'where' then the cop enforces
`where(...).exists?` over `exists?(...)`.

=== Examples

==== EnforcedStyle: exists

[source,ruby]
----
# bad
Expand All @@ -4314,6 +4322,34 @@ User.where('length(name) > 10').exists?
user.posts.exists?(published: true)
----

==== EnforcedStyle: where

[source,ruby]
----
# bad
User.exists?(name: 'john')
User.exists?(['name = ?', 'john'])
User.exists?('name = ?', 'john')
user.posts.exists?(published: true)
# good
User.where(name: 'john').exists?
User.where(['name = ?', 'john']).exists?
User.where('name = ?', 'john').exists?
user.posts.where(published: true).exists?
User.where('length(name) > 10').exists?
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| EnforcedStyle
| `exists`
| `exists`, `where`
|===

== Rails/WhereNot

|===
Expand Down
73 changes: 68 additions & 5 deletions lib/rubocop/cop/rails/where_exists.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
module RuboCop
module Cop
module Rails
# This cop enforces the use of `exists?(...)` over `where(...).exists?`.
# This cop enforces consistent style when using `exists?`.
#
# @example
# Two styles are supported for this cop. When EnforcedStyle is 'exists'
# then the cop enforces `exists?(...)` over `where(...).exists?`.
#
# When EnforcedStyle is 'where' then the cop enforces
# `where(...).exists?` over `exists?(...)`.
#
# @example EnforcedStyle: exists
# # bad
# User.where(name: 'john').exists?
# User.where(['name = ?', 'john']).exists?
Expand All @@ -17,15 +23,34 @@ module Rails
# User.where('length(name) > 10').exists?
# user.posts.exists?(published: true)
#
# @example EnforcedStyle: where
# # bad
# User.exists?(name: 'john')
# User.exists?(['name = ?', 'john'])
# User.exists?('name = ?', 'john')
# user.posts.exists?(published: true)
#
# # good
# User.where(name: 'john').exists?
# User.where(['name = ?', 'john']).exists?
# User.where('name = ?', 'john').exists?
# user.posts.where(published: true).exists?
# User.where('length(name) > 10').exists?
class WhereExists < Cop
include ConfigurableEnforcedStyle

MSG = 'Prefer `%<good_method>s` over `%<bad_method>s`.'

def_node_matcher :where_exists_call?, <<~PATTERN
(send (send _ :where $...) :exists?)
PATTERN

def_node_matcher :exists_with_args?, <<~PATTERN
(send _ :exists? $...)
PATTERN

def on_send(node)
where_exists_call?(node) do |args|
find_offenses(node) do |args|
return unless convertable_args?(args)

range = correction_range(node)
Expand All @@ -35,7 +60,7 @@ def on_send(node)
end

def autocorrect(node)
args = where_exists_call?(node)
args = find_offenses(node)

lambda do |corrector|
corrector.replace(
Expand All @@ -47,21 +72,59 @@ def autocorrect(node)

private

def where_style?
style == :where
end

def exists_style?
style == :exists
end

def find_offenses(node, &block)
if exists_style?
where_exists_call?(node, &block)
elsif where_style?
exists_with_args?(node, &block)
end
end

def convertable_args?(args)
return false if args.empty?

args.size > 1 || args[0].hash_type? || args[0].array_type?
end

def correction_range(node)
node.receiver.loc.selector.join(node.loc.selector)
if exists_style?
node.receiver.loc.selector.join(node.loc.selector)
elsif where_style?
node.loc.selector.with(end_pos: node.loc.expression.end_pos)
end
end

def build_good_method(args)
if exists_style?
build_good_method_exists(args)
elsif where_style?
build_good_method_where(args)
end
end

def build_good_method_exists(args)
if args.size > 1
"exists?([#{args.map(&:source).join(', ')}])"
else
"exists?(#{args[0].source})"
end
end

def build_good_method_where(args)
if args.size > 1
"where(#{args.map(&:source).join(', ')}).exists?"
else
"where(#{args[0].source}).exists?"
end
end
end
end
end
Expand Down
Loading

0 comments on commit 66bb979

Please sign in to comment.