From bf7ed9d0ebfe91b9d18a35c1fc6220105e553af0 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 17 Mar 2021 02:56:32 +0900 Subject: [PATCH] [Fix #9608] Fix a false positive for `Layout/EmptyLineAfterGuardClause` Fixes #9608. This PR fixes a false positive for `Layout/EmptyLineAfterGuardClause` when using guard clause is after `rubocop:enable` comment. --- ...or_layout_empty_line_after_guard_clause.md | 1 + .../layout/empty_line_after_guard_clause.rb | 23 +++++++- lib/rubocop/directive_comment.rb | 5 ++ .../empty_line_after_guard_clause_spec.rb | 56 +++++++++++++++++++ spec/rubocop/directive_comment_spec.rb | 16 ++++++ 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 changelog/fix_false_positive_for_layout_empty_line_after_guard_clause.md diff --git a/changelog/fix_false_positive_for_layout_empty_line_after_guard_clause.md b/changelog/fix_false_positive_for_layout_empty_line_after_guard_clause.md new file mode 100644 index 00000000000..ecfadc7be2f --- /dev/null +++ b/changelog/fix_false_positive_for_layout_empty_line_after_guard_clause.md @@ -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][]) diff --git a/lib/rubocop/cop/layout/empty_line_after_guard_clause.rb b/lib/rubocop/cop/layout/empty_line_after_guard_clause.rb index 7e0c20162cb..88d5a05e264 100644 --- a/lib/rubocop/cop/layout/empty_line_after_guard_clause.rb +++ b/lib/rubocop/cop/layout/empty_line_after_guard_clause.rb @@ -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) @@ -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 @@ -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? diff --git a/lib/rubocop/directive_comment.rb b/lib/rubocop/directive_comment.rb index 6b55d7f08f1..024dfbf7677 100644 --- a/lib/rubocop/directive_comment.rb +++ b/lib/rubocop/directive_comment.rb @@ -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? diff --git a/spec/rubocop/cop/layout/empty_line_after_guard_clause_spec.rb b/spec/rubocop/cop/layout/empty_line_after_guard_clause_spec.rb index fc1e9dd45ec..cc3ae7a989e 100644 --- a/spec/rubocop/cop/layout/empty_line_after_guard_clause_spec.rb +++ b/spec/rubocop/cop/layout/empty_line_after_guard_clause_spec.rb @@ -199,6 +199,50 @@ def foo RUBY end + it 'registers and corrects when using guard clause is after `rubocop:disable` comment' do + 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 @@ -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 diff --git a/spec/rubocop/directive_comment_spec.rb b/spec/rubocop/directive_comment_spec.rb index 7e16870c729..56a66638a98 100644 --- a/spec/rubocop/directive_comment_spec.rb +++ b/spec/rubocop/directive_comment_spec.rb @@ -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? }