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

[Fix #912] Enhance Rails/Delegate by adding delegation detection for self.class, constants, class variables, global variables, and instance variables #1438

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#912](https://github.com/rubocop/rubocop-rails/issues/912): Enhance `Rails/Delegate` by adding delegation detection for `self.class`, constants, class variables, global variables, and instance variables. ([@ydakuka][])
49 changes: 41 additions & 8 deletions lib/rubocop/cop/rails/delegate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class Delegate < Base

def_node_matcher :delegate?, <<~PATTERN
(def _method_name _args
(send {(send nil? _) (self)} _ ...))
(send {(send nil? _) (self) (send (self) :class) ({cvar gvar ivar} _) (const _ _)} _ ...))
PATTERN

def on_def(node)
Expand All @@ -82,17 +82,39 @@ def on_def(node)

def register_offense(node)
add_offense(node.loc.keyword) do |corrector|
body = node.body
receiver = determine_register_offense_receiver(node.body.receiver)
delegation = build_delegation(node, receiver)

receiver = body.receiver.self_type? ? 'self' : ":#{body.receiver.method_name}"

delegation = ["delegate :#{body.method_name}", "to: #{receiver}"]
delegation << ['prefix: true'] if node.method?(prefixed_method_name(node.body))
corrector.replace(node, delegation)
end
end

corrector.replace(node, delegation.join(', '))
def determine_register_offense_receiver(receiver)
case receiver.type
when :self
'self'
when :const
full_name = full_const_name(receiver)
full_name.include?('::') ? ":'#{full_name}'" : ":#{full_name}"
when :cvar, :gvar, :ivar
":#{receiver.source}"
else
":#{receiver.method_name}"
end
end

def build_delegation(node, receiver)
delegation = ["delegate :#{node.body.method_name}", "to: #{receiver}"]
delegation << ['prefix: true'] if node.method?(prefixed_method_name(node.body))
delegation.join(', ')
end

def full_const_name(node)
return node.source unless node.namespace

"#{full_const_name(node.namespace)}::#{node.children.last}"
end

def trivial_delegate?(def_node)
delegate?(def_node) &&
method_name_matches?(def_node.method_name, def_node.body) &&
Expand Down Expand Up @@ -120,7 +142,18 @@ def include_prefix_case?
def prefixed_method_name(body)
return '' if body.receiver.self_type?

[body.receiver.method_name, body.method_name].join('_').to_sym
[determine_prefixed_method_receiver_name(body.receiver), body.method_name].join('_').to_sym
end

def determine_prefixed_method_receiver_name(receiver)
case receiver.type
when :cvar, :gvar, :ivar
receiver.source
when :const
full_const_name(receiver)
else
receiver.method_name.to_s
end
end

def private_or_protected_delegation(node)
Expand Down
87 changes: 78 additions & 9 deletions spec/rubocop/cop/rails/delegate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,6 @@ def new
RUBY
end

it 'ignores delegation to constant' do
expect_no_offenses(<<~RUBY)
FOO = []
def size
FOO.size
end
RUBY
end

it 'ignores code with no receiver' do
expect_no_offenses(<<~RUBY)
def change
Expand Down Expand Up @@ -249,4 +240,82 @@ def foo
end
RUBY
end

it 'detects delegation to `self.class`' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
self.class.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :class
RUBY
end

it 'detects delegation to a constant' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
CONST.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :CONST
RUBY
end

it 'detects delegation to a namespaced constant' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
SomeModule::CONST.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :'SomeModule::CONST'
RUBY
end

it 'detects delegation to an instance variable' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
@instance_variable.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :@instance_variable
RUBY
end

it 'detects delegation to a class variable' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
@@class_variable.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :@@class_variable
RUBY
end

it 'detects delegation to a global variable' do
expect_offense(<<~RUBY)
def foo
^^^ Use `delegate` to define delegations.
$global_variable.foo
end
RUBY

expect_correction(<<~RUBY)
delegate :foo, to: :$global_variable
RUBY
end
end
Loading