Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails/SaveBang does not account for control flow #53

Closed
Malabarba opened this issue May 10, 2019 · 2 comments · Fixed by #90
Closed

Rails/SaveBang does not account for control flow #53

Malabarba opened this issue May 10, 2019 · 2 comments · Fixed by #90
Labels
bug Something isn't working

Comments

@Malabarba
Copy link

Malabarba commented May 10, 2019

Expected behavior

I expect the following code to be allowed by rubocop, because the value of the create method is (possibly) returned by the function, and AllowImplicitReturn is true by default.

def find_or_create(**opts)
  find(**opts) || create(**opts)
end

Actual behavior

The Rails/SaveBang cop reports that I should use create! instead of create.

my_model.rb:3:21: C: Rails/SaveBang: create returns a model which is always truthy.
    find(**opts) || create(**opts)
                    ^^^^^^

Steps to reproduce the problem

Assuming you have Rails cops enabled. Save the following file and run rubocop filename.rb.

class MyModel < ActiveRecord::Model
  def find_or_create(**opts)
    find(**opts) || create(**opts)
  end
end

RuboCop version

0.68.1 (using Parser 2.6.3.0, running on ruby 2.6.1 x86_64-linux)
@Malabarba
Copy link
Author

👋 :)

@koic koic added the bug Something isn't working label Jul 3, 2019
koic added a commit to koic/rubocop-rails that referenced this issue Jul 20, 2019
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
```
koic added a commit to koic/rubocop-rails that referenced this issue Jul 20, 2019
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
```
@koic koic closed this as completed in #90 Jul 22, 2019
koic added a commit that referenced this issue Jul 22, 2019
…bang

[Fix #53] Fix a false positive for `Rails/SaveBang`
@Malabarba
Copy link
Author

Thank you very much! :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants