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
Support autocorrect for Style/CaseEquality
cop
#8322
Support autocorrect for Style/CaseEquality
cop
#8322
Conversation
@fatkodima Since rubocop 0.88 is released, you should update |
ea3bd14
to
dfee211
Compare
Updated. |
dfee211
to
c6b2f86
Compare
@@ -29,13 +29,20 @@ module Style | |||
# (1..100).include?(7) | |||
# some_string =~ /something/ | |||
# | |||
class CaseEquality < Cop | |||
class CaseEquality < Base |
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 think this should not be changed:
class CaseEquality < Base | |
class CaseEquality < Cop |
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.
The auto-correction has been added by the new API, I think it is okay to extend Base
. Rather, changing the inheritance to Cop
will fail tests.
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.
@koic I might be a little confused between Base and Cop. I see most cops inherit from Cop
, do not extend AutoCorrector
and have separate method autocorrect
. Which of these is the preferred/new approach?
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.
Inheriting from Base
is preferred/new approach. Here's the doc: https://docs.rubocop.org/rubocop/v1_upgrade_notes.html
child = lhs.children.first | ||
"#{lhs.source}.include?(#{rhs.source})" if child&.range_type? | ||
when :const | ||
"#{rhs.source}.is_a?(#{lhs.source})" |
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.
Not all constants are class/module instances. Regexps are often bound to a constant, to be used with ===
.
I think a better bet is to replace the expression only if the LHS constant is CamelCased.
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 feedback. I've opened the PR #8526.
def replacement(lhs, rhs) | ||
case lhs.type | ||
when :regexp | ||
"#{rhs.source} =~ #{lhs.source}" |
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.
/regexp/ === object
and object =~ /regexp/
are not interchangeable. The former returns true or false whereas the latter returns an index or nil.
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.
Exactly! I've opened the PR #8527.
Follow rubocop#8322 (comment) This PR prevents an incorrect auto-correction for `Style/CaseEquality` cop when comparing with `===` against a regular expression receiver. OTOH, the automatic correction from `a === b` to `a.match?(b)` needs to consider `Regexp.last_match?`, `$~`, `$1`, and etc. This correction is expected to be supported by `Performance/Regexp` cop. See: rubocop/rubocop-performance#152
Follow rubocop#8322 (comment) Fix a false positive for `Style/CaseEquality` cop when the receiver is not a camel cased constant. The following constant should not be replaced with `is_a?`. What kind of object is actually assigned depends on the constant. e.g.: ```ruby REGEXP_CONSTANT === something #=> does not register an offense. ``` This PR will only consider camel-cased constant as class name. e.g.: ```ruby Array === something #=> auto-correct to `something.is_a?(Array)` ```
No description provided.