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 #7899] Fix an infinite loop error for Layout/SpaceAroundOperators #7901

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* [#7635](https://github.com/rubocop-hq/rubocop/issues/7635): Fix a false positive for `Style/MultilineWhenThen` when `then` required for a body of `when` is used. ([@koic][])
* [#7905](https://github.com/rubocop-hq/rubocop/pull/7905): Fix an error when running `rubocop --only` or `rubocop --except` options without cop name argument. ([@koic][])
* [#7903](https://github.com/rubocop-hq/rubocop/pull/7903): Fix an incorrect autocorrect for `Style/HashTransformKeys` and `Style/HashTransformValues` cops when line break before `to_h` method. ([@diogoosorio][], [@koic][])
* [#7899](https://github.com/rubocop-hq/rubocop/issues/7899): Fix an infinite loop error for `Layout/SpaceAroundOperators` with `Layout/ExtraSpacing` when using `ForceEqualSignAlignment: true`. ([@koic][])

### Changes

Expand Down
19 changes: 18 additions & 1 deletion lib/rubocop/cop/layout/space_around_operators.rb
Expand Up @@ -139,7 +139,7 @@ def autocorrect(range)
elsif range.source.end_with?("\n")
corrector.replace(range, " #{range.source.strip}\n")
else
corrector.replace(range, " #{range.source.strip} ")
enclose_operator_with_space(corrector, range)
end
end
end
Expand Down Expand Up @@ -170,6 +170,19 @@ def offense(type, operator, with_space, right_operand)
yield msg if msg
end

def enclose_operator_with_space(corrector, range)
operator = range.source

# If `ForceEqualSignAlignment` is true, `Layout/ExtraSpacing` cop
# inserts spaces before operator. If `Layout/SpaceAroundOperators` cop
# inserts a space, it collides and raises the infinite loop error.
if force_equal_sign_alignment?
corrector.insert_after(range, ' ') unless operator.end_with?(' ')
else
corrector.replace(range, " #{operator.strip} ")
end
end

def offense_message(type, operator, with_space, right_operand)
if should_not_have_surrounding_space?(operator)
return if with_space.is?(operator.source)
Expand Down Expand Up @@ -216,6 +229,10 @@ def space_around_exponent_operator?
cop_config['EnforcedStyleForExponentOperator'] == 'space'
end

def force_equal_sign_alignment?
config.for_cop('Layout/ExtraSpacing')['ForceEqualSignAlignment']
end

def should_not_have_surrounding_space?(operator)
operator.is?('**') ? !space_around_exponent_operator? : false
end
Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Expand Up @@ -106,6 +106,30 @@ def batch
RUBY
end

it 'corrects `Layout/SpaceAroundOperators` and `Layout/ExtraSpacing` ' \
'offenses when using `ForceEqualSignAlignment: true`' do
create_file('.rubocop.yml', <<~YAML)
Layout/ExtraSpacing:
ForceEqualSignAlignment: true
YAML

create_file('example.rb', <<~RUBY)
test123456 = nil
test1234 = nil
test1_test2_test3_test4_12 =nil
RUBY

expect(cli.run(['--auto-correct'])).to eq(1)

expect(IO.read('example.rb')).to eq(<<~RUBY)
# frozen_string_literal: true

test123456 = nil
test1234 = nil
test1_test2_test3_test4_12 = nil
RUBY
end

it 'does not correct SpaceAroundOperators in a hash that would be ' \
'changed back' do
create_file('.rubocop.yml', <<~YAML)
Expand Down
1 change: 0 additions & 1 deletion spec/rubocop/cop/layout/space_around_operators_spec.rb
Expand Up @@ -6,7 +6,6 @@
let(:config) do
RuboCop::Config
.new(
'Layout/ExtraSpacing' => { 'ForceEqualSignAlignment' => true },
'Layout/HashAlignment' => { 'EnforcedHashRocketStyle' => hash_style },
'Layout/SpaceAroundOperators' => {
'AllowForAlignment' => allow_for_alignment,
Expand Down