From 7be423a6b9618e999ea90c5bb60a3f0d4940e79f Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 6 May 2021 07:56:26 +0900 Subject: [PATCH] Fix an incorrect auto-correct for `Style/RedundantBegin` This PR fixes an incorrect auto-correct for `Style/RedundantBegin` when using multi-line `if` in `begin` block. And fixes https://github.com/testdouble/standard/issues/289 that is causing the following infinite loop error due to `Style/RedundantBegin` with `Style/MultilineMemoization`. ```ruby % cat example.rb @memo ||= begin if condition do_something end end % bundle exec rubocop -a --only Style/RedundantBegin,Style/MultilineMemoization (snip) Inspecting 1 file C Offenses: example.rb:1:1: C: [Corrected] Style/MultilineMemoization: Wrap multiline memoization blocks in begin and end. @memo ||= (if condition ... ^^^^^^^^^^^^^^^^^^^^^^^ example.rb:1:11: C: [Corrected] Style/RedundantBegin: Redundant begin block detected. @memo ||= begin ^^^^^ 0 files inspected, 2 offenses detected, 2 offenses corrected Infinite loop detected in /Users/koic/src/github.com/koic/rubocop-issues/289/example.rb and caused by Style/MultilineMemoization /Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/runner.rb:285:in `block in iterate_until_no_changes' /Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/runner.rb:281:in `loop' /Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/runner.rb:281:in `iterate_until_no_changes' ``` --- ...t_autocorrect_for_style_redundant_begin.md | 1 + lib/rubocop/cop/style/redundant_begin.rb | 2 +- spec/rubocop/cli/autocorrect_spec.rb | 19 +++++++++++++++++++ .../rubocop/cop/style/redundant_begin_spec.rb | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_an_incorrect_autocorrect_for_style_redundant_begin.md diff --git a/changelog/fix_an_incorrect_autocorrect_for_style_redundant_begin.md b/changelog/fix_an_incorrect_autocorrect_for_style_redundant_begin.md new file mode 100644 index 00000000000..9aa23635859 --- /dev/null +++ b/changelog/fix_an_incorrect_autocorrect_for_style_redundant_begin.md @@ -0,0 +1 @@ +* [#9777](https://github.com/rubocop/rubocop/pull/9777): Fix an incorrect auto-correct for `Style/RedundantBegin` when using multi-line `if` in `begin` block. ([@koic][]) diff --git a/lib/rubocop/cop/style/redundant_begin.rb b/lib/rubocop/cop/style/redundant_begin.rb index 46f8ebd8e3c..c0eaccc1f28 100644 --- a/lib/rubocop/cop/style/redundant_begin.rb +++ b/lib/rubocop/cop/style/redundant_begin.rb @@ -110,7 +110,7 @@ def replace_begin_with_statement(corrector, offense_range, node) first_child = node.children.first source = first_child.source - source = "(#{source})" if first_child.if_type? + source = "(#{source})" if first_child.if_type? && first_child.modifier_form? corrector.replace(offense_range, source) corrector.remove(range_between(offense_range.end_pos, first_child.source_range.end_pos)) diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb index 0eab209480a..904bcbf5b60 100644 --- a/spec/rubocop/cli/autocorrect_spec.rb +++ b/spec/rubocop/cli/autocorrect_spec.rb @@ -553,6 +553,25 @@ def verify_section expect(File.read('example.rb')).to eq(corrected) end + it 'corrects `Style/RedundantBegin` with `Style/MultilineMemoization`' do + source = <<~RUBY + @memo ||= begin + if condition + do_something + end + end + RUBY + create_file('example.rb', source) + expect(cli.run(['-a', '--only', 'Style/RedundantBegin,Style/MultilineMemoization'])).to eq(0) + corrected = <<~RUBY + @memo ||= if condition + do_something + end + #{trailing_whitespace * 10} + RUBY + expect(File.read('example.rb')).to eq(corrected) + end + it 'corrects LineEndConcatenation offenses leaving the ' \ 'RedundantInterpolation offense unchanged' do # If we change string concatenation from plus to backslash, the string diff --git a/spec/rubocop/cop/style/redundant_begin_spec.rb b/spec/rubocop/cop/style/redundant_begin_spec.rb index ef0121e4cb2..b875420a85a 100644 --- a/spec/rubocop/cop/style/redundant_begin_spec.rb +++ b/spec/rubocop/cop/style/redundant_begin_spec.rb @@ -248,6 +248,23 @@ def method RUBY end + it 'registers and corrects an offense when using multi-line `if` in `begin` block' do + expect_offense(<<~RUBY) + var ||= begin + ^^^^^ Redundant `begin` block detected. + if condition + foo + end + end + RUBY + + expect_correction(<<~RUBY) + var ||= if condition + foo + end\n + RUBY + end + it 'does not register an offense when using `begin` with multiple statement for or assignment' do expect_no_offenses(<<~RUBY) var ||= begin