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 #9608] Fix a false positive for Layout/EmptyLineAfterGuardClause
#9609
[Fix #9608] Fix a false positive for Layout/EmptyLineAfterGuardClause
#9609
Conversation
I'm assuming that's problem existing in other cops checking for a blank line after something. |
@@ -41,20 +41,21 @@ class EmptyLineAfterGuardClause < Base | |||
|
|||
MSG = 'Add empty line after guard clause.' | |||
END_OF_HEREDOC_LINE = 1 | |||
RUBOCOP_ENABLE_COMMENT_PATTERN = /#\s*rubocop\s*:\s*enable/.freeze |
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.
We can use DirectiveComment here
@@ -85,8 +86,10 @@ def contains_guard_clause?(node) | |||
node.if_branch&.guard_clause? | |||
end | |||
|
|||
def next_line_empty?(line) | |||
processed_source[line].blank? | |||
def next_line_empty_or_rubocop_enable_comment?(line) |
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 we have to still hold empty line after directive comment. What do you think?
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.
Yeah, I agree. There should still be an empty line, but it should come right after the enable directive.
If it is a general behaviour, maybe we can add an extra helper class to check it? |
@@ -199,6 +199,28 @@ def foo | |||
RUBY | |||
end | |||
|
|||
it 'registers and corrects when using guard clause is after `rubocop:disable` comment' do |
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.
Theres should also be a test making sure an empty line is required after the enable directive.
Should this be specific to directive comments, or maybe generalized to any comment (or multiple lines of comment) after the clause? |
@dvandersluis Only directives. |
@koic ping :-) |
f424686
to
ccf2827
Compare
…rdClause` Fixes rubocop#9608. This PR fixes a false positive for `Layout/EmptyLineAfterGuardClause` when using guard clause is after `rubocop:enable` comment.
ccf2827
to
ccd2ddd
Compare
Thank you for reviews. I updated this PR and I will proceed separately for similar other cops. |
Thanks! |
Fixes #9608.
This PR fixes a false positive for
Layout/EmptyLineAfterGuardClause
when using guard clause is afterrubocop:enable
comment.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.