Skip to content
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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1 @@
* [#9608](https://github.com/rubocop/rubocop/issues/9608): Fix a false positive for `Layout/EmptyLineAfterGuardClause` when using guard clause is after `rubocop:enable` comment. ([@koic][])
23 changes: 20 additions & 3 deletions lib/rubocop/cop/layout/empty_line_after_guard_clause.rb
Expand Up @@ -47,14 +47,13 @@ def on_if(node)

if node.modifier_form? && last_argument_is_heredoc?(node)
heredoc_node = last_heredoc_argument(node)

return if next_line_empty?(heredoc_line(node, heredoc_node))
return if next_line_empty_or_enable_directive_comment?(heredoc_line(node, heredoc_node))

add_offense(heredoc_node.loc.heredoc_end) do |corrector|
autocorrect(corrector, heredoc_node)
end
else
return if next_line_empty?(node.last_line)
return if next_line_empty_or_enable_directive_comment?(node.last_line)

add_offense(offense_location(node)) do |corrector|
autocorrect(corrector, node)
Expand All @@ -71,6 +70,11 @@ def autocorrect(corrector, node)
range_by_whole_lines(node.source_range)
end

next_line = node_range.last_line + 1
if next_line_enable_directive_comment?(next_line)
node_range = processed_source.comment_at_line(next_line)
end

corrector.insert_after(node_range, "\n")
end

Expand All @@ -85,10 +89,23 @@ def contains_guard_clause?(node)
node.if_branch&.guard_clause?
end

def next_line_empty_or_enable_directive_comment?(line)
return true if next_line_empty?(line)

next_line = line + 1
next_line_enable_directive_comment?(next_line) && next_line_empty?(next_line)
end

def next_line_empty?(line)
processed_source[line].blank?
end

def next_line_enable_directive_comment?(line)
return false unless (comment = processed_source.comment_at_line(line))

DirectiveComment.new(comment).enabled?
end

def next_line_rescue_or_ensure?(node)
parent = node.parent
parent.nil? || parent.rescue_type? || parent.ensure_type?
Expand Down
5 changes: 5 additions & 0 deletions lib/rubocop/directive_comment.rb
Expand Up @@ -54,6 +54,11 @@ def disabled?
%w[disable todo].include?(mode)
end

# Checks if this directive enables cops
def enabled?
mode == 'enable'
end

# Checks if this directive enables all cops
def enabled_all?
!disabled? && all_cops?
Expand Down
56 changes: 56 additions & 0 deletions spec/rubocop/cop/layout/empty_line_after_guard_clause_spec.rb
Expand Up @@ -199,6 +199,50 @@ def foo
RUBY
end

it 'registers and corrects when using guard clause is after `rubocop:disable` comment' do
Copy link
Collaborator

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.

expect_offense(<<~RUBY)
def foo
return if condition
^^^^^^^^^^^^^^^^^^^ Add empty line after guard clause.
# rubocop:disable Department/Cop
bar
# rubocop:enable Department/Cop
end
RUBY

expect_correction(<<~RUBY)
def foo
return if condition

# rubocop:disable Department/Cop
bar
# rubocop:enable Department/Cop
end
RUBY
end

it 'registers and corrects when using guard clause is after `rubocop:enable` comment' do
expect_offense(<<~RUBY)
def foo
# rubocop:disable Department/Cop
return if condition
^^^^^^^^^^^^^^^^^^^ Add empty line after guard clause.
# rubocop:enable Department/Cop
bar
end
RUBY

expect_correction(<<~RUBY)
def foo
# rubocop:disable Department/Cop
return if condition
# rubocop:enable Department/Cop

bar
end
RUBY
end

it 'accepts modifier if' do
expect_no_offenses(<<~RUBY)
def foo
Expand Down Expand Up @@ -270,6 +314,18 @@ def foo
RUBY
end

it 'accepts using guard clause is after `rubocop:enable` comment' do
expect_no_offenses(<<~RUBY)
def foo
# rubocop:disable Department/Cop
return if condition
# rubocop:enable Department/Cop

bar
end
RUBY
end

it 'accepts a guard clause inside oneliner block' do
expect_no_offenses(<<~RUBY)
def foo
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/directive_comment_spec.rb
Expand Up @@ -133,6 +133,22 @@
end
end

describe '#enabled?' do
subject { directive_comment.enabled? }

[
['when disable', '# rubocop:disable all', false],
['when enable', '# rubocop:enable Foo/Bar', true],
['when todo', '# rubocop:todo all', false]
].each do |example|
context example[0] do
let(:text) { example[1] }

it { is_expected.to eq example[2] }
end
end
end

describe '#all_cops?' do
subject { directive_comment.all_cops? }

Expand Down