Skip to content

Commit

Permalink
[Fix #8281] Improve WhileUntilModifier detection and correction
Browse files Browse the repository at this point in the history
* Detect inline cases (while foo; bar; end)
* Preserve comments when auto-correcting
* Handle RHS usage in assignments
  • Loading branch information
Darhazer authored and bbatsov committed Jan 11, 2021
1 parent dd54974 commit d2c2529
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 69 deletions.
1 change: 1 addition & 0 deletions 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][])
6 changes: 2 additions & 4 deletions lib/rubocop/cop/style/while_until_modifier.rb
Expand Up @@ -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
Expand Down
65 changes: 0 additions & 65 deletions spec/rubocop/cop/style/if_unless_modifier_spec.rb
Expand Up @@ -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
Expand Down
89 changes: 89 additions & 0 deletions spec/support/condition_modifier_cop.rb
Expand Up @@ -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
Expand Down

0 comments on commit d2c2529

Please sign in to comment.