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

Open
exterm opened this issue Nov 8, 2023 · 2 comments · May be fixed by #1180
Open

Rails/ReflectionClassName complains on variables #1179

exterm opened this issue Nov 8, 2023 · 2 comments · May be fixed by #1180

Comments

@exterm
Copy link

exterm commented Nov 8, 2023

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.

exterm added a commit to exterm/rubocop-rails that referenced this issue Nov 10, 2023
exterm added a commit to exterm/rubocop-rails that referenced this issue Nov 10, 2023
@exterm exterm linked a pull request Nov 10, 2023 that will close this issue
9 tasks
exterm added a commit to exterm/rubocop-rails that referenced this issue Nov 10, 2023
exterm added a commit to exterm/rubocop-rails that referenced this issue Nov 14, 2023
exterm added a commit to exterm/rubocop-rails that referenced this issue Nov 14, 2023
exterm added a commit to exterm/rubocop-rails that referenced this issue Dec 1, 2023
@vlad-pisanov
Copy link
Contributor

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
Copy link
Author

exterm commented Dec 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants