Use Cop::Base API for Lint department
#8395
Conversation
| @@ -21,21 +21,20 @@ module Lint | |||
| # | |||
| # # With parentheses, there's no ambiguity. | |||
| # do_something(/pattern/i) | |||
| class AmbiguousRegexpLiteral < Cop | |||
| class AmbiguousRegexpLiteral < Base | |||
| include ParserDiagnostic | |||
There was a problem hiding this comment.
My 2 cents is that this module doesn't do enough to justify its existence and makes things more complicated than if it didn't exist. If you think it should exist, than maybe it could define on_new_investigation and call on_diagnostic, but that's it. There is nothing else it does that is of any use.
|
Wow, impressive PR 💪 🙇♂️ |
koic
force-pushed
the
use_cop_base_api_for_lint_department
branch
from
b66d27a
to
40c21b0
Compare
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def autcocorrect(comment) |
There was a problem hiding this comment.
Oh! Interesting! 😅
This is a problem, because now the block is called even if autocorrection won't happen...
So this would always do the chmod.
I had not considered this case 😆, and there's no test for that either I think.
If you add if autocorrect_requested? you should be pretty much ok.
(Also, there's a typo)
There was a problem hiding this comment.
(Also, there's a typo)
Oops! I missed this typo 💦
This is a problem, because now the block is called even if autocorrection won't happen...
I did print debugging. The block execution seems to be running only when offensed. Is if autocorrect_requested? required?
There was a problem hiding this comment.
I added a spec in #8441
I think that if you rebase now, you will get a failure. Sorry, the expect_offense method need improvement...
There was a problem hiding this comment.
The build failed as expected with #8441. I added if autocorrect_requested? and the build has been passed. Thank you for adding the reproduction test.
lib/rubocop/cop/lint/uri_regexp.rb
Outdated
| end | ||
| end | ||
|
|
||
| private | ||
| def autocorrect(corrector, node) | ||
| if (captured_values = uri_regexp_with_argument?(node)) |
There was a problem hiding this comment.
I'm surprised we don't have a cop to change this to
captured_values = uri_regexp_with_argument?(node) ||
uri_regexp_without_argument?(node)
There was a problem hiding this comment.
Sure! I've also improved other redundant logic in uri_regexp.rb. Thank you for your review ❤️
koic
force-pushed
the
use_cop_base_api_for_lint_department
branch
from
40c21b0
to
8624579
Compare
Yeah, I would like to refactor it in another PR due to this update changes the module structure. |
See discussion of [rubocop#8395]
Follow rubocop#7868. This PR uses `Cop::Base` API for `Lint` department's cops.
koic
force-pushed
the
use_cop_base_api_for_lint_department
branch
from
8624579
to
71c9653
Compare
|
Cool! Great work 💪
Looking forward to it, thanks! |
Follow rubocop#8395 (comment). This PR does a refactoring to inline `ParserDiagnostic` module. It seems that it is simpler and easier to maintain if it is implemented by each cop rather than mix-in the module. I don't think `ParserDiagnostic` module is used outside of RuboCop core, at least not in the RuboCop HQ products. On the other hand, I added it to the changelog just in case for 3rd party gems.
|
I've opened #8442 :-) |
Follow #8395 (comment). This PR does a refactoring to inline `ParserDiagnostic` module. It seems that it is simpler and easier to maintain if it is implemented by each cop rather than mix-in the module. I don't think `ParserDiagnostic` module is used outside of RuboCop core, at least not in the RuboCop HQ products. On the other hand, I added it to the changelog just in case for 3rd party gems.
Follow rubocop#7868, rubocop#8377, and rubocop#8395 This PR uses `Cop::Base` API for `Lint/TopLevelReturnWithArgument`.
Follow #7868.
This PR uses
Cop::BaseAPI forLintdepartment's cops.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.