rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
802 stars 257 forks source link

Rails/SkipsModelValidations false positive when using `upsert` #1286

Closed hirowatari closed 3 months ago

hirowatari commented 3 months ago

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
hirowatari commented 3 months ago

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.