ExtraSpacing Infinite Correction Loop with AlignHash

[jfelchner]

Currently it appears that ExtraSpacing conflicts (possibly?) with the AlignHash cop's autocorrection. It seems as though ExtraSpacing is trying to correct spacing on hash values which I believe it should be prohibited from doing since that's the job of the AlignHash cop.

When I attempt to run it, I get:

/usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:126:in `check_for_infinite_loop': Infinite loop detected in /Users/jfelchner/Projects/my_project/my_file.rb. (RuboCop::Runner::InfiniteCorrectionLoop)
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:101:in `block in do_inspection_loop'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `loop'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `do_inspection_loop'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:78:in `process_file'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:53:in `block in inspect_files'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `each'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `inspect_files'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:30:in `run'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/cli.rb:26:in `run'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:13:in `block in <top (required)>'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/2.2.0/benchmark.rb:303:in `realtime'
        from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:12:in `<top (required)>'
        from /usr/local/var/rbenv/versions/2.2.2/bin/rubocop:23:in `load'
        from /usr/local/var/rbenv/versions/2.2.2/bin/rubocop:23:in `<main>'

@jonas054 I'll attach a hash that will repro this issue in a couple hours.

[jonas054]

@jfelchner Do have that hash for reproducing? It would help a lot.

[Pilooz]

Hi,
It seems that rubocop doesn't like empty begin rescue endblock ?
This code produce an error in rubocop verification. Here is the test case :
test.rb

begin
  # Some comment
rescue => e
  puts e.response
end

here is rubocop response :

$ rubocop -a test.rb 
Inspecting 1 file
C

Offenses:

test.rb:3:1: C: [Corrected] Use 2 (not 0) spaces for indentation.
rescue => e

test.rb:3:3: C: [Corrected] rescue at 3, 2 is not aligned with end at 5, 0.
  rescue => e
  ^^^^^^
test.rb:4:3: C: [Corrected] Use 2 (not 4) spaces for indentation.
      puts e.response
  ^^^^

0 files inspected, 3 offenses detected, 3 offenses corrected
/usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:126:in `check_for_infinite_loop': Infinite loop detected in /Users/pgl/developpement/test.rb. (RuboCop::Runner::InfiniteCorrectionLoop)
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:101:in `block in do_inspection_loop'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `loop'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:100:in `do_inspection_loop'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:78:in `process_file'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:53:in `block in inspect_files'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `each'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:51:in `inspect_files'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/runner.rb:30:in `run'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/lib/rubocop/cli.rb:26:in `run'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:13:in `block in <top (required)>'
    from /usr/local/Cellar/ruby/2.2.1/lib/ruby/2.2.0/benchmark.rb:303:in `realtime'
    from /usr/local/lib/ruby/gems/2.2.0/gems/rubocop-0.33.0/bin/rubocop:12:in `<top (required)>'
    from /usr/local/bin/rubocop:23:in `load'
    from /usr/local/bin/rubocop:23:in `<main>'
$ 

And when I re-run rubocop on 'corrected file', I obtain the same error.

[jonas054]

@Pilooz Thanks, but that's a different error, not related to ExtraSpacing. Please open a new issue.

[lackac]

Here's an example that reproduces this issue:

# .rubocop.yml
Style/AlignHash:
  EnforcedHashRocketStyle: table
  EnforcedColonStyle: table

The offending ruby file:

hash = {
  alice: {
    age:  23,
    role: 'Director'
  },
  bob:   {
    age:  25,
    role: 'Consultant'
  }
}
p hash

Output of RuboCop:

Inspecting 1 file
C

Offenses:

hash.rb:6:3: C: [Corrected] Align the elements of a hash literal if they span more than one line.
  bob: {
  ^^^^^^
hash.rb:6:7: C: [Corrected] Unnecessary spacing detected.
  bob:   {
      ^^

0 files inspected, 2 offenses detected, 2 offenses corrected
/Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:161:in `check_for_infinite_loop': Infinite loop detected in /Users/LacKac/Code/rubocop-hash-with-space/hash.rb. (RuboCop::Runner::InfiniteCorrectionLoop)
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:136:in `block in do_inspection_loop'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:135:in `loop'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:135:in `do_inspection_loop'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:87:in `process_file'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:57:in `block in inspect_files'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:55:in `each'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:55:in `inspect_files'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/runner.rb:33:in `run'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/lib/rubocop/cli.rb:26:in `run'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/bin/rubocop:13:in `block in <top (required)>'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/2.2.0/benchmark.rb:303:in `realtime'
        from /Users/LacKac/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/rubocop-0.34.0/bin/rubocop:12:in `<top (required)>'
        from /Users/LacKac/.rbenv/versions/2.2.1/bin/rubocop:23:in `load'
        from /Users/LacKac/.rbenv/versions/2.2.1/bin/rubocop:23:in `<main>'

[lackac]

To resolve the issue I propose adding an exception for AlignHash when the keys are not on adjacent lines.

[jfelchner]

@lackac thank you for finding a has for @jonas054. I've been incredibly busy.

However IMO making AlignHash skip non-adjacent keys would not be the correct solution.

[lackac]

How do you think it should be handled?

It seems to me that currently the two conflicting cops have different ideas about alignment. AlignHash would still enforce alignment when there are other lines in-between, even if those lines are not empty or only include a comment. At the same time ExtraSpacing only allows extra spaces with the AllowForAlignment option if the only lines in-between the two aligned ones are empty or comment lines.

[jonas054]

I think we can just let ExtraSpacing stay away from hash literals altogether. Let AlignHash deal with them, since it already handles some of the same concerns (extra spacing before the value, depending on configuration). I realize that there might be a few offenses we miss if we do it that way, but it's a pretty simple solution.

[lackac]

That would certainly solve the conflict at hand. However, I think it's still worth considering my proposal for AlignHash not enforcing alignment for keys that have non-empty lines between them.

What would be the benefit of aligning values of top-level keys in the hash example above? It's even more of a pain to fix alignment as the value hashes become bigger.

[jonas054]

@lackac There's no benefit in your example above, but it's easy to tweak the example so aligning the values looks better than not doing so:

hash = {
  alice: { age:  23,
           role: 'Director' },
  bob:   { age:  25,
           role: 'Consultant' }
}

So I prefer to let AlignHash be as it is now, and fix the problem in ExtraSpacing.