Skip to content

Commit

Permalink
[Fix #9453] Fix an infinite loop error for `Layout/FirstParameterInde…
Browse files Browse the repository at this point in the history
…ntation`

Fixes #9453.

This PR fixes the following infinite loop error for `Layout/FirstParameterIndentation` when
`EnforcedStyle: with_fixed_indentation` is specified for `Layout/ArgumentAlignment`.

```console
% cat example.rb
# frozen_string_literal: true

RSpec.describe MyController, type: :controller do
  it 'abc' do
    expect(response).to redirect_to(path(
      obj1,
      id: obj2.id
    ))
  end
end

% cat .rubocop.yml
AllCops:
NewCops: enable

Layout/ArgumentAlignment:
  EnforcedStyle: with_fixed_indentation

% bundle exec rubocop -a --only Layout/ArgumentAlignment,Layout/FirstArgumentIndentation
(snip)

Infinite loop detected in
/Users/koic/src/github.com/koic/rubocop-issues/9453/example.rb and
caused by Layout/FirstArgumentIndentation -> Layout/ArgumentAlignment
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/runner.rb:304:in
`check_for_infinite_loop'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/runner.rb:287:in
`block in iterate_until_no_changes'
```

The following is the reason why an error occurs.

First, `Layout/FirstArgumentIndentation` will auto-correct it.

```console
% bundle exec rubocop -a --only Layout/FirstArgumentIndentation
Inspecting 1 file
C

Offenses:

example.rb:6:7: C: [Corrected] Layout/FirstArgumentIndentation: Indent
the first argument one step more than path(.
obj1,
^^^^

1 file inspected, 1 offense detected, 1 offense corrected

% cat example.rb
# frozen_string_literal: true

RSpec.describe MyController, type: :controller do
  it 'abc' do
    expect(response).to redirect_to(path(
                                      obj1,
      id: obj2.id
    ))
  end
end
```

Next, `Layout/ArgumentAlignment` will auto-correct it.

```consloe
% bundle exec rubocop -a --only Layout/ArgumentAlignment
Inspecting 1 file
C

Offenses:

example.rb:6:39: C: [Corrected] Layout/ArgumentAlignment: Use one level
of indentation for arguments following the first line of a multi-line
method call.
obj1,
^^^^

1 file inspected, 1 offense detected, 1 offense corrected

% cat example.rb
# frozen_string_literal: true

RSpec.describe MyController, type: :controller do
  it 'abc' do
    expect(response).to redirect_to(path(
      obj1,
      id: obj2.id
    ))
  end
end
```

An infinite loop error occurs because it is corrected to the original code.

This PR makes `Layout/FirstParameterIndentation` respect `Layout/ArgumentAlignment`
when `EnforcedStyle: with_fixed_indentation` alternative configuration is specified
for `Layout/ArgumentAlignment` to prevent the error.
  • Loading branch information
koic authored and bbatsov committed Feb 1, 2021
1 parent 2c2a8e3 commit 72c94e9
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9453](https://github.com/rubocop-hq/rubocop/issues/9453): Fix infinite loop error for `Layout/FirstParameterIndentation` when `EnforcedStyle: with_fixed_indentation` is specified for `Layout/ArgumentAlignment`. ([@koic][])
18 changes: 16 additions & 2 deletions lib/rubocop/cop/layout/first_argument_indentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ module RuboCop
module Cop
module Layout
# This cop checks the indentation of the first argument in a method call.
# Arguments after the first one are checked by Layout/ArgumentAlignment,
# Arguments after the first one are checked by `Layout/ArgumentAlignment`,
# not by this cop.
#
# For indenting the first parameter of method _definitions_, check out
# Layout/FirstParameterIndentation.
# `Layout/FirstParameterIndentation`.
#
# This cop will respect `Layout/ArgumentAlignment` and will not work when
# `EnforcedStyle: with_fixed_indentation` is specified for `Layout/ArgumentAlignment`.
#
# @example
#
Expand Down Expand Up @@ -149,6 +152,7 @@ class FirstArgumentIndentation < Cop
MSG = 'Indent the first argument one step more than %<base>s.'

def on_send(node)
return if enforce_first_argument_with_fixed_indentation?
return if !node.arguments? || node.operator_method?

indent = base_indentation(node) + configured_indentation_width
Expand Down Expand Up @@ -250,6 +254,16 @@ def comment_lines
def on_new_investigation
@comment_lines = nil
end

def enforce_first_argument_with_fixed_indentation?
return false unless argument_alignment_config['Enabled']

argument_alignment_config['EnforcedStyle'] == 'with_fixed_indentation'
end

def argument_alignment_config
config.for_cop('Layout/ArgumentAlignment')
end
end
end
end
Expand Down
32 changes: 32 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1688,6 +1688,38 @@ def self.some_method(foo, bar: 1)
RUBY
end

it 'does not crash `Layout/ArgumentAlignment` and offenses and accepts `Layout/FirstArgumentIndentation` ' \
'when specifying `EnforcedStyle: with_fixed_indentation` of `Layout/ArgumentAlignment`' do
create_file('example.rb', <<~RUBY)
# frozen_string_literal: true
expect(response).to redirect_to(path(
obj1,
id: obj2.id
))
RUBY

create_file('.rubocop.yml', <<~YAML)
Layout/ArgumentAlignment:
EnforcedStyle: with_fixed_indentation
YAML

expect(cli.run([
'--auto-correct',
'--only',
'Layout/ArgumentAlignment,Layout/FirstArgumentIndentation'
])).to eq(0)
expect($stderr.string).to eq('')
expect(IO.read('example.rb')).to eq(<<~RUBY)
# frozen_string_literal: true
expect(response).to redirect_to(path(
obj1,
id: obj2.id
))
RUBY
end

it 'does not crash Lint/SafeNavigationWithEmpty and offenses and accepts Style/SafeNavigation ' \
'when checking `foo&.empty?` in a conditional' do
create_file('example.rb', <<~RUBY)
Expand Down

0 comments on commit 72c94e9

Please sign in to comment.