Skip to content

Commit

Permalink
Fix an infinite loop error for Layout/SpaceInsideBlockBraces
Browse files Browse the repository at this point in the history
This PR fixes an infinite loop error for `Layout/SpaceInsideBlockBraces`
when`EnforcedStyle: no_space` with `SpaceBeforeBlockParameters: false`
are set in multiline block.

```yaml
# .rubocop.yml
AllCops:
  DisabledByDefault: true

Layout/SpaceInsideBlockBraces:
  Enabled: true
  EnforcedStyle: no_space
  SpaceBeforeBlockParameters: false
```

```ruby
# example.rb
items.map {|item|
  item.do_something
}
```

```console
% rubocop -a --only Layout/SpaceInsideBlockBraces
Inspecting 1 file
C

Offenses:

example.rb:2:20: C: [Corrected] Layout/SpaceInsideBlockBraces: Space
inside } detected.
  item.do_something ...

0 files inspected, 1 offense detected, 1 offense corrected
Infinite loop detected in /private/tmp/foo/example.rb.
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:268:in `check_for_infinite_loop'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:251:in `block in iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:250:in `loop'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:250:in `iterate_until_no_changes'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:221:in `do_inspection_loop'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:124:in `block in file_offenses'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:142:in `file_offense_cache'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:122:in `file_offenses'
/Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.72.0/lib/rubocop/runner.rb:110:in `process_file'

(snip)
```

Furthermore, it is changed by auto-correction to the following
unnatural code.

```diff
diff --git a/example.rb b/example.rb
index f1427be..fb547ee 100644
--- a/example.rb
+++ b/example.rb
@@ -1,3 +1,2 @@
 items.map {|item|
 -  item.do_something
 -}
 +  item.do_something}
```

This PR prevents the error by noticing right brace of multiline block.
  • Loading branch information
koic authored and bbatsov committed Jul 15, 2019
1 parent 3c70aad commit e360c5d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
* [#7099](https://github.com/rubocop-hq/rubocop/issues/7099): Fix an error of `Layout/RescueEnsureAlignment` on assigned blocks. ([@tatsuyafw][])
* [#5088](https://github.com/rubocop-hq/rubocop/issues/5088): Fix an error of `Layout/MultilineMethodCallIndentation` on method chains inside an argument. ([@buehmann][])
* [#4719](https://github.com/rubocop-hq/rubocop/issues/4719): Make `Layout/Tab` detect tabs between string literals. ([@buehmann][])
* [#7203](https://github.com/rubocop-hq/rubocop/pull/7203): Fix an infinite loop error for `Layout/SpaceInsideBlockBraces` when `EnforcedStyle: no_space` with `SpaceBeforeBlockParameters: false` are set in multiline block. ([@koic][])

### Changes

Expand Down
16 changes: 14 additions & 2 deletions lib/rubocop/cop/layout/space_inside_block_braces.rb
Expand Up @@ -130,7 +130,8 @@ def braces_with_contents_inside(node, inner)
args_delimiter = node.arguments.loc.begin # Can be ( | or nil.

check_left_brace(inner, node.loc.begin, args_delimiter)
check_right_brace(inner, node.loc.end, node.single_line?)
check_right_brace(inner, node.loc.begin, node.loc.end,
node.single_line?)
end

def check_left_brace(inner, left_brace, args_delimiter)
Expand All @@ -141,15 +142,26 @@ def check_left_brace(inner, left_brace, args_delimiter)
end
end

def check_right_brace(inner, right_brace, single_line)
def check_right_brace(inner, left_brace, right_brace, single_line)
if single_line && inner =~ /\S$/
no_space(right_brace.begin_pos, right_brace.end_pos,
'Space missing inside }.')
else
return if multiline_block?(left_brace, right_brace) &&
aligned_braces?(left_brace, right_brace)

space_inside_right_brace(right_brace)
end
end

def multiline_block?(left_brace, right_brace)
left_brace.first_line != right_brace.first_line
end

def aligned_braces?(left_brace, right_brace)
left_brace.first_line == right_brace.last_column
end

def no_space_inside_left_brace(left_brace, args_delimiter)
if pipe?(args_delimiter)
if left_brace.end_pos == args_delimiter.begin_pos &&
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/layout/space_inside_block_braces_spec.rb
Expand Up @@ -329,6 +329,27 @@
expect_no_offenses('->(x) {x}')
end

it 'does not register an offense when braces are aligned in ' \
'multiline block' do
expect_no_offenses(<<~RUBY)
items.map {|item|
item.do_something
}
RUBY
end

it 'registers an offense when braces are not aligned in ' \
'multiline block' do
inspect_source(<<~RUBY)
items.map {|item|
item.do_something
}
RUBY

expect(cop.offenses.size).to eq(1)
expect(cop.messages).to eq(['Space inside } detected.'])
end

it 'auto-corrects unwanted space' do
new_source = autocorrect_source('each { |x| puts}')
expect(new_source).to eq('each {|x| puts}')
Expand Down

0 comments on commit e360c5d

Please sign in to comment.