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/SkipsModelValidations false positive when using upsert #1286

Closed
hirowatari opened this issue May 27, 2024 · 1 comment · Fixed by #1287
Closed

Rails/SkipsModelValidations false positive when using upsert #1286

hirowatari opened this issue May 27, 2024 · 1 comment · Fixed by #1287
Labels
bug Something isn't working

Comments

@hirowatari
Copy link

I'm getting a false positive for Rails/SkipsModelValidations. I have other classes that have the method upsert.


Expected behavior

No offences detected

Actual behavior

Offenses detected

Steps to reproduce the problem

# frozen_string_literal: true

class Example # not a active record model
  def self.upsert(args)
    # do something
  end
end

Example.upsert(shop)

bundle exec rubocop ./example.rb
output:

Inspecting 1 file
C

Offenses:

example.rb:9:9: C: Rails/SkipsModelValidations: Avoid using upsert because it skips validations. (https://guides.rubyonrails.org/active_record_validations.html#skipping-validations)
Example.upsert(shop)
        ^^^^^^

1 file inspected, 1 offense detected

RuboCop version

$ bundle exec rubocop -V
1.64.0 (using Parser 3.3.1.0, rubocop-ast 1.31.3, running on ruby 3.2.4) +server [x86_64-linux]
  - rubocop-capybara 2.20.0
  - rubocop-factory_bot 2.25.1
  - rubocop-graphql 1.5.1
  - rubocop-performance 1.21.0
  - rubocop-rails 2.25.0
  - rubocop-rake 0.6.0
  - rubocop-rspec 2.29.2
  - rubocop-rspec_rails 2.28.3
  - rubocop-thread_safety 0.5.1
@koic koic added the bug Something isn't working label May 27, 2024
koic added a commit to koic/rubocop-rails that referenced this issue May 27, 2024
Fixes rubocop#1286.

`Rails/SkipsModelValidations` cop is unsafe if the receiver object is not an Active Record object.
So, this PR marks `SkipsModelValidations` as unsafe.
koic added a commit that referenced this issue May 28, 2024
…_as_unsafe

[Fix #1286] Mark `Rails/SkipsModelValidations` as unsafe
@hirowatari
Copy link
Author

I do not believe this fixes the issue. I tried

  gem "rubocop-rails", github: "rubocop/rubocop-rails", ref: "afcf639"

but found the same error.

The issue is not whether it's unsafe or safe. The issue is that the cop is applying to places where it's not relevant. Namely, any methods named .upsert. You could surely have an .upsert method calling an external API that has nothing to do with Rails.

fwolfst pushed a commit to fwolfst/rubocop-rails that referenced this issue Jun 3, 2024
Fixes rubocop#1286.

`Rails/SkipsModelValidations` cop is unsafe if the receiver object is not an Active Record object.
So, this PR marks `SkipsModelValidations` as unsafe.
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