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 #704] Add new Rails/StripHeredoc
cop
#706
Conversation
c3a9c0c
to
c64a2a5
Compare
c64a2a5
to
85f75fd
Compare
Follow up rubocop/rubocop-rails#706. This PR adds "Prefer using squiggly heredoc over `strip_heredoc`" rule.
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.
Thanks!
Checked on a real-world repo:
$ rubocop --only Rails/StripHeredoc /Users/pirj/source/real-world-rspec/
57497 files inspected, 258 offenses detected, 258 offenses autocorrectable
All offences look legitimate.
85f75fd
to
d8e0511
Compare
minimum_target_ruby_version 2.3 | ||
|
||
def on_send(node) | ||
return unless (receiver = node.receiver) |
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.
Even though there's some stability in RESTRICT_ON_SEND
, accoring to:
The
on_send
callback is the most used and can be optimized by restricting the acceptable method names with a constantRESTRICT_ON_SEND
.
Do you think it makes sense to keep this line just to be on the safe side?
return unless node.method?(:strip_heredoc)
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.
My suggestion might be overly paranoid.
I can't tell the exact share, but some cops seem to have this duplication, even when there's just one short method name:
https://github.com/rubocop/rubocop/blob/cdf0d4b9abbbefdff63aab9c87c74b20bd8ed8af/lib/rubocop/cop/style/array_join.rb#L27
lib/rubocop/cop/style/float_division.rb
I'm not aware of any practice regarding this. Please don't hesitate to ignore the above suggestion completely as you see fit.
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.
AFAIK, method?
may be redundant with RESTRICT_ON_SEND
because it only checks a method name. So, it is different from the def_node_matcher
that checks other than a method name.
Closes rubocop#704. Enforces the use of squiggly heredoc over `strip_heredoc`. ```ruby # bad <<EOS.strip_heredoc some text EOS # bad <<-EOS.strip_heredoc some text EOS # good <<~EOS some text EOS ```
d8e0511
to
25c2e6b
Compare
Follow up rubocop/rubocop-rails#706. This PR adds "Prefer using squiggly heredoc over `strip_heredoc`" rule.
Closes #704.
Enforces the use of squiggly heredoc over
strip_heredoc
.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.and description in grammatically correct, complete sentences.