Skip to content

Commit

Permalink
[Fix rubocop#6888] Fix an error for Rails/ActiveRecordOverride
Browse files Browse the repository at this point in the history
Fixes rubocop#6888.

This PR fixes the following error for `Rails/ActiveRecordOverride`
when there is no `parent_class` to match against.

```ruby
% cat app/models/user.rb
class User
  def initialize; end

  def create; end
end
```

```console
% rubocop app/models/user.rb --only Rails/ActiveRecordOverride -d
For /Users/dannyiachini/github/jpw/rubocop_ex: configuration from /Users/dannyiachini/github/jpw/rubocop_ex/.rubocop.yml
Default configuration from /Users/dannyiachini/.gem/ruby/2.5.3/gems/rubocop-0.67.2/config/default.yml
Inspecting 1 file
Scanning /Users/dannyiachini/github/jpw/rubocop_ex/app/models/user.rb
An error occurred while Rails/ActiveRecordOverride cop was inspecting /Users/dannyiachini/github/jpw/rubocop_ex/app/models/user.rb:6:2.
undefined method `const_name' for nil:NilClass
/Users/dannyiachini/.gem/ruby/2.5.3/gems/rubocop-0.67.2/lib/rubocop/cop/rails/active_record_override.rb:42:in `on_def'

(snip)

1 file inspected, no offenses detected

1 error occurred:
An error occurred while Rails/ActiveRecordOverride cop was inspecting /Users/dannyiachini/github/jpw/rubocop_ex/app/models/user.rb:6:2.
Errors are usually caused by RuboCop bugs.
Please, report your problems to RuboCop's issue tracker.
https://github.com/rubocop-hq/rubocop/issues

Mention the following information in the issue report:
0.67.2 (using Parser 2.6.2.0, running on ruby 2.5.3 x86_64-darwin17)
```
  • Loading branch information
diachini committed Apr 12, 2019
1 parent cdf442b commit 9e1c4ed
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#6914](https://github.com/rubocop-hq/rubocop/issues/6914): [Fix #6914] Fix an error for `Rails/RedundantAllowNil` when with interpolations. ([@Blue-Pix][])
* [#6888](https://github.com/rubocop-hq/rubocop/issues/6888): Fix an error for `Rails/ActiveRecordOverride ` when no `parent_class` present. ([@diachini][])

### Changes

Expand Down Expand Up @@ -3937,3 +3938,4 @@
[@XrXr]: https://github.com/XrXr
[@thomthom]: https://github.com/thomthom
[@Blue-Pix]: https://github.com/Blue-Pix
[@diachini]: https://github.com/diachini
28 changes: 22 additions & 6 deletions lib/rubocop/cop/rails/active_record_override.rb
Expand Up @@ -29,17 +29,15 @@ class ActiveRecordOverride < Cop
'Use %<prefer>s callbacks instead of overriding the Active Record ' \
'method `%<bad>s`.'.freeze
BAD_METHODS = %i[create destroy save update].freeze
ACTIVE_RECORD_CLASSES = %w[ApplicationRecord ActiveModel::Base
ActiveRecord::Base].freeze

def on_def(node)
method_name = node.method_name
return unless BAD_METHODS.include?(node.method_name)

parent_parts = node.parent.node_parts
parent_class = parent_parts.take_while do |part|
!part.nil? && part.const_type?
end.last
return unless %w[ApplicationRecord ActiveModel::Base]
.include?(parent_class.const_name)
parent_class_name = find_parent_class_name(node)
return unless active_model?(parent_class_name)

return unless node.descendants.any?(&:zsuper_type?)

Expand All @@ -48,6 +46,10 @@ def on_def(node)

private

def active_model?(parent_class_name)
ACTIVE_RECORD_CLASSES.include?(parent_class_name)
end

def callback_names(method_name)
names = %w[before_ around_ after_].map do |prefix|
"`#{prefix}#{method_name}`"
Expand All @@ -61,6 +63,20 @@ def callback_names(method_name)
def message(method_name)
format(MSG, prefer: callback_names(method_name), bad: method_name)
end

def find_parent_class_name(node)
return nil unless node

if node.class_type?
parent_class_name = node.node_parts[1]

return nil if parent_class_name.nil?

return parent_class_name.source
end

find_parent_class_name(node.parent)
end
end
end
end
Expand Down
14 changes: 14 additions & 0 deletions spec/rubocop/cop/rails/active_record_override_spec.rb
Expand Up @@ -41,6 +41,8 @@ def save
it 'registers an offense when overriding update' do
expect_offense(<<-RUBY.strip_indent)
class X < ActiveModel::Base
module_function
def update
^^^^^^^^^^ Use `before_update`, `around_update`, or `after_update` callbacks instead of overriding the Active Record method `update`.
super
Expand Down Expand Up @@ -84,4 +86,16 @@ def save
RUBY
end
end

context 'when class has no parent specified' do
it 'registers no offense when overriding save' do
expect_no_offenses(<<-RUBY.strip_indent)
class X
def initialize; end
def save; end
end
RUBY
end
end
end

0 comments on commit 9e1c4ed

Please sign in to comment.