rubocop / rubocop

A Ruby static code analyzer and formatter, based on the community Ruby style guide.
https://docs.rubocop.org
MIT License
12.66k stars 3.07k forks source link

Layout/IndentationStyle EnforcedStyle tabs leads to an infinite loop unless IndentationWidth is set to 1 #9373

Open joeyparis opened 3 years ago

joeyparis commented 3 years ago

Using just:

Layout/IndentationStyle:
  EnforcedStyle: tabs

instead of:

Layout/IndentationStyle:
  IndentationWidth: 1
  EnforcedStyle: tabs

leads to an infinite loop detected error.

Second, but related bug Setting the IndentationWidth to 1 actually makes RuboCop indent 2 tabs instead of 1. I tried setting it to 0 but that does nothing instead. I figure these two bugs are closely related so it's not worth creating a second ticket for. If that's not the case, just let me know and I will create a separate ticket for that bug.


Expected behavior

Rubocop automatically indents lines with tabs just as it would with spaces

Actual behavior

Run:

rubocop --auto-correct --debug

Output:

...
0 files inspected, 41 offenses detected, 40 offenses corrected
Infinite loop detected in /Users/joey/Sites/AcquireApp/backend/Gemfile and caused by Layout/CommentIndentation, Layout/IndentationConsistency, Layout/IndentationStyle, Layout/IndentationWidth -> Layout/CommentIndentation, Layout/IndentationConsistency, Layout/IndentationStyle, Layout/IndentationWidth -> Layout/CommentIndentation, Layout/IndentationConsistency, Layout/IndentationStyle, Layout/IndentationWidth, Layout/LineLength
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:304:in `check_for_infinite_loop'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:287:in `block in iterate_until_no_changes'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:286:in `loop'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:286:in `iterate_until_no_changes'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:255:in `do_inspection_loop'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:132:in `block in file_offenses'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:157:in `file_offense_cache'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:131:in `file_offenses'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:122:in `process_file'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:101:in `block in each_inspected_file'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:100:in `each'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:100:in `reduce'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:100:in `each_inspected_file'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:86:in `inspect_files'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/runner.rb:47:in `run'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/command/execute_runner.rb:26:in `block in execute_runner'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/command/execute_runner.rb:52:in `with_redirect'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/command/execute_runner.rb:25:in `execute_runner'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/command/execute_runner.rb:17:in `run'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/command.rb:11:in `run'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli/environment.rb:18:in `run'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli.rb:65:in `run_command'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli.rb:72:in `execute_runners'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/lib/rubocop/cli.rb:41:in `run'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/exe/rubocop:13:in `block in <top (required)>'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/2.7.0/benchmark.rb:308:in `realtime'
/Users/joey/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rubocop-1.8.1/exe/rubocop:12:in `<top (required)>'
/Users/joey/.rbenv/versions/2.7.1/bin/rubocop:23:in `load'
/Users/joey/.rbenv/versions/2.7.1/bin/rubocop:23:in `<main>'
Finished in 0.6281139999991865 seconds

Steps to reproduce the problem

Add the following to .rubocopy.yml

Layout/IndentationStyle:
  EnforcedStyle: tabs

RuboCop version

$ bundle exec rubocop -V
warning: parser/current is loading parser/ruby27, which recognizes
warning: 2.7.2-compliant syntax, but you are running 2.7.1.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
1.8.1 (using Parser 3.0.0.0, rubocop-ast 1.4.0, running on ruby 2.7.1 x86_64-darwin20)
  - rubocop-rails 2.9.1

Potential Solutions

joeyparis commented 3 years ago

Pinging @DracoAter as he originally created the IndentationStyle cop and probably knows the most about how it works.

P.S. As a fellow stubborn-tab-user I really appreciate you creating this cop because it's what finally made me happy using rubocop in my projects

joeyparis commented 3 years ago

Upon further debugging, I was able to get the desired behavior. Although this may be a "bug" I think it could sufficiently be corrected with improved documentation.

If Layout/IndentationStyle -> EnforcedStyle is set to tabs, then Layout/IndentationStyle -> IndentationWidth MUST be set to 1. To avoid double indentation it is also recommended to set Layout/IndentationWidth -> Width to 1.

Alternatively, a fix for this bug would be to change line 85 to "\t" * match.size and making not that IndentationWidth is ignored when EnforcedStyle is tabs.

https://github.com/rubocop-hq/rubocop/blob/3dc4e6298e6caa96fcc11a6d2a614ce6a2d4c44d/lib/rubocop/cop/layout/indentation_style.rb#L82-L88

I still think adding a note about setting LayoutIndentation/Width to 1 would be helpful. I'll make a pull request now for these changes.

amomchilov commented 1 year ago

It looks to me like the Layout/IndentationWidth cop is redundant to the IndentationWidth parameter of Layout/IndentationStyle cop. Would it make sense to deprecate one and standardize on the other?

There are some places that check both:

https://github.com/rubocop/rubocop/blob/689d473bccf3cef525a53f51820ca34fc191455d/lib/rubocop/cop/mixin/line_length_help.rb#L88-L91

It would be really weird of the two values differed from each other. We should just standardize on one.

akirataguchi115 commented 1 year ago

Having this same issue with relaxed.ruby.style + Layout/IndentationStyle: EnforcedStyle: tabs