-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
Make some cops aware of safe navigation operator #1204
Make some cops aware of safe navigation operator #1204
Conversation
5eb526a
to
3ef1c5b
Compare
@@ -112,6 +123,17 @@ | |||
RUBY | |||
end | |||
|
|||
it 'registers an offense and corrects when using `!=` and anonymous placeholder with safe navigation' do | |||
expect_offense(<<~RUBY) | |||
User&.where(['name != ?', 'Gabe']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a violation for some cops.
ydakuka@yauhenid:~/Work/project$ bin/rails_docker rubocop app/models/user.rb
Inspecting 1 file
W
Offenses:
app/models/user.rb:26:9: W: [Correctable] Lint/RedundantSafeNavigation: Redundant safe navigation detected.
User&.where.not(name: 'Gabe')
^^^^^^^
app/models/user.rb:26:16: W: [Correctable] Lint/SafeNavigationChain: Do not chain ordinary method call after safe navigation operator.
User&.where.not(name: 'Gabe')
^^^^^^^^^^^^^^^^^^
1 file inspected, 2 offenses detected, 2 offenses autocorrectable
I think I should be replaced with the following:
users = User.all
users&.where&.not(name: 'Gabe')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this seems like an overreach for the role of this cop. The role is ensure that "manually constructed SQL in where
can be replaced with where.not(...)
" only.
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
User&.where.not(name: 'Gabe') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
irb(main):007:0> users = nil
=> nil
irb(main):008:0> users&.where.not(name: 'Gabe')
Traceback (most recent call last):
1: from (irb):8
NoMethodError (undefined method `not' for nil:NilClass)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I've updated it.
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
User&.where(name: 'john').exists? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
irb(main):010:0> users = nil
=> nil
irb(main):011:0> users&.where(name: 'john').exists?
Traceback (most recent call last):
1: from (irb):11
NoMethodError (undefined method `exists?' for nil:NilClass)
496fa6d
to
43d7d73
Compare
Fixes rubocop#1191, rubocop#1192, rubocop#1193, rubocop#1194, rubocop#1196, rubocop#1197, rubocop#1201, and rubocop#1202. This PR makes `Rails/ActiveSupportAliases`, `Rails/FindBy`, `Rails/FindById`, `Rails/Inquiry`, `Rails/Pick` `Rails/PluckId`, `Rails/PluckInWhere`, `Rails/WhereEquals`, `Rails/WhereExists`, and `Rails/WhereNot` cops aware of safe navigation operator.
43d7d73
to
7c672a0
Compare
Fixes #1191, #1192, #1193, #1194, #1196, #1197, #1201, and #1202.
This PR makes
Rails/ActiveSupportAliases
,Rails/FindBy
,Rails/FindById
,Rails/Inquiry
,Rails/Pick
Rails/PluckId
,Rails/PluckInWhere
,Rails/WhereEquals
,Rails/WhereExists
, andRails/WhereNot
cops aware of safe navigation operator.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.