Skip to content

Commit

Permalink
[Fix rubocop#12721] Add DebuggerRequires to Lint/Debugger
Browse files Browse the repository at this point in the history
I debated adding requires like `debug` or `pry` but believe that would
do more harm than good.
RuboCops own spec_helper requires pry. This seems to come from some sort of template.
  • Loading branch information
Earlopain committed Mar 9, 2024
1 parent ca8b38c commit 08a0ed4
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/change_make_lint_debugger_aware_of_require.md
@@ -0,0 +1 @@
* [#12721](https://github.com/rubocop/rubocop/issues/12721): Make `Lint/Debugger` aware of `ruby/debug` requires. ([@earlopain][])
7 changes: 6 additions & 1 deletion config/default.yml
Expand Up @@ -1657,7 +1657,7 @@ Lint/Debugger:
Description: 'Check for debugger calls.'
Enabled: true
VersionAdded: '0.14'
VersionChanged: '1.46'
VersionChanged: '<<next>>'
DebuggerMethods:
# Groups are available so that a specific group can be disabled in
# a user's configuration, but are otherwise not significant.
Expand Down Expand Up @@ -1693,6 +1693,11 @@ Lint/Debugger:
- jard
WebConsole:
- binding.console
DebuggerRequires:
debug.rb:
- debug/start
- debug/open
- debug/open_nonstop

Lint/DeprecatedClassMethods:
Description: 'Check for deprecated class method calls.'
Expand Down
29 changes: 27 additions & 2 deletions lib/rubocop/cop/lint/debugger.rb
Expand Up @@ -29,6 +29,11 @@ module Lint
# MyDebugger.debug_this
# ----
#
# Some gems also ship files that will start a debugging session when required,
# for example `require 'debug/start'` from `ruby/debug`. These requires can
# be configured through `DebuggerRequires`. It has the same structure as
# `DebuggerMethods`, which you can read about above.
#
# @example
#
# # bad (ok during development)
Expand Down Expand Up @@ -64,14 +69,20 @@ module Lint
# def some_method
# my_debugger
# end
#
# @example DebuggerRequires: [my_debugger/start]
#
# # bad (ok during development)
#
# require 'my_debugger/start'
class Debugger < Base
MSG = 'Remove debugger entry point `%<source>s`.'
BLOCK_TYPES = %i[block numblock kwbegin].freeze

def on_send(node)
return if !debugger_method?(node) || assumed_usage_context?(node)
return if assumed_usage_context?(node)

add_offense(node)
add_offense(node) if debugger_method?(node) || debugger_require?(node)
end

private
Expand All @@ -87,12 +98,26 @@ def debugger_methods
end
end

def debugger_requires
@debugger_requires ||= begin
config = cop_config.fetch('DebuggerRequires', [])
config.is_a?(Array) ? config : config.values.flatten
end
end

def debugger_method?(send_node)
return false if send_node.parent&.send_type? && send_node.parent.receiver == send_node

debugger_methods.include?(chained_method_name(send_node))
end

def debugger_require?(send_node)
return false unless send_node.method?(:require) && send_node.arguments.count == 1
return false unless (argument = send_node.first_argument).str_type?

debugger_requires.include?(argument.value)
end

def assumed_usage_context?(node)
# Basically, debugger methods are not used as a method argument without arguments.
return false unless node.arguments.empty? && node.each_ancestor(:send, :csend).any?
Expand Down
56 changes: 56 additions & 0 deletions spec/rubocop/cop/lint/debugger_spec.rb
Expand Up @@ -124,6 +124,62 @@
end
end

context 'with the DebuggerRequires configuration' do
let(:cop_config) do
{
'DebuggerRequires' => {
'my_debugger' => %w[my_debugger],
'other_debugger' => %w[other_debugger/start]
}
}
end

it 'registers an offense' do
expect_offense(<<~RUBY)
require 'my_debugger'
^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `require 'my_debugger'`.
RUBY
end

it 'registers no offense for a require without arguments' do
expect_no_offenses('require')
end

it 'registers no offense for a require with multiple arguments' do
expect_no_offenses('require "my_debugger", "something_else"')
end

it 'registers no offense for a require with non-string arguments' do
expect_no_offenses('require my_debugger')
end

context 'when a require group is disabled with nil' do
before { cop_config['DebuggerRequires']['my_debugger'] = nil }

it 'does not register an offense for a require call' do
expect_no_offenses('require "my_debugger"')
end

it 'does register an offense for another group' do
expect_offense(<<~RUBY)
require 'other_debugger/start'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `require 'other_debugger/start'`.
RUBY
end
end

context 'when the config is specified as an array' do
let(:cop_config) { { 'DebuggerRequires' => %w[start_debugging] } }

it 'registers an offense' do
expect_offense(<<~RUBY)
require 'start_debugging'
^^^^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `require 'start_debugging'`.
RUBY
end
end
end

context 'built-in methods' do
it 'registers an offense for a irb binding call' do
expect_offense(<<~RUBY)
Expand Down

0 comments on commit 08a0ed4

Please sign in to comment.