Skip to content

Commit

Permalink
Fix an infinite loop error for Style/NonNilCheck with `Style/NilCom…
Browse files Browse the repository at this point in the history
…parison`

This PR fixes the following infinite loop error for `IncludeSemanticChanges: false` of
`Style/NonNilCheck` with `EnforcedStyle: comparison` of `Style/NilComparison`.

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

!(foo == nil)
```

```yaml
Style/NilComparison:
  Enabled: true
  EnforcedStyle: comparison # alternative config
```

```console
% bundle exec rubocop -A
(snip)

Inspecting 1 file
C

Offenses:

example.rb:3:1: C: [Corrected] Style/InverseMethods: Use != instead of
inverting ==.
!(foo == nil)
^^^^^^^^^^^^^
example.rb:3:5: C: [Corrected] Style/NonNilCheck: Prefer
!expression.nil? over expression != nil.
foo != nil
    ^^
example.rb:3:6: C: [Corrected] Style/NilComparison: Prefer the use of
the == comparison.
!foo.nil?
     ^^^^

0 files inspected, 3 offenses detected, 3 offenses corrected
Infinite loop detected in
/Users/koic/src/github.com/koic/rubocop-issues/9387/example.rb and
caused by Style/InverseMethods -> Style/NonNilCheck -> Style/NilComparison
/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'
/Users/koic/src/github.com/rubocop-hq/rubocop/lib/rubocop/runner.rb:286:in `loop'
```

This PR makes `Style/NonNilCheck` allows `foo != nil` when using
`IncludeSemanticChanges: false` of `Style/NonNilCheck` and `EnforcedStyle: comparison` of
`Style/NilComparison` cop, then `Style/NonNilCheck` cop does not register an offense for
 `x != nil` and does no changes to `!x.nil?` style.
  • Loading branch information
koic committed Jan 18, 2021
1 parent 0381d1b commit 8dc27c1
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/fix_infinite_loop_error_for_non_nil_check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9389](https://github.com/rubocop-hq/rubocop/pull/9389): Fix an infinite loop error for `IncludeSemanticChanges: false` of `Style/NonNilCheck` with `EnforcedStyle: comparison` of `Style/NilComparison`. ([@koic][])
12 changes: 11 additions & 1 deletion lib/rubocop/cop/style/non_nil_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ module Style
# With `IncludeSemanticChanges` set to `false` by default, this cop
# does not report offenses for `!x.nil?` and does no changes that might
# change behavior.
# Also `IncludeSemanticChanges` set to `false` with `EnforcedStyle: comparison` of
# `Style/NilComparison` cop, this cop does not report offenses for `x != nil` and
# does no changes to `!x.nil?` style.
#
# With `IncludeSemanticChanges` set to `true`, this cop reports offenses
# for `!x.nil?` and autocorrects that and `x != nil` to solely `x`, which
Expand Down Expand Up @@ -49,7 +52,8 @@ class NonNilCheck < Base
def_node_matcher :not_and_nil_check?, '(send (send _ :nil?) :!)'

def on_send(node)
return if ignored_node?(node)
return if ignored_node?(node) ||
!include_semantic_changes? && nil_comparison_style == 'comparison'
return unless (offense_node = find_offense_node(node))

message = message(node)
Expand Down Expand Up @@ -138,6 +142,12 @@ def autocorrect_unless_nil(corrector, node, receiver)
corrector.replace(node.parent.loc.keyword, 'if')
corrector.replace(node, receiver.source)
end

def nil_comparison_style
nil_comparison_conf = config.for_cop('Style/NilComparison')

nil_comparison_conf['Enabled'] && nil_comparison_conf['EnforcedStyle']
end
end
end
end
Expand Down
31 changes: 31 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,37 @@ def self.some_method(foo, bar: 1)
RUBY
end

it 'corrects `Style/InverseMethods` offenses when specifying `IncludeSemanticChanges: false` of ' \
'`Style/NonNilCheck` and `EnforcedStyle: comparison` of `Style/NilComparison`' do
create_file('example.rb', <<~RUBY)
# frozen_string_literal: true
!(foo == nil)
RUBY

create_file('.rubocop.yml', <<~YAML)
Style/NilComparison:
Enabled: true
EnforcedStyle: comparison # alternative config
Style/NonNilCheck:
Enabled: true
IncludeSemanticChanges: false # default config
YAML

expect(cli.run([
'--auto-correct-all',
'--only',
'Style/InverseMethods,Style/NonNilCheck,Style/NilComparison'
])).to eq(0)
expect($stderr.string).to eq('')
expect(IO.read('example.rb')).to eq(<<~RUBY)
# frozen_string_literal: true
foo != nil
RUBY
end

it 'corrects Lint/ParenthesesAsGroupedExpression and offenses and ' \
'accepts Style/RedundantParentheses' do
create_file('example.rb', <<~RUBY)
Expand Down
39 changes: 39 additions & 0 deletions spec/rubocop/cop/style/non_nil_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,43 @@ def Test.signed_in?
RUBY
end
end

context 'when `EnforcedStyle: comparison` of `Style/NilComparison` cop' do
let(:other_cops) do
{
'Style/NilComparison' => { 'EnforcedStyle' => 'comparison' }
}
end

context '`IncludeSemanticChanges: false`' do
let(:cop_config) do
{
'IncludeSemanticChanges' => false
}
end

it 'does not register an offense for `foo != nil`' do
expect_no_offenses('foo != nil')
end
end

context '`IncludeSemanticChanges: true`' do
let(:cop_config) do
{
'IncludeSemanticChanges' => true
}
end

it 'does not register an offense for `foo != nil`' do
expect_offense(<<~RUBY)
foo != nil
^^ Prefer `!expression.nil?` over `expression != nil`.
RUBY

expect_correction(<<~RUBY)
foo
RUBY
end
end
end
end

0 comments on commit 8dc27c1

Please sign in to comment.