ORM is implicitly assumed to be ActiveRecord

[leoarnold]

Even though most Rails projects use ActiveRecord as ORM, the Rails team has made a big effort to enable developers to use other ORMs.

Is your feature request related to a problem? Please describe.

The following examples are all taken from a Rails 6 project using Mongoid 7 as ORM. The list makes no attempt to list all relevant cops.

Note: The active_record gem will be present somehow in such projects as indirect dependency, brought in through gems like factory_bot or database_cleaner.

Rails/Pick

test.rb:1:67: C: Rails/Pick: Prefer pick(:age) over pluck(:age).first.
Customer.where(active: true).only(:age).sort(age: :desc).limit(1).pluck(:age).first
                                                                  ^^^^^^^^^^^^^^^^^

2.5.8 :001 > Customer.where(active: true).only(:age).sort(age: :desc).limit(1).pick(:age)
Traceback (most recent call last):
        1: from (irb):1
NoMethodError (undefined method `pick' for #<Mongoid::Contextual::Mongo:0x000055e6c815ff90>)

Rails/PluckId

test.rb:1:30: C: Rails/PluckId: Use ids instead of pluck(:id).
Customer.where(active: true).pluck(:id)
                             ^^^^^^^^^^

2.5.8 :001 > Customer.where(active: true).ids
Traceback (most recent call last):
        1: from (irb):1
NoMethodError (undefined method `ids' for #<Mongoid::Criteria:0x000055cc42950830>)

Rails/WhereExists

test.rb:1:10: C: Rails/WhereExists: Prefer exists?(active: true) over where(active: true).exists?.
Customer.where(active: true).exists?
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

2.5.8 :001 > Customer.exists?(active: true)
Traceback (most recent call last):
        1: from (irb):1
ArgumentError (wrong number of arguments (given 1, expected 0))

Describe the solution you'd like

In my opinion, it's a good choice to have the "ActiveRecord cops" as part of this gem and enabled by default is valid and good, since it reflects the reality of the majority of Rails projects.

On the other hand, the minority projects would benefit if they could ...

• disable all "ActiveRecord cops" with a single option
• integrate a RuboCop extension for their ORM with a single line of code

I think this could be addressed by adding a feature toggle like

Rails/ActiveRecord:
  Enabled: false # default: true

and of course any other ORM may feel free to add its own cops through e.g.

require: rubocop-mongoid

The main body of work seems to be the identification of cops which implicitly assume the use of ActiveRecord, and I'm willing to help with this.

[leoarnold]

Thinking one step further, it might be attractive to modularize this gem in the same way Rails itself is effectively a bundle of gems:

For example Rails/Pluck is a cop which addresses #pluck from ActiveRecord relations as well as from ActiveSupport's Enumerable. Therefore it may be of interest to include rubocop-rails in a project which only uses active_support (but none of the other Rails gems) and just set

Rails:
  Enabled: false # default

Rails/ActiveSupport:
  Enabled: true

whereby activating all cops from rubocop-rails which pertain to active_support.

As described above, Rails/Pluck would then have to be adapted/split up in a way such that it reports Enumerables, but not ActiveRecord specific usages.

[tejasbubane]

Currently rubocop detects rails version from Gemfile.lock. One approach to solving this would be to detect active_support & active_record versions as well and based on that enable/disable the dependent cops.

[andyw8]

@tejasbubane both those gems are packaged with Rails, so would still be in Gemfile.lock even if not used.

One approach might be check for the string ActiveRecord::Schema within db/schema.rb.

[leoarnold]

@andyw8 There is nothing keeping you from using both ORMs within the same Rails app.

If each AR cop were to check whether the message receiver actually isbfrom the ActiveRecord namespace, that would be more reliable.

[jafuentest]

Another issue is the Rails/FindEach cop, which doesn't make sense using mongoid since it automatically uses a cursor and batches queries

[gravitystorm]

If each AR cop were to check whether the message receiver actually is from the ActiveRecord namespace, that would be more reliable.

I'd like to second this. One of our models is a FrozenRecord, not ActiveRecord (e.g. class Foo < FrozenRecord::Base) but rubocop-rails flags up various things incorrectly whenever that model class is used (e.g. Rails/WhereExists). Adding specific disable statements is slightly tiresome, but I also want to keep these checks enabled for all the ActiveRecord-based classes.

Is it theoretically possible for rubocop to constrain the cop matches to only subclasses of ActiveRecord? I'm not a rubocop expert, but I can see that the code for the matcher doesn't check anything like that. Are there other examples elsewhere in rubocop that detect a method call but for only a specific class and its descendents? e.g. something that only matches on subclasses of String or Integer etc?

[dvandersluis]

Rubocop performs static analysis and only evaluates one file at a time so it cannot know what base class is used for a given class (and also looking at the actual base class name could be a false positive because you could inherit from something that inherits from ActiveRecord::Base).

Potentially there could be a way added to exclude specific class names from these checks (it seems Lint/NumberConversion is the only cop that uses an IgnoredClasses configuration option presently).

If you are not using ActiveRecord at all, then you can disable the relevant cops in your rubocop.yml file with Enabled: false.