From 0723cced1bfe6d471cf37d88397feb1d8f6b8d3c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 12 Aug 2021 17:26:48 +0900 Subject: [PATCH] [Fix #10004] Fix a false positive for `Style/RedundantBegin` Fixes #10004. This PR fixes a false positive for `Style/RedundantBegin` when using one-liner with semicolon. --- ...alse_positive_for_style_redundant_begin.md | 1 + lib/rubocop/cop/style/redundant_begin.rb | 25 ++++++++++ .../rubocop/cop/style/redundant_begin_spec.rb | 47 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 changelog/fix_false_positive_for_style_redundant_begin.md diff --git a/changelog/fix_false_positive_for_style_redundant_begin.md b/changelog/fix_false_positive_for_style_redundant_begin.md new file mode 100644 index 00000000000..c3822bc9351 --- /dev/null +++ b/changelog/fix_false_positive_for_style_redundant_begin.md @@ -0,0 +1 @@ +* [#10004](https://github.com/rubocop/rubocop/issues/10004): Fix a false positive for `Style/RedundantBegin` when using one-liner with semicolon. ([@koic][]) diff --git a/lib/rubocop/cop/style/redundant_begin.rb b/lib/rubocop/cop/style/redundant_begin.rb index c0eaccc1f28..4f07b8b024b 100644 --- a/lib/rubocop/cop/style/redundant_begin.rb +++ b/lib/rubocop/cop/style/redundant_begin.rb @@ -84,6 +84,7 @@ def on_block(node) def on_kwbegin(node) return if empty_begin?(node) || + begin_block_has_multiline_statements?(node) || contain_rescue_or_ensure?(node) || valid_context_using_only_begin?(node) @@ -102,6 +103,9 @@ def register_offense(node) corrector.remove(offense_range) end + if use_modifier_form_after_multiline_begin_block?(node) + correct_modifier_form_after_multiline_begin_block(corrector, node) + end corrector.remove(node.loc.end) end end @@ -127,10 +131,31 @@ def restore_removed_comments(corrector, offense_range, node, first_child) corrector.insert_before(node.parent, comments) unless comments.blank? end + def use_modifier_form_after_multiline_begin_block?(node) + return unless (parent = node.parent) + + node.multiline? && parent.if_type? && parent.modifier_form? + end + + def correct_modifier_form_after_multiline_begin_block(corrector, node) + condition_range = condition_range(node.parent) + + corrector.insert_after(node.children.first, " #{condition_range.source}") + corrector.remove(range_by_whole_lines(condition_range, include_final_newline: true)) + end + + def condition_range(node) + range_between(node.loc.keyword.begin_pos, node.condition.source_range.end_pos) + end + def empty_begin?(node) node.children.empty? end + def begin_block_has_multiline_statements?(node) + node.children.count >= 2 + end + def contain_rescue_or_ensure?(node) first_child = node.children.first diff --git a/spec/rubocop/cop/style/redundant_begin_spec.rb b/spec/rubocop/cop/style/redundant_begin_spec.rb index b875420a85a..8d504659a91 100644 --- a/spec/rubocop/cop/style/redundant_begin_spec.rb +++ b/spec/rubocop/cop/style/redundant_begin_spec.rb @@ -415,4 +415,51 @@ def a_method end RUBY end + + it 'accepts when one-liner `begin` block has multiple statements with modifier condition' do + expect_no_offenses(<<~RUBY) + begin foo; bar; end unless condition + RUBY + end + + it 'accepts when multi-line `begin` block has multiple statements with modifier condition' do + expect_no_offenses(<<~RUBY) + begin + foo; bar + end unless condition + RUBY + end + + it 'reports an offense when one-liner `begin` block has single statement with modifier condition' do + expect_offense(<<~RUBY) + begin foo end unless condition + ^^^^^ Redundant `begin` block detected. + RUBY + + expect_correction(" foo unless condition\n") + end + + it 'reports an offense when multi-line `begin` block has single statement with modifier condition' do + expect_offense(<<~RUBY) + begin + ^^^^^ Redundant `begin` block detected. + foo + end unless condition + RUBY + + expect_correction("\n foo unless condition\n") + end + + it 'reports an offense when multi-line `begin` block has single statement and it is inside condition' do + expect_offense(<<~RUBY) + unless condition + begin + ^^^^^ Redundant `begin` block detected. + foo + end + end + RUBY + + expect_correction("unless condition\n \n foo\n \nend\n") + end end