From 52bfe3a3c9976427889710ada2f66835ac2b040b Mon Sep 17 00:00:00 2001 From: Tejas Bubane Date: Sun, 24 May 2020 23:57:58 +0530 Subject: [PATCH] Fix infinite loop for `Style/IfUnlessModifier` Closes #8006 & #8025 --- CHANGELOG.md | 1 + lib/rubocop/cop/mixin/statement_modifier.rb | 7 ++++--- .../cop/style/if_unless_modifier_spec.rb | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e09889306..7b5e7127145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * [#8035](https://github.com/rubocop-hq/rubocop/issues/8035): Fix a false positive for `Lint/DeprecatedOpenSSLConstant` when using double quoted string argument. ([@koic][]) * [#7971](https://github.com/rubocop-hq/rubocop/issues/7971): Fix an issue where `--disable-uncorrectable` would not update uncorrected code with `rubocop:todo`. ([@rrosenblum][]) * [#8035](https://github.com/rubocop-hq/rubocop/issues/8035): Fix a false positive for `Lint/DeprecatedOpenSSLConstant` when argument is a variable, method, or consntant. ([@koic][]) +* [#8006](https://github.com/rubocop-hq/rubocop/issues/8006): Fix infinite loop for `Style/IfUnlessModifier`. ([@tejasbubane][]) ### Changes diff --git a/lib/rubocop/cop/mixin/statement_modifier.rb b/lib/rubocop/cop/mixin/statement_modifier.rb index 88ea52a0334..3bea60ebe7c 100644 --- a/lib/rubocop/cop/mixin/statement_modifier.rb +++ b/lib/rubocop/cop/mixin/statement_modifier.rb @@ -41,9 +41,10 @@ def modifier_fits_on_single_line?(node) def length_in_modifier_form(node, cond) keyword = node.loc.keyword - indentation = keyword.source_line[/^\s*/] - line_length("#{indentation}#{node.body.source} #{keyword.source} " \ - "#{cond.source}") + line_before_condition, = keyword.source_line.split(keyword.source) + condition_source = "#{node.body.source} #{keyword.source} #{cond.source}" + source = line_before_condition + condition_source + line_length(source) end def max_line_length diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index 848dbd73554..01f7b2302d8 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -421,6 +421,26 @@ def f expect(corrected).to eq "a = (1 if b)\n" end + context 'when the code before condition pushes inline length over limit' do + let(:line_length_config) { { 'Enabled' => true, 'Max' => 100 } } + + it 'does not result in infinite loop with assignment' do + expect_no_offenses(<<~RUBY) + timespan = if params[:timespan] && POSSIBLE_TIMESPANS.include?(params[:timespan]) + params[:timespan].to_sym + end + RUBY + end + + it 'does not result in infinite loop with any other statement' do + expect_no_offenses(<<~RUBY) + puts 'ab'; if params[:timespan] && POSSIBLE_TIMESPANS.include?(params[:timespan]) + params[:timespan].to_sym + end + RUBY + end + end + it "doesn't break if-end when used as RHS of instance var assignment" do corrected = autocorrect_source(<<~RUBY) @a = if b