Skip to content

Commit

Permalink
[Fix #6888] Fix an error for Rails/ActiveRecordOverride (#6889)
Browse files Browse the repository at this point in the history
Fixes #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 authored and bbatsov committed Apr 18, 2019
1 parent b638bb8 commit 4ff05ce
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@

* Do not annotate message with cop name in JSON output. ([@elebow][])
* [#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][])
* [#6941](https://github.com/rubocop-hq/rubocop/issues/6941): Add missing absence validations to `Rails/Validation`. ([@jmanian][])
* [#6926](https://github.com/rubocop-hq/rubocop/issues/6926): [Fix #6926] Allow nil values to unset config defaults. ([@dduugg][])
* [#6946](https://github.com/rubocop-hq/rubocop/pull/6946): Allow `Rails/ReflectionClassName` to use string interpolation for `class_name`. ([@r7kamura][])
Expand Down Expand Up @@ -3948,5 +3949,6 @@
[@XrXr]: https://github.com/XrXr
[@thomthom]: https://github.com/thomthom
[@Blue-Pix]: https://github.com/Blue-Pix
[@diachini]: https://github.com/diachini
[@Mange]: https://github.com/Mange
[@jmanian]: https://github.com/jmanian
[@jmanian]: https://github.com/jmanian
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 4ff05ce

Please sign in to comment.