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 #9500] Update Lint/Debugger so that only specific receivers for debug methods lead to offenses #9504

Merged
merged 1 commit into from Feb 12, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/fix_update_lintdebugger_so_that_only.md
@@ -0,0 +1 @@
* [#9500](https://github.com/rubocop-hq/rubocop/issues/9500): Update `Lint/Debugger` so that only specific receivers for debug methods lead to offenses. ([@dvandersluis][])
40 changes: 24 additions & 16 deletions config/default.yml
Expand Up @@ -1446,23 +1446,31 @@ Lint/Debugger:
Description: 'Check for debugger calls.'
Enabled: true
VersionAdded: '0.14'
VersionChanged: '0.49'
DebuggerReceivers:
- binding
- Kernel
- Pry
VersionChanged: <<next>>
DebuggerReceivers: [] # deprecated
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want users to get error messages about an invalid configuration (in case they've set this) so I've added it to the obsoletion config (with warning severity) but kept the configuration.

DebuggerMethods:
- debugger
- byebug
- remote_byebug
- pry
- remote_pry
- pry_remote
- console
- rescue
- save_and_open_page
- save_and_open_screenshot
- irb
# Groups are available so that a specific group can be disabled in
# a user's configuration, but are otherwise not significant.
Kernel:
- binding.irb
Byebug:
- byebug
- remote_byebug
- Kernel.byebug
- Kernel.remote_byebug
Capybara:
- save_and_open_page
- save_and_open_screenshot
Pry:
- binding.pry
- binding.remote_pry
- binding.pry_remote
- Pry.rescue
Rails:
- debugger
- Kernel.debugger
WebConsole:
- binding.console

Lint/DeprecatedClassMethods:
Description: 'Check for deprecated class method calls.'
Expand Down
4 changes: 4 additions & 0 deletions config/obsoletion.yml
Expand Up @@ -187,6 +187,10 @@ changed_parameters:
parameters: ExcludedMethods
alternative: IgnoredMethods
severity: warning
- cops: Lint/Debugger
parameters: DebuggerReceivers
reason: "`DebuggerReceivers` is no longer necessary, method receivers should be specified in `DebuggerMethods` instead."
severity: warning

# Enforced styles that have been removed or replaced
changed_enforced_styles:
Expand Down
72 changes: 58 additions & 14 deletions lib/rubocop/cop/lint/debugger.rb
Expand Up @@ -3,8 +3,21 @@
module RuboCop
module Cop
module Lint
# This cop checks for calls to debugger or pry.
# The cop can be configured to define which methods and receivers must be fixed.
# This cop checks for debug calls (such as `debugger` or `binding.pry`) that should
# not be kept for production code.
#
# The cop can be configured using `DebuggerMethods`. By default, a number of gems
# debug entrypoints are configured (`Kernel`, `Byebug`, `Capybara`, `Pry`, `Rails`,
# and `WebConsole`). Additional methods can be added.
#
# Specific default groups can be disabled if necessary:
#
# [source,yaml]
# ----
# Lint/Debugger:
# WebConsole: ~
# ---
#
#
# @example
#
Expand Down Expand Up @@ -33,14 +46,32 @@ module Lint
# def some_method
# do_something
# end
#
# @example DebuggerMethods: [my_debugger]
#
# # bad (ok during development)
#
# def some_method
# my_debugger
# end
class Debugger < Base
MSG = 'Remove debugger entry point `%<source>s`.'

RESTRICT_ON_SEND = [].freeze

def_node_matcher :kernel?, <<~PATTERN
(const {nil? cbase} :Kernel)
PATTERN

def_node_matcher :valid_receiver?, <<~PATTERN
{
(const {nil? cbase} %1)
(send {nil? #kernel?} %1)
}
PATTERN

def on_send(node)
return unless debugger_method?(node.method_name)
return if !node.receiver.nil? && !debugger_receiver?(node)
return unless debugger_method?(node)

add_offense(node)
end
Expand All @@ -51,19 +82,32 @@ def message(node)
format(MSG, source: node.source)
end

def debugger_method?(name)
cop_config.fetch('DebuggerMethods', []).include?(name.to_s)
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.join.to_sym,
method_name: method_name.to_sym
}
end.compact
end
end

def debugger_receiver?(node)
receiver = case node.receiver
when RuboCop::AST::SendNode
node.receiver.method_name
when RuboCop::AST::ConstNode
node.receiver.const_name
end
def debugger_method?(send_node)
debugger_methods.any? do |method|
next unless method[:method_name] == send_node.method_name

cop_config.fetch('DebuggerReceivers', []).include?(receiver.to_s)
if method[:receiver].nil?
send_node.receiver.nil?
else
valid_receiver?(send_node.receiver, method[:receiver])
end
end
end
end
end
Expand Down