Skip to content

Commit

Permalink
Merge pull request #9390 from koic/fix_infinite_loop_error_for_non_ni…
Browse files Browse the repository at this point in the history
…l_check

Fix an infinite loop error for `Style/NonNilCheck` with `Style/NilComparison`
  • Loading branch information
koic authored Jan 19, 2021
2 parents 0381d1b + 8dc27c1 commit 70c62cc
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 70c62cc

Please sign in to comment.