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/ReflectionClassName complains on variables #1179

Open exterm opened 10 months ago

exterm commented 10 months ago

Expected behavior

These are the examples from the docs:

# bad
has_many :accounts, class_name: Account
has_many :accounts, class_name: Account.name

# good
has_many :accounts, class_name: 'Account'

I would also expect

# good
has_many :accounts, class_name: account_class
has_many :accounts, class_name: classes[:account]
has_many :accounts, class_name: classes.account

Because the purpose of the cop, if I understand correctly, is to prevent constant references and the autoload they cause, so the cop should only reject constant references.

Actual behavior

The cop rejects everything that is not a string literal.

Steps to reproduce the problem

See above.

RuboCop version

1.56.2 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.2.2) [arm64-darwin22]
  - rubocop-factory_bot 2.24.0
  - rubocop-graphql 1.0.1
  - rubocop-performance 1.14.3
  - rubocop-rails 2.21.0
  - rubocop-sorbet 0.7.0

I checked changelog and code and this doesn't seem to be fixed in 2.22.1, which is the most current version as I'm writing this.

vlad-pisanov commented 9 months ago

Method calls are unsafe too, since they could reference an unloaded constant:

def account_class
  Account
end

has_many :accounts, class_name: account_class
exterm commented 9 months ago

Yeah, this cop will never be able to catch all cases. That's OK. I'm just looking to reduce the number of false positives.