Use correct debugger frame by advising Pdb.__init__ instead of entry points

lib/python/pyflyby/_interactive.py

@@ -33,8 +33,8 @@
 from   pyflyby._log             import logger
 from   pyflyby._modules         import ModuleHandle
 from   pyflyby._parse           import PythonBlock
-from   pyflyby._util            import (AdviceCtx, Aspect, CwdCtx,
-                                        FunctionWithGlobals, advise)
+from   pyflyby._util            import (Aspect, CwdCtx, FunctionWithGlobals,
+                                        advise)
 
 if TYPE_CHECKING:
     from IPython.core.application import BaseIPythonApplication
@@ -646,6 +646,10 @@ def _enable_pdb_hooks(pdb_instance):
     # Enable hooks in pdb.Pdb.
     # Should be called after pdb.Pdb.__init__().
     logger.debug("_enable_pdb_hooks(%r)", pdb_instance)
+    # Already done by our advice on pdb.Pdb.__init__, if it's installed.
+    if getattr(pdb_instance, "_pyflyby_hooks_enabled", False):
+        return
+    pdb_instance._pyflyby_hooks_enabled = True
     # Patch Pdb._getval() to use auto_eval.
     # This supports 'ipdb> p foo'.
     @advise(pdb_instance._getval)
@@ -1570,64 +1574,27 @@ def safe_execfile_with_autoimport(filename,
             return False
 
     def _enable_debugger_hook(self, ip):
-        try:
-            Pdb = _get_IPdb_class()
-        except Exception as e:
-            logger.debug("Couldn't locate Pdb class: %s: %s",
-                         type(e).__name__, e)
-            return False
+        import pdb
+        # Advise the debugger classes rather than the ways of entering a
+        # debugger: advice around e.g. breakpoint() adds pyflyby frames below
+        # the user's, and the debugger would open in those.  IPython's Pdb and
+        # TerminalPdb call pdb.Pdb.__init__ from theirs; in a kernel, pdb.Pdb
+        # is already IPython's InterruptiblePdb.
+        @self._advise((pdb.Pdb, "__init__"))
+        def Pdb_with_autoimport(self_pdb, *args, **kwargs):
+            __original__(self_pdb, *args, **kwargs)
+            _enable_pdb_hooks(self_pdb)
         try:
             TerminalPdb = _get_TerminalPdb_class()
         except Exception as e:
             logger.debug("Couldn't locate TerminalPdb class: %s: %s",
                          type(e).__name__, e)
-            TerminalPdb = None
-        @contextmanager
-        def HookPdbCtx():
-            def Pdb_with_autoimport(self_pdb, *args):
-                __original__(self_pdb, *args)
-                _enable_pdb_hooks(self_pdb)
-            def TerminalPdb_with_autoimport(self_pdb, *args):
-                __original__(self_pdb, *args)
+        else:
+            @self._advise((TerminalPdb, "__init__"))
+            def TerminalPdb_with_autoimport(self_pdb, *args, **kwargs):
+                __original__(self_pdb, *args, **kwargs)
                 _enable_terminal_pdb_hooks(self_pdb, self)
-            with AdviceCtx(Pdb.__init__, Pdb_with_autoimport):
-                if TerminalPdb is None:
-                    yield
-                else:
-                    with AdviceCtx(TerminalPdb.__init__, TerminalPdb_with_autoimport):
-                        yield
         ok = True
-
-        @self._advise(sys.breakpointhook)
-        def breakpoint_debugger_with_autoimport(*args, **kwargs):
-            with HookPdbCtx():
-                return __original__(*args, **kwargs)
-
-        try:
-            import ipdb
-
-            if hasattr(ipdb, "set_trace"):
-                @self._advise((ipdb, "set_trace"))
-                def ipdb_debugger_with_autoimport(*args, **kwargs):
-                    with HookPdbCtx():
-                        return __original__(*args, **kwargs)
-            else:
-                ok = False
-        except ModuleNotFoundError:
-            pass
-
-        iptb = getattr(ip, "InteractiveTB", None)
-        if hasattr(iptb, "debugger"):
-            # Hook ip.InteractiveTB.debugger().  This implements auto
-            # importing for "%debug" (postmortem mode).
-            # Tested with IPython 0.10, 0.11, 0.12, 0.13, 1.0, 1.1, 1.2, 2.0,
-            # 2.1, 2.2, 2.3, 2.4, 3.0, 3.1, 3.2, 4.0.
-            @self._advise(iptb.debugger)
-            def debugger_with_autoimport(*args, **kwargs):
-                with HookPdbCtx():
-                    return __original__(*args, **kwargs)
-        else:
-            ok = False
         if hasattr(ip, 'magics_manager'):
             # Hook ExecutionMagics._run_with_debugger().  This implements auto
             # importing for "%debug <statement>".
@@ -1642,10 +1609,8 @@ def run_with_debugger_with_autoimport(code, code_ns,
                                                       *args, **kwargs):
                     db = ImportDB.get_default(filename or ".")
                     auto_import(code, namespaces=[code_ns], db=db)
-                    with HookPdbCtx():
-                        return __original__(code, code_ns, filename,
-                                            *args, **kwargs
-                        )
+                    return __original__(code, code_ns, filename,
+                                        *args, **kwargs)
         else:
             ok = False
         return ok

tests/test_interactive.py

@@ -218,7 +218,7 @@ def parse_template(template, clear_tab_completions=False):
     template = dedent(template).strip().encode('utf-8')
     input = []
     expected = b""
-    pattern = re.compile(br"^(?:In \[[0-9]+\]:|   [.][.][.]+:|ipdb>|>>>)(?: |$)", re.M)
+    pattern = re.compile(br"^(?:In \[[0-9]+\]:|   [.][.][.]+:|ipdb>|\(Pdb\)|>>>)(?: |$)", re.M)
     while template:
         m = pattern.search(template)
         if not m:
@@ -483,10 +483,12 @@ def _parse_version(version_string):
 _IPYTHON_PROMPT2 = br"\n\r?   [.][.][.]+: "
 _PYTHON_PROMPT = br">>> "
 _IPDB_PROMPT = br"\nipdb> "
+_PDB_PROMPT = br"\n\(Pdb\) "
 _IPYTHON_PROMPTS = [_IPYTHON_PROMPT1,
                     _IPYTHON_PROMPT2,
                     _PYTHON_PROMPT,
-                    _IPDB_PROMPT]
+                    _IPDB_PROMPT,
+                    _PDB_PROMPT]
 # Currently _interact_ipython() assumes the first is "In [nn]:"
 assert b"In " in _IPYTHON_PROMPTS[0]
 
@@ -3703,6 +3705,104 @@ def test_breakpoint_auto_import_p_1(enter_debugger, frontend):
     """, frontend=frontend)
 
 
+# "%debug pass" is excluded: it runs the given statement under the debugger,
+# so it legitimately stops elsewhere.
+caller_frame_debugger_activations = [
+    a for a in debugger_activations if not a.startswith("%debug")
+]
+
+# The debugger is entered on the last line of each: with no next line to stop
+# on, this is where generators and coroutines behave differently.
+caller_frame_functions = {
+    "function":  ("def f():",       [],          "f()"),
+    "generator": ("def f():",       ["yield 1"], "list(f())"),
+    "coroutine": ("async def f():", [],          "await f()"),
+}
+
+
+@pytest.mark.parametrize("kind", caller_frame_functions)
+@pytest.mark.parametrize("enter_debugger", caller_frame_debugger_activations)
+def test_breakpoint_stops_in_caller_frame_1(enter_debugger, kind, frontend):
+    # Verify that the debugger stops in the frame that entered it, the only
+    # place the local is visible. What's printed before the prompt varies:
+    # Python 3.12 and older, or another sys.monitoring tool such as
+    # coverage's, print "--Return--" first.
+    header, body, call = caller_frame_functions[kind]
+    lines = ["wallaby53023190 = 1", *body, enter_debugger]
+    cell = ("\n" + " " * 8).join("   ...:     " + line for line in lines)
+    ipython(f"""
+        In [1]: import pyflyby; pyflyby.enable_auto_importer()
+        In [2]: {header}
+        {cell}
+           ...:
+        In [3]: {call}
+        ....
+        ipdb> p wallaby53023190
+        1
+        ipdb> q
+    """, frontend=frontend)
+
+
+def test_breakpoint_stdlib_pdb_1(frontend):
+    # Verify auto importing, and the stop frame, in plain pdb: what
+    # breakpoint() uses when PYTHONBREAKPOINT is unset.
+    ipython("""
+        In [1]: import pyflyby; pyflyby.enable_auto_importer()
+        In [2]: import os; del os.environ["PYTHONBREAKPOINT"]
+        In [3]: def f():
+           ...:     wallaby53023190 = 1
+           ...:     breakpoint()
+           ...:     wallaby53023190 = 2
+           ...:
+        In [4]: f()
+        > <ipython-input>(...)f()
+        -> ...
+        (Pdb) p wallaby53023190
+        1
+        (Pdb) p b64decode("UGFzY2Fs")
+        [PYFLYBY] from base64 import b64decode
+        b'Pascal'
+        (Pdb) c
+    """, frontend=frontend)
+
+
+def test_debugger_hook_with_replaced_pdb_init_1(frontend):
+    # Verify that the auto importer still enables, and hooks the debugger,
+    # when something else has already replaced pdb.Pdb.__init__.
+    ipython("""
+        In [1]: import pdb
+        In [2]: def wrap(init):
+           ...:     def wrapper(self, *args, **kwargs):
+           ...:         init(self, *args, **kwargs)
+           ...:     return wrapper
+           ...:
+        In [3]: pdb.Pdb.__init__ = wrap(pdb.Pdb.__init__)
+        In [4]: import pyflyby; pyflyby.enable_auto_importer()
+        In [5]: %debug pass
+        ....
+        ipdb> p b64decode("V29tYmF0")
+        [PYFLYBY] from base64 import b64decode
+        b'Wombat'
+        ipdb> q
+    """, frontend=frontend)
+
+
+def test_ipdb_set_trace_context_1(frontend):
+    # Verify that the context ipdb asks for reaches its debugger.  ipdb passes
+    # it to the constructor, and falls back to the defaults on TypeError.
+    ipython("""
+        In [1]: import pyflyby; pyflyby.enable_auto_importer()
+        In [2]: def f():
+           ...:     wallaby53023190 = 1
+           ...:     wallaby53023190 = 2
+           ...:     import ipdb; ipdb.set_trace(context=1)
+           ...:     wallaby53023190 = 3
+           ...:
+        In [3]: f()
+        > <ipython-input>(...)f()
+        ipdb> q
+    """, frontend=frontend)
+
 @pytest.mark.skipif(
     (3, 14, 1) <= sys.version_info[:3] <= (3, 14, 5),
     reason="linux + >=3.14.1 show some extra stacks.",