Skip to content

Commit

Permalink
Fix a false positive for Lint/Debugger when methods containing diff…
Browse files Browse the repository at this point in the history
…erent method chains

This PR is fix a false positive for `Lint/Debugger` when methods containing different method chains.

## Motivation
I don't think that methods such as `p.do_something` for example should be an offense of this cop.
I think it would be better to make it a violation only if it is an exact match, including the method chain, but what do you think?

## This PR will result in the following

```ruby
# bad
p 'foo'
foo(p 'foo')
p(p 'foo')

# good
p.do_something
Foo.p
```
  • Loading branch information
ydah committed Feb 8, 2023
1 parent b1b393a commit 23cc42b
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 27 deletions.
1 change: 1 addition & 0 deletions changelog/fix_a_false_positive_for_lint_debugger.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11552](https://github.com/rubocop/rubocop/pull/11552): Fix a false positive for `Lint/Debugger` when methods containing different method chains. ([@ydah][])
35 changes: 8 additions & 27 deletions lib/rubocop/cop/lint/debugger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,44 +82,25 @@ def message(node)
def debugger_methods
@debugger_methods ||= begin
config = cop_config.fetch('DebuggerMethods', [])
values = config.is_a?(Array) ? config : config.values.flatten
values.map do |v|
next unless v

*receiver, method_name = v.split('.')
{
receiver: receiver.empty? ? nil : receiver.map(&:to_sym),
method_name: method_name.to_sym
}
end.compact
config.is_a?(Array) ? config : config.values.flatten
end
end

def debugger_method?(send_node)
method_name = send_node.method_name

debugger_methods.any? do |method|
next unless method[:method_name] == method_name
return if send_node.parent&.send_type? && send_node.parent.receiver == send_node

if method[:receiver].nil?
send_node.receiver.nil?
else
method[:receiver] == receiver_chain(send_node)
end
end
debugger_methods.include?(chained_method_name(send_node))
end

def receiver_chain(send_node)
receivers = []
def chained_method_name(send_node)
chained_method_name = send_node.method_name.to_s
receiver = send_node.receiver

while receiver
name = receiver.send_type? ? receiver.method_name : receiver.const_name&.to_sym
receivers.unshift(name)
name = receiver.send_type? ? receiver.method_name : receiver.const_name
chained_method_name = "#{name}.#{chained_method_name}"
receiver = receiver.receiver
end

receivers
chained_method_name
end
end
end
Expand Down
34 changes: 34 additions & 0 deletions spec/rubocop/cop/lint/debugger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,40 @@
^^^^^^^^^^^^^^^^^^ Remove debugger entry point `Kernel.binding.irb`.
RUBY
end

it 'registers an offense for a p call' do
expect_offense(<<~RUBY)
p 'foo'
^^^^^^^ Remove debugger entry point `p 'foo'`.
RUBY
end

it 'registers an offense for a p call with foo method argument' do
expect_offense(<<~RUBY)
foo(p 'foo')
^^^^^^^ Remove debugger entry point `p 'foo'`.
RUBY
end

it 'registers an offense for a p call with p method argument' do
expect_offense(<<~RUBY)
p(p 'foo')
^^^^^^^ Remove debugger entry point `p 'foo'`.
^^^^^^^^^^ Remove debugger entry point `p(p 'foo')`.
RUBY
end

it 'does not register an offense for a p.do_something call' do
expect_no_offenses(<<~RUBY)
p.do_something
RUBY
end

it 'does not register an offense for a p with Foo call' do
expect_no_offenses(<<~RUBY)
Foo.p
RUBY
end
end

context 'byebug' do
Expand Down

0 comments on commit 23cc42b

Please sign in to comment.