Open leoarnold opened 3 years ago
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 Enumerable
s, but not ActiveRecord specific usages.
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.
@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
.
@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.
Another issue is the Rails/FindEach
cop, which doesn't make sense using mongoid since it automatically uses a cursor and batches queries
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?
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
.
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 likefactory_bot
ordatabase_cleaner
.Rails/Pick
Rails/PluckId
Rails/WhereExists
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 ...
I think this could be addressed by adding a feature toggle like
and of course any other ORM may feel free to add its own cops through e.g.
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.