From e75331131d9ac5071a12f48b68d334bec58108a3 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 19 Sep 2020 00:07:51 +0900 Subject: [PATCH] Fix a false positive for `Layout/EmptyLinesAroundAccessModifier` This PR fixes the following infinite loop error for `Layout/EmptyLinesAroundAccessModifier` with `Layout/EmptyLinesAroundBlockBody` when using access modifier with block argument. ```console % cat example.rb FactoryBot.define do factory :model do name { 'hoge' } private { true } end end % rubocop -a --only Layout/EmptyLinesAroundAccessModifier,Layout/EmptyLinesAroundBlockBody (snip) Inspecting 1 file C Offenses: example.rb:5:5: C: [Corrected] Layout/EmptyLinesAroundAccessModifier: Keep a blank line before and after private. private { true } ^^^^^^^ example.rb:6:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end. 0 files inspected, 2 offenses detected, 2 offenses corrected Infinite loop detected in /Users/koic/src/github.com/koic/rubocop-issues/factory_bot/example.rb and caused by Layout/EmptyLinesAroundAccessModifier -> Layout/EmptyLinesAroundBlockBody ``` --- CHANGELOG.md | 1 + .../empty_lines_around_access_modifier.rb | 22 +++++++++----- spec/rubocop/cli/cli_autocorrect_spec.rb | 30 +++++++++++++++++++ ...empty_lines_around_access_modifier_spec.rb | 8 +++++ 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75c67127dc5..db7bb5c702d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#8742](https://github.com/rubocop-hq/rubocop/issues/8742): Fix some assignment counts for `Metrics/AbcSize`. ([@marcandre][]) * [#8750](https://github.com/rubocop-hq/rubocop/pull/8750): Fix an incorrect auto-correct for `Style/MultilineWhenThen` when line break for multiple condidate values of `when` statement. ([@koic][]) * [#8754](https://github.com/rubocop-hq/rubocop/pull/8754): Fix an error for `Style/RandomWithOffset` when using a range with non-integer bounds. ([@eugeneius][]) +* [#8756](https://github.com/rubocop-hq/rubocop/issues/8756): Fix an infinite loop error for `Layout/EmptyLinesAroundAccessModifier` with `Layout/EmptyLinesAroundBlockBody` when using access modifier with block argument. ([@koic][]) ### Changes diff --git a/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb b/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb index 90f9e743220..b0f02e3c43e 100644 --- a/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb +++ b/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb @@ -84,14 +84,7 @@ def on_block(node) end def on_send(node) - return unless node.bare_access_modifier? - - case style - when :around - return if empty_lines_around?(node) - when :only_before - return if allowed_only_before_style?(node) - end + return unless register_offense?(node) message = message(node) add_offense(node, message: message) do |corrector| @@ -105,6 +98,19 @@ def on_send(node) private + def register_offense?(node) + return false unless node.bare_access_modifier? && !node.parent.block_type? + + case style + when :around + return false if empty_lines_around?(node) + when :only_before + return false if allowed_only_before_style?(node) + end + + true + end + def allowed_only_before_style?(node) if node.special_modifier? return true if processed_source[node.last_line] == 'end' diff --git a/spec/rubocop/cli/cli_autocorrect_spec.rb b/spec/rubocop/cli/cli_autocorrect_spec.rb index a429b30ef1f..514839eb11e 100644 --- a/spec/rubocop/cli/cli_autocorrect_spec.rb +++ b/spec/rubocop/cli/cli_autocorrect_spec.rb @@ -1599,6 +1599,36 @@ def self.some_method(foo, bar: 1) RUBY end + it 'does not crash when using Lint/SafeNavigationWithEmpty and Layout/EmptyLinesAroundBlockBody' do + create_file('example.rb', <<~RUBY) + FactoryBot.define do + factory :model do + name { 'value' } + + private { value } + end + end + RUBY + + expect( + cli.run( + [ + '--auto-correct', + '--only', 'Layout/EmptyLinesAroundAccessModifier,Layout/EmptyLinesAroundBlockBody' + ] + ) + ).to eq(0) + expect(IO.read('example.rb')).to eq(<<~RUBY) + FactoryBot.define do + factory :model do + name { 'value' } + + private { value } + end + end + RUBY + end + it 'corrects TrailingCommaIn(Array|Hash)Literal and ' \ 'Multiline(Array|Hash)BraceLayout offenses' do create_file('.rubocop.yml', <<~YAML) diff --git a/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb b/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb index 027ab3b2a57..74c092b77f4 100644 --- a/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb +++ b/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb @@ -105,6 +105,14 @@ def #{access_modifier}? RUBY end + it "ignores #{access_modifier} with block argument" do + expect_no_offenses(<<~RUBY) + def foo + #{access_modifier} { do_something } + end + RUBY + end + it 'autocorrects blank line after #{access_modifier} with comment' do expect_offense(<<~RUBY) class Test