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/ActiveRecordOverride crashing on modules #6943

Closed
ghiculescu opened this issue Apr 17, 2019 · 5 comments
Closed

Rails/ActiveRecordOverride crashing on modules #6943

ghiculescu opened this issue Apr 17, 2019 · 5 comments

Comments

@ghiculescu
Copy link
Contributor

Rails/ActiveRecordOverride crashes on the following code:

module Foo
  def bar
    true
  end

  def update
    true
  end
end
Scanning test.rb
An error occurred while Rails/ActiveRecordOverride cop was inspecting test.rb:6:2.
undefined method `const_name' for nil:NilClass
/Users/alex/.rvm/gems/ruby-2.5.3/gems/rubocop-0.67.2/lib/rubocop/cop/rails/active_record_override.rb:42:in `on_def'

Note that it does not crash if the module only contains 1 method.

This is the same root cause as #6888 but I logged it separately as it impacts modules, which IMO this cop shouldn't check.


Expected behavior

Don't crash, but record a lint error.

Actual behavior

Crash.

Steps to reproduce the problem

Code above.

RuboCop version

0.67.2 (using Parser 2.6.2.1, running on ruby 2.5.3 x86_64-darwin15)

@andreasklinger
Copy link

Same error

    rubocop (0.67.2)
    parser (2.6.2.1)
    ruby 2.4.5p335 (2018-10-18 revision 65137) [x86_64-darwin18]

@andreasklinger
Copy link

happens on all rubocop version - seems to be the cop itself

i didn't look into this properly now but this workaround might help someone landing in this issue
add this to rubocop.yml

Rails/ActiveRecordOverride:
  Enabled: false

@tiagotex
Copy link
Contributor

tiagotex commented Apr 18, 2019

I also found the same problem, modules don't have a parent class so the code fails when we ask for the parent_class.const_name

return unless %w[ApplicationRecord ActiveModel::Base]
                        .include?(parent_class.const_name)

I will create a PR to return early when there is no parent class since we can't determine if the methods are overriding active record.

@tiagotex
Copy link
Contributor

Well, it seems like this is a duplicate of #6888 and it is already solved on master (#6889) 😄

@koic koic closed this as completed Apr 18, 2019
@ghiculescu
Copy link
Contributor Author

Thanks for that, any ideas when that’ll be on rubygems?

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

No branches or pull requests

4 participants