Skip to content

Commit

Permalink
Mark Rails/WhereExists as unsafe auto-correction
Browse files Browse the repository at this point in the history
## Summary

This cop is unsafe because the behavior may change on the following case:

```ruby
Author.includes(:articles).where(articles: {id: id}).exists?
#=> Perform `eager_load` behaviour (`LEFT JOIN` query) and get result.

Author.includes(:articles).exists?(articles: {id: id})
#=> Perform `preload` behaviour and `ActiveRecord::StatementInvalid` error occurs.
```

## Additional Information

And this cop may be considered disabled by default as there is no consensus on the default style.

- rubocop#286
- rubocop#342
  • Loading branch information
koic committed Apr 3, 2021
1 parent 442f612 commit 6857bdd
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* [#381](https://github.com/rubocop/rubocop-rails/pull/381): Update `IgnoredMethods` list for `Lint/NumberConversion` to allow Rails' duration methods. ([@dvandersluis][])
* [#444](https://github.com/rubocop/rubocop-rails/issues/444): Mark `Rails/Blank` as unsafe auto-correction. ([@koic][])
* [#451](https://github.com/rubocop/rubocop-rails/issues/451): Make `Rails/RelativeDateConstant` aware of `yesterday` and `tomorrow` methods. ([@koic][])
* [#454](https://github.com/rubocop/rubocop-rails/pull/454): Mark `Rails/WhereExists` as unsafe auto-correction. ([@koic][])

## 2.9.1 (2020-12-16)

Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ Rails/WhereEquals:
Rails/WhereExists:
Description: 'Prefer `exists?(...)` over `where(...).exists?`.'
Enabled: 'pending'
SafeAutoCorrect: false
EnforcedStyle: exists
SupportedStyles:
- exists
Expand Down
13 changes: 12 additions & 1 deletion docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4512,7 +4512,7 @@ User.where(users: { name: 'Gabe' })

| Pending
| Yes
| Yes
| Yes (Unsafe)
| 2.7
| 2.8
|===
Expand All @@ -4525,6 +4525,17 @@ then the cop enforces `exists?(...)` over `where(...).exists?`.
When EnforcedStyle is 'where' then the cop enforces
`where(...).exists?` over `exists?(...)`.

This cop is unsafe for auto-correction because the behavior may change on the following case:

[source,ruby]
----
Author.includes(:articles).where(articles: {id: id}).exists?
#=> Perform `eager_load` behavior (`LEFT JOIN` query) and get result.
Author.includes(:articles).exists?(articles: {id: id})
#=> Perform `preload` behavior and `ActiveRecord::StatementInvalid` error occurs.
----

=== Examples

==== EnforcedStyle: exists (default)
Expand Down
11 changes: 11 additions & 0 deletions lib/rubocop/cop/rails/where_exists.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ module Rails
# When EnforcedStyle is 'where' then the cop enforces
# `where(...).exists?` over `exists?(...)`.
#
# This cop is unsafe for auto-correction because the behavior may change on the following case:
#
# [source,ruby]
# ----
# Author.includes(:articles).where(articles: {id: id}).exists?
# #=> Perform `eager_load` behavior (`LEFT JOIN` query) and get result.
#
# Author.includes(:articles).exists?(articles: {id: id})
# #=> Perform `preload` behavior and `ActiveRecord::StatementInvalid` error occurs.
# ----
#
# @example EnforcedStyle: exists (default)
# # bad
# User.where(name: 'john').exists?
Expand Down

0 comments on commit 6857bdd

Please sign in to comment.