From 08a0ed4f11646aaf792c4eaac1db26b63a2560c9 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Sat, 9 Mar 2024 21:23:47 +0100 Subject: [PATCH] [Fix #12721] Add `DebuggerRequires` to `Lint/Debugger` 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. --- ...nge_make_lint_debugger_aware_of_require.md | 1 + config/default.yml | 7 ++- lib/rubocop/cop/lint/debugger.rb | 29 +++++++++- spec/rubocop/cop/lint/debugger_spec.rb | 56 +++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 changelog/change_make_lint_debugger_aware_of_require.md diff --git a/changelog/change_make_lint_debugger_aware_of_require.md b/changelog/change_make_lint_debugger_aware_of_require.md new file mode 100644 index 00000000000..bcb2fe7ad6d --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index e0887f71161..643e7a3ea57 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1657,7 +1657,7 @@ Lint/Debugger: Description: 'Check for debugger calls.' Enabled: true VersionAdded: '0.14' - VersionChanged: '1.46' + VersionChanged: '<>' DebuggerMethods: # Groups are available so that a specific group can be disabled in # a user's configuration, but are otherwise not significant. @@ -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.' diff --git a/lib/rubocop/cop/lint/debugger.rb b/lib/rubocop/cop/lint/debugger.rb index f178d4828b4..0f40e9b611e 100644 --- a/lib/rubocop/cop/lint/debugger.rb +++ b/lib/rubocop/cop/lint/debugger.rb @@ -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) @@ -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 `%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 @@ -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? diff --git a/spec/rubocop/cop/lint/debugger_spec.rb b/spec/rubocop/cop/lint/debugger_spec.rb index 291c6b0ab8e..4beb960b4ea 100644 --- a/spec/rubocop/cop/lint/debugger_spec.rb +++ b/spec/rubocop/cop/lint/debugger_spec.rb @@ -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)