Skip to content

Commit

Permalink
[Fix rubocop#53] Fix a false positive for Rails/SaveBang
Browse files Browse the repository at this point in the history
Fixes rubocop#53.

This PR fixes a false positive for `Rails/SaveBang` when implicitly
return using finder method and creation method connected by `||`.

And this PR updates .rubocop_todo.yml with `rubocop --auto_gen_config`
to prevent the following offense.

```console
% rubocop lib/rubocop/cop/rails/save_bang.rb
Inspecting 1 file
C

Offenses:

lib/rubocop/cop/rails/save_bang.rb:100:7: C: Metrics/ClassLength: Class
has too many lines. [164/159]
      class SaveBang < Cop ...
            ^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```
  • Loading branch information
koic committed Jul 20, 2019
1 parent 69118b1 commit f2aa820
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 5 deletions.
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-07-20 09:04:51 +0900 using RuboCop version 0.72.0.
# on 2019-07-20 17:51:45 +0900 using RuboCop version 0.72.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand All @@ -21,7 +21,7 @@ Metrics/AbcSize:
# Offense count: 4
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 159
Max: 164

# Offense count: 23
# Configuration parameters: CountComments, ExcludedMethods.
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,8 @@

## master (unreleased)

* [#53](https://github.com/rubocop-hq/rubocop-rails/issues/53): Fix a false positive for `Rails/SaveBang` when implicitly return using finder method and creation method connected by `||`. ([@koic][])

## 2.2.1 (2019-07-13)

### Bug fixes
Expand Down
14 changes: 11 additions & 3 deletions lib/rubocop/cop/rails/save_bang.rb
Expand Up @@ -140,9 +140,9 @@ def check_assignment(assignment)
def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity
return unless persist_method?(node)
return if return_value_assigned?(node)
return if implicit_return?(node)
return if check_used_in_conditional(node)
return if argument?(node)
return if implicit_return?(node)
return if explicit_return?(node)

add_offense_for_node(node)
Expand Down Expand Up @@ -277,10 +277,18 @@ def implicit_return?(node)
return false unless cop_config['AllowImplicitReturn']

node = assignable_node(node)
method = node.parent
method, sibling_index = find_method_with_sibling_index(node.parent)
return unless method && (method.def_type? || method.block_type?)

method.children.size == node.sibling_index + 1
method.children.size == node.sibling_index + sibling_index
end

def find_method_with_sibling_index(node, sibling_index = 1)
return node, sibling_index unless node&.or_type?

sibling_index += 1

find_method_with_sibling_index(node.parent, sibling_index)
end

def argument?(node)
Expand Down
8 changes: 8 additions & 0 deletions spec/rubocop/cop/rails/save_bang_spec.rb
Expand Up @@ -548,6 +548,14 @@ def whatever
if x.persisted?; something; end
RUBY
end

it "when using #{method} with `||`" do
expect_no_offenses(<<~RUBY)
def find_or_create(**opts)
find(**opts) || #{method}(**opts)
end
RUBY
end
end

described_class::CREATE_PERSIST_METHODS.each do |method|
Expand Down

0 comments on commit f2aa820

Please sign in to comment.