Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an incorrect auto-correct for Style/IfUnlessModifier #9690

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1 @@
* [#9690](https://github.com/rubocop/rubocop/pull/9690): Fix an incorrect auto-correct for `Style/IfUnlessModifier` when using a method with heredoc argument. ([@koic][])
40 changes: 37 additions & 3 deletions lib/rubocop/cop/style/if_unless_modifier.rb
Expand Up @@ -38,6 +38,7 @@ class IfUnlessModifier < Base
include StatementModifier
include LineLengthHelp
include IgnoredPattern
include RangeHelp
extend AutoCorrector

MSG_USE_MODIFIER = 'Favor modifier `%<keyword>s` usage when having a ' \
Expand Down Expand Up @@ -66,7 +67,16 @@ def on_if(node)

def autocorrect(corrector, node)
replacement = if node.modifier_form?
to_normal_form(node)
indentation = ' ' * node.source_range.column
last_argument = node.if_branch.last_argument

if last_argument.respond_to?(:heredoc?) && last_argument.heredoc?
heredoc = extract_heredoc_from(last_argument)
remove_heredoc(corrector, heredoc)
to_normal_form_with_heredoc(node, indentation, heredoc)
else
to_normal_form(node, indentation)
end
else
to_modifier_form(node)
end
Expand Down Expand Up @@ -151,14 +161,38 @@ def another_statement_on_same_line?(node)
node && (sibling = node.children[index + 1]) && sibling.source_range.first_line == line_no
end

def to_normal_form(node)
indentation = ' ' * node.source_range.column
def to_normal_form(node, indentation)
<<~RUBY.chomp
#{node.keyword} #{node.condition.source}
#{indentation} #{node.body.source}
#{indentation}end
RUBY
end

def to_normal_form_with_heredoc(node, indentation, heredoc)
heredoc_body, heredoc_end = heredoc

<<~RUBY.chomp
#{node.keyword} #{node.condition.source}
#{indentation} #{node.body.source}
#{indentation} #{heredoc_body.source.chomp}
#{indentation} #{heredoc_end.source.chomp}
#{indentation}end
RUBY
end

def extract_heredoc_from(last_argument)
heredoc_body = last_argument.loc.heredoc_body
heredoc_end = last_argument.loc.heredoc_end

[heredoc_body, heredoc_end]
end

def remove_heredoc(corrector, heredoc)
heredoc.each do |range|
corrector.remove(range_by_whole_lines(range, include_final_newline: true))
end
end
end
end
end
Expand Down
19 changes: 19 additions & 0 deletions spec/rubocop/cop/style/if_unless_modifier_spec.rb
Expand Up @@ -73,6 +73,25 @@ def f
end
end

context 'when using a method with heredoc argument' do
it 'accepts' do
expect_offense(<<~RUBY)
fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(<<~EOS) if condition
^^ Modifier form of `if` makes the line too long.
string
EOS
RUBY

expect_correction(<<~RUBY)
if condition
fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo(<<~EOS)
string
EOS
end
RUBY
end
end

describe 'IgnoreCopDirectives' do
let(:spaces) { ' ' * 57 }
let(:comment) { '# rubocop:disable Style/For' }
Expand Down