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

New cop: Dangerous name for column and enum #158

Closed
pocke opened this issue Nov 27, 2019 · 3 comments
Closed

New cop: Dangerous name for column and enum #158

pocke opened this issue Nov 27, 2019 · 3 comments
Labels
enhancement New feature or request feature request Request for new functionality

Comments

@pocke
Copy link
Contributor

pocke commented Nov 27, 2019

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

Some keywords, which is allowed by AR, conflict Rails features.
The most popular example is "type". It conflicts with STI.

And we can find other examples.

class C < ApplicationRecord
  enum x: {
    :present, # It breaks C#present?
    :none,
}
end

Describe the solution you'd like

Detect the dangerous names by RuboCop.

Describe alternatives you've considered

In the enum case, we can use _prefix: true option instead.
So the cop should be aware of the option.

Additional context

I think we need more examples for the cop. If you have any idea, please comment to this issue!

I was inspired by ruby-jp members. Thanks a lot!

@pocke pocke added enhancement New feature or request feature request Request for new functionality labels Nov 27, 2019
@pocke
Copy link
Contributor Author

pocke commented Nov 27, 2019

Rails has detecting conflict feature, but I cannot get any errors with enum and present. I'm not sure why I cannot get it. I guess we need to investigate the rails code.
https://github.com/rails/rails/blob/e3ed320cb44f042f01d64e75278aa502789b2fe5/activerecord/lib/active_record/enum.rb#L251-L261

@tomasv
Copy link

tomasv commented Feb 4, 2020

I have another case. With Rails 6 enum now autogenerates negated scopes for each enum value as well and it becomes dangerous to have enum values starting with not_.

Consider a case like this:

class MyModel < ApplicationRecord
  enum status: {
    not_foo: 1,
    foo: 2,
    bar: 3,
  }
end

MyModel.not_foo.to_sql generates WHERE status != 2, which isn't
nice, because we actually expect WHERE status = 1.

It does throw a warning but warnings often get swallowed by CI environments.

@ybiquitous
Copy link
Contributor

FYI. The next Rails version (7.1?) may cause runtime errors when using some dangerous methods from Object:

r7kamura added a commit to r7kamura/rubocop-rails that referenced this issue Jun 13, 2023
r7kamura added a commit to r7kamura/rubocop-rails that referenced this issue Jun 13, 2023
r7kamura added a commit to r7kamura/rubocop-rails that referenced this issue Jun 13, 2023
@koic koic closed this as completed in 1f11020 Aug 24, 2023
koic added a commit that referenced this issue Aug 24, 2023
[Fix #158] Add `Rails/DangerousColumnNames` cop
masato-bkn pushed a commit to masato-bkn/rubocop-rails that referenced this issue Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request Request for new functionality
Projects
None yet
Development

No branches or pull requests

3 participants