[Fix #9500] Update Lint/Debugger so that only specific receivers fo…

…r debug methods lead to offenses.

Prior to #8929, `Lint/Debugger` looked for a specific receiver/method pair, but that PR changed it so that any of the `DebuggerReceivers` would apply, *or also* no receiver. This caused regressions (#7636 and #9500, for instance) because method names specified as debugger by this cop would now register offenses when called with no receiver.

In order to fix it, I changed the configuration for `Lint/Debugger` a bit, but in a way that should not cause any major incompatibility. The allow list is now specified including its receiver so that the receiver can be checked. Existing configurations that just pass in a method name will be treated as expecting no receiver (there's a slight incompatibility there because previously `foo` would be matched by `Kernel.foo` but I think this is ok (and is resolvable by adding `Kernel.foo` to the configuration).

I've also grouped the debugger methods by source (it's often confusing to me where each method comes from), which also allows an end-user to disable a pre-defined group of methods by specifying `Byebug: ~` for example.

Finally I've deprecated the `DebuggerReceivers` configuration as it is no longer useful.

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][])

config/default.yml

@@ -1453,23 +1453,31 @@ Lint/Debugger:
   Description: 'Check for debugger calls.'
   Enabled: true
   VersionAdded: '0.14'
-  VersionChanged: '0.49'
-  DebuggerReceivers:
-    - binding
-    - Kernel
-    - Pry
+  VersionChanged: <<next>>
+  DebuggerReceivers: [] # deprecated
   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.'

config/obsoletion.yml

@@ -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:

lib/rubocop/cop/lint/debugger.rb

@@ -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
       #
@@ -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
@@ -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