From d2c252962793a67c0a17d08b258a386e41005f27 Mon Sep 17 00:00:00 2001 From: Maxim Krizhanovski Date: Sun, 10 Jan 2021 01:20:29 +0200 Subject: [PATCH] [Fix #8281] Improve WhileUntilModifier detection and correction * Detect inline cases (while foo; bar; end) * Preserve comments when auto-correcting * Handle RHS usage in assignments --- ...fix_while_until_modifier_autocorrection.md | 1 + lib/rubocop/cop/style/while_until_modifier.rb | 6 +- .../cop/style/if_unless_modifier_spec.rb | 65 -------------- spec/support/condition_modifier_cop.rb | 89 +++++++++++++++++++ 4 files changed, 92 insertions(+), 69 deletions(-) create mode 100644 changelog/fix_while_until_modifier_autocorrection.md diff --git a/changelog/fix_while_until_modifier_autocorrection.md b/changelog/fix_while_until_modifier_autocorrection.md new file mode 100644 index 00000000000..2c8d70dd578 --- /dev/null +++ b/changelog/fix_while_until_modifier_autocorrection.md @@ -0,0 +1 @@ +* [#8281](https://github.com/rubocop-hq/rubocop/issues/8281): Fix Style/WhileUntilModifier handling comments and assignment when correcting to modifier form. ([@Darhazer][]) diff --git a/lib/rubocop/cop/style/while_until_modifier.rb b/lib/rubocop/cop/style/while_until_modifier.rb index e5442d9948d..11e161ec1e4 100644 --- a/lib/rubocop/cop/style/while_until_modifier.rb +++ b/lib/rubocop/cop/style/while_until_modifier.rb @@ -41,12 +41,10 @@ class WhileUntilModifier < Base 'having a single-line body.' def on_while(node) - return unless node.multiline? && single_line_as_modifier?(node) + return unless single_line_as_modifier?(node) add_offense(node.loc.keyword, message: format(MSG, keyword: node.keyword)) do |corrector| - oneline = "#{node.body.source} #{node.keyword} #{node.condition.source}" - - corrector.replace(node, oneline) + corrector.replace(node, to_modifier_form(node)) end end alias on_until on_while diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index b82e9095b4e..b668354d4df 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -357,71 +357,6 @@ def f RUBY end - it "doesn't break if-end when used as RHS of local var assignment" do - expect_offense(<<~RUBY) - a = if b - ^^ Favor modifier `if` usage when having a single-line body. [...] - 1 - end - RUBY - - expect_correction(<<~RUBY) - a = (1 if b) - RUBY - end - - it "doesn't break if-end when used as RHS of instance var assignment" do - expect_offense(<<~RUBY) - @a = if b - ^^ Favor modifier `if` usage when having a single-line body. [...] - 1 - end - RUBY - - expect_correction(<<~RUBY) - @a = (1 if b) - RUBY - end - - it "doesn't break if-end when used as RHS of class var assignment" do - expect_offense(<<~RUBY) - @@a = if b - ^^ Favor modifier `if` usage when having a single-line body. [...] - 1 - end - RUBY - - expect_correction(<<~RUBY) - @@a = (1 if b) - RUBY - end - - it "doesn't break if-end when used as RHS of constant assignment" do - expect_offense(<<~RUBY) - A = if b - ^^ Favor modifier `if` usage when having a single-line body. [...] - 1 - end - RUBY - - expect_correction(<<~RUBY) - A = (1 if b) - RUBY - end - - it "doesn't break if-end when used as RHS of binary arithmetic" do - expect_offense(<<~RUBY) - a + if b - ^^ Favor modifier `if` usage when having a single-line body. [...] - 1 - end - RUBY - - expect_correction(<<~RUBY) - a + (1 if b) - RUBY - end - it 'adds parens in autocorrect when if-end used with `||` operator' do expect_offense(<<~RUBY) a || if b diff --git a/spec/support/condition_modifier_cop.rb b/spec/support/condition_modifier_cop.rb index 1684e003931..cffe9bfa508 100644 --- a/spec/support/condition_modifier_cop.rb +++ b/spec/support/condition_modifier_cop.rb @@ -80,6 +80,95 @@ RUBY end + it "doesn't break when used as RHS of local var assignment" do + expect_offense(<<~RUBY, keyword: keyword) + a = %{keyword} b + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + 1 + end + RUBY + + expect_correction(<<~RUBY) + a = (1 #{keyword} b) + RUBY + end + + it "doesn't break when used as RHS of instance var assignment" do + expect_offense(<<~RUBY, keyword: keyword) + @a = %{keyword} b + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + 1 + end + RUBY + + expect_correction(<<~RUBY) + @a = (1 #{keyword} b) + RUBY + end + + it "doesn't break when used as RHS of class var assignment" do + expect_offense(<<~RUBY, keyword: keyword) + @@a = %{keyword} b + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + 1 + end + RUBY + + expect_correction(<<~RUBY) + @@a = (1 #{keyword} b) + RUBY + end + + it "doesn't break when used as RHS of constant assignment" do + expect_offense(<<~RUBY, keyword: keyword) + A = %{keyword} b + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + 1 + end + RUBY + + expect_correction(<<~RUBY) + A = (1 #{keyword} b) + RUBY + end + + it "doesn't break when used as RHS of binary arithmetic" do + expect_offense(<<~RUBY, keyword: keyword) + a + %{keyword} b + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + 1 + end + RUBY + + expect_correction(<<~RUBY) + a + (1 #{keyword} b) + RUBY + end + + it 'handles inline comments during autocorrection' do + expect_offense(<<~RUBY, keyword: keyword) + %{keyword} bar # important comment not to be nuked + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + baz + end + RUBY + + expect_correction(<<~RUBY) + baz #{keyword} bar # important comment not to be nuked + RUBY + end + + it 'handles one-line usage' do + expect_offense(<<~RUBY, keyword: keyword) + %{keyword} foo; bar; end + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + RUBY + + expect_correction(<<~RUBY) + bar #{keyword} foo + RUBY + end + # See: https://github.com/rubocop-hq/rubocop/issues/8273 context 'accepts multiline condition in modifier form' do it 'registers an offense' do