Fix a false positive for Style/CaseEquality cop
#8526
Conversation
| @@ -33,11 +33,14 @@ class CaseEquality < Base | |||
| extend AutoCorrector | |||
|
|
|||
| MSG = 'Avoid the use of the case equality operator `===`.' | |||
| CAMEL_CASE = /[A-Z]+[a-z]+/.freeze | |||
There was a problem hiding this comment.
I think this pattern should be more specific, otherwise SomeNamespace::REGXP would pass when it shouldn't. I'd suggest something like \A(?=.*[a-z])(?:[A-Z][a-z]*|::)+\z instead, but I'm not sure if that's good enough.
There was a problem hiding this comment.
This regular expression is based on the following:
https://github.com/rubocop-hq/rubocop/blob/v0.89.1/lib/rubocop/cop/style/inverse_methods.rb#L45
But the suggested regexp seems more better. I updated this PR. Thank you.
There was a problem hiding this comment.
Considering that the last component of a constant name is all that matters, this pattern could work better: (\A|::)(?=.*[a-z])([A-Z][a-z0-9]*)+\z. (I also took digits into account)
In any case, class names like URI::HTTP and Addressable::URI cannot be handled nicely unless you keep a known class name list.
There was a problem hiding this comment.
Just do lhs.children.last.match? /[[:lower:]]/. The last child is the name of the constant without its scope. parser did all this nice work for us, let's avoid checking the source whenever we can.
There was a problem hiding this comment.
Maybe I should introduce a ConstNode and provide class_const?
There was a problem hiding this comment.
How about this... rubocop/rubocop-ast#99
There was a problem hiding this comment.
@marcandre Thank you for creating the new API. I'd like to replace it with class_name? when new RuboCop AST is released.
There was a problem hiding this comment.
@koic I added it as module_name? and aliased it to class_name? 👍
There was a problem hiding this comment.
I've updated this PR with module_name?. This PR requires the next RuboCop AST release (0.4.0?) due to its dependency on the unreleased API.
koic
force-pushed
the
fix_false_positive_for_style_case_equality
branch
2 times, most recently
from
94c1a9b
to
a65594f
Compare
koic
force-pushed
the
fix_false_positive_for_style_case_equality
branch
from
a65594f
to
bfac3d7
Compare
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)` ```
koic
force-pushed
the
fix_false_positive_for_style_case_equality
branch
from
bfac3d7
to
252fe51
Compare
| @@ -38,7 +38,7 @@ Gem::Specification.new do |s| | |||
| s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0') | |||
| s.add_runtime_dependency('regexp_parser', '>= 1.7') | |||
| s.add_runtime_dependency('rexml') | |||
| s.add_runtime_dependency('rubocop-ast', '>= 0.3.0', '< 1.0') | |||
| s.add_runtime_dependency('rubocop-ast', '>= 0.4.0', '< 1.0') | |||
There was a problem hiding this comment.
RuboCop AST 0.4.0 has been released and I updated this one.
https://rubygems.org/gems/rubocop-ast/versions/0.4.0
Follow #8322 (comment)
Fix a false positive for
Style/CaseEqualitycop 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.:
This PR will only consider camel-cased constant as class name. e.g.:
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 RuboCop for itself, and generates the documentation.