Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Rails/WhereExists cop #286

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Conversation

fatkodima
Copy link
Contributor

This cop enforces the use of exists?(...) over where(...).exists?.

  # bad
  User.where(name: 'john').exists?
  User.where(['name = ?', 'john']).exists?
  User.where('name = ?', 'john').exists?
  user.posts.where(published: true).exists?

  # good
  User.exists?(name: 'john')
  User.where('length(name) > 10').exists?
  user.posts.exists?(published: true)

@koic koic merged commit d1fe676 into rubocop:master Jul 15, 2020
@koic
Copy link
Member

koic commented Jul 15, 2020

Thanks!

@beauraF
Copy link

beauraF commented Jul 27, 2020

I find this less readable. Is there another reason than consistency involve in this choice?

@fatkodima
Copy link
Contributor Author

Just consistency and style preferences.

@dvandersluis
Copy link
Member

Can this be made configurable so that the opposite can be enforced? where(...).exists? instead of exists?(...)

@dvandersluis
Copy link
Member

I created #342 to make the preferred style configurable.

koic added a commit to koic/rubocop-rails that referenced this pull request Apr 2, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 2, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 3, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 3, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 3, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 3, 2021
## 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
koic added a commit to koic/rubocop-rails that referenced this pull request Apr 8, 2021
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants