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 #8006] Fix infinite loop for Style/IfUnlessModifier #8026

Closed
wants to merge 1 commit into from
Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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][])
tejasbubane marked this conversation as resolved.
Show resolved Hide resolved

### Changes

Expand Down
7 changes: 4 additions & 3 deletions lib/rubocop/cop/mixin/statement_modifier.rb
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions spec/rubocop/cop/style/if_unless_modifier_spec.rb
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An infinite loop error is caused by a combination of cops. Con you add a reproduction test for infinite loop error to spec/rubocop/cli/cli_autocorrect_spec.rb? E.g.: https://github.com/rubocop-hq/rubocop/pull/7901/files#diff-0e7dbdd6e99e0defc2bab2cb544b42da
And I think the description of this test better to be updated to not register an offense.

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
Expand Down