Skip to content

Commit

Permalink
[Fix rubocop#376] Fix redunctant_allow_nil on different helpers false…
Browse files Browse the repository at this point in the history
… offense
  • Loading branch information
ngouy committed Nov 28, 2020
1 parent 7801ea7 commit c79003d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* [#371](https://github.com/rubocop-hq/rubocop-rails/pull/371): Fix an infinite loop error for `Rails/ActiveRecordCallbacksOrder` when callbacks have inline comments. ([@fatkodima][])
* [#364](https://github.com/rubocop-hq/rubocop-rails/pull/364): Fix a problem that `Rails/UniqueValidationWithoutIndex` doesn't work in classes defined with compact style. ([@sinsoku][])
* [#384](https://github.com/rubocop-hq/rubocop-rails/issues/384): Mark unsafe for `Rails/NegateInclude`. ([@koic][])
* [#394](https://github.com/rubocop-hq/rubocop-rails/pull/394): Fix false offense detection of `Rails/RedundantAllowNil` when using both allow_nil and allow_blank on different helpers of the same validator`. ([@ngouy][])

### Changes

Expand Down
21 changes: 11 additions & 10 deletions lib/rubocop/cop/rails/redundant_allow_nil.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,22 @@ def register_offense(allow_nil, message)
end

def find_allow_nil_and_allow_blank(node)
allow_nil = nil
allow_blank = nil
allow_nil, allow_blank = nil

node.each_descendant do |descendant|
next unless descendant.pair_type?
node.each_child_node do |child_node|
if child_node.pair_type?
key = child_node.children.first.source

key = descendant.children.first.source

allow_nil = descendant if key == 'allow_nil'
allow_blank = descendant if key == 'allow_blank'
allow_nil = child_node if key == 'allow_nil'
allow_blank = child_node if key == 'allow_blank'
end
return [allow_nil, allow_blank] if allow_nil && allow_blank

break if allow_nil && allow_blank
found_in_children_nodes = find_allow_nil_and_allow_blank(child_node)
return found_in_children_nodes if found_in_children_nodes
end

[allow_nil, allow_blank]
nil
end

def previous_sibling(node)
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/rails/redundant_allow_nil_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,25 @@
RUBY
end
end

context 'when using allow_nil and allow_blank on different helpers' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
validates :email, format: { with: /regexp/, allow_blank: true }, presence: { allow_nil: true }
RUBY
end
end

context 'when using allow_nil and allow_blank true on the same helper' do
it 'registers no offense' do
expect_offense(<<~RUBY)
validates :email, format: { with: /regexp/, allow_nil: true, allow_blank: true }
^^^^^^^^^^^^^^^ `allow_nil` is redundant when `allow_blank` has the same value.
RUBY

expect_correction(<<~RUBY)
validates :email, format: { with: /regexp/, allow_blank: true }
RUBY
end
end
end

0 comments on commit c79003d

Please sign in to comment.