-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix Rails/Presence cop when #present? or #blank? is used in if or unless modifier #5238
Conversation
be920c7
to
2d2a07d
Compare
|
||
it 'does not register an offense when if or unless modifier is used ' do | ||
[ | ||
'foo if foo.present?', |
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.
In this case, I think it should register an offense for this code...
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.
I tried to register this pattern as offense, but I could not get it right. Could somebody help me?
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.
Probably we should rewrite the redundant_receiver_and_other
and/or redundant_negative_receiver_and_other
patterns. Let's try to rewrite the patterns with a documentation of Node Pattern, or replace the expect_no_offenses
with expect_offense
and make pending the test case.
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.
Thank you for your advice. I didn't have to rewrite neither redundant_receiver_and_other
or redundant_negative_receiver_and_other
. What I did was that adding a unless a.blank?
to offense.
Thank you for fixing. I think this is a serious bug 🙇♂️ |
lib/rubocop/cop/rails/presence.rb
Outdated
@@ -75,7 +75,7 @@ def on_if(node) | |||
unless receiver | |||
receiver, other = redundant_negative_receiver_and_other(node) | |||
end | |||
return unless receiver | |||
return unless receiver && other |
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.
Maybe it should use blocks.
For example:
redundant_receiver_and_other(node) do |receiver, other|
# ...
end
redundant_negative_receiver_and_other(node) do |receiver, other|
# ...
end
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 idea!
ab6fa10
to
3c1714a
Compare
3c1714a
to
9020588
Compare
👍 |
This PR should fix
Rails/Presence
cop when#present?
on#blank?
is used inif
orunless
modifier.Below is a sample output that represents bug.
Before submitting the PR make sure the following are checked:
Commit message starts with[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).and description in grammatically correct, complete sentences.
rake default
orrake parallel
. It executes all tests and RuboCop for itself, and generates the documentation.