bug#56105: 29.0.50; python-nav-beginning-of-defun does not handle nested functions properly

bug#56105: 29.0.50; python-nav-beginning-of-defun does not handle nested functions properly

[kobarity]

Hi,

The following steps will place the point at the beginning of the line
"def bar():", not the line "def foo():".

1. emacs -Q
2. Load the following Python file using M-x find-file

#+begin_src python
def foo():
    def bar():
        pass
    pass
#+end_src

3. M->
4. M-: (python-nav-beginning-of-defun)

This issue causes mark-defun/python-mark-defun to mark only the
function bar, not the function foo, if the point is located at the end
of the buffer.

Best Regards,

--

In GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu)
 of 2022-06-19 built on ubuntu
System Description: Ubuntu 22.04 LTS

Configured using:
 'configure --without-x --with-gnutls=ifavailable'

Configured features:
ACL LIBXML2 MODULES NOTIFY INOTIFY PDUMPER SECCOMP SOUND SQLITE3 THREADS
XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Python

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media rmc puny
dired dnd dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils python json map comint regexp-opt ring cl-loaddefs cl-lib
ansi-color seq term/screen term/xterm xterm gv subr-x byte-opt bytecomp
byte-compile cconv iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select mouse jit-lock font-lock
syntax font-core term/tty-colors frame minibuffer nadvice simple
cl-generic indonesian philippine cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
faces cus-face macroexp files window text-properties overlay sha1 md5
base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads inotify multi-tty
make-network-process emacs)

Memory information:
((conses 16 53843 6906)
 (symbols 48 6607 1)
 (strings 32 18599 1675)
 (string-bytes 1 607055)
 (vectors 16 10579)
 (vector-slots 8 125410 6998)
 (floats 8 27 381)
 (intervals 56 194 0)
 (buffers 992 12))

bug#56105: 29.0.50; python-nav-beginning-of-defun does not handle nested functions properly

[kobarity]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

Hi,

In python-nav--beginning-of-defun, body-indentation is expected to be
the indentation level of the body of the current function. It is
calculated by searching blocks backward for defuns and adding
python-indent-offset. However, the found defun is not always the defun
for the current function, because there may be nested functions.

Attached is a patch to fix this issue. In this patch, min-indentation
is introduced to keep track of the minimum indentation of the current
function body to distinguish nested defuns from current defun.
(python-info-looking-at-beginning-of-defun) in the initialization of
min-indentation is a special case. In this case,
python-nav-beginning-of-defun is expected to navigate to the previous
defun of the same level, not the enclosing defun.

There is a test case for this issue in
python-nav-beginning-of-defun-1, but unfortunately the expectation is
wrong. I corrected it and added some tests including a test with
python-mark-defun in this patch.

Best Regards,

[-- Attachment #2: fix-56105.patch --]
[-- Type: application/octet-stream, Size: 4166 bytes --]

commit 4c3a0249efdb14acf63eb3a8e37034354fa3fe7e
Author: kobarity <kobarity@gmail.com>
Date:   Mon Jun 20 20:53:09 2022 +0900

    Fix nested defuns handling in `python-nav-beginning-of-defun'
    
    * lisp/progmodes/python.el (python-nav--beginning-of-defun): Fix
    handling of nested defuns (bug#56105).

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index c2483436fe..e0c937d7ce 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1455,11 +1455,17 @@ python-nav--beginning-of-defun
          (line-beg-pos (line-beginning-position))
          (line-content-start (+ line-beg-pos (current-indentation)))
          (pos (point-marker))
+         (min-indentation (+ (current-indentation)
+                             (if (python-info-looking-at-beginning-of-defun)
+                                 python-indent-offset 0)))
          (body-indentation
           (and (> arg 0)
                (save-excursion
                  (while (and
-                         (not (python-info-looking-at-beginning-of-defun))
+                         (or (not (python-info-looking-at-beginning-of-defun))
+                             (>= (current-indentation) min-indentation))
+                         (setq min-indentation
+                               (min min-indentation (current-indentation)))
                          (python-nav-backward-block)))
                  (or (and (python-info-looking-at-beginning-of-defun)
                           (+ (current-indentation) python-indent-offset))
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index 8db0a07170..e17bc0df92 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -1736,6 +1736,27 @@ python-mark-defun-3
      (should (= (marker-position (mark-marker))
                 expected-mark-end-position)))))
 
+(ert-deftest python-mark-defun-4 ()
+  "Test `python-mark-defun' with nested functions."
+  (python-tests-with-temp-buffer
+   "
+def foo(x):
+    def bar():
+        return x
+    if True:
+        return bar
+"
+   (let ((expected-mark-beginning-position
+          (progn
+            (python-tests-look-at "def foo(x):")
+            (1- (line-beginning-position))))
+         (expected-mark-end-position (point-max)))
+     (python-tests-look-at "return bar")
+     (python-mark-defun 1)
+     (should (= (point) expected-mark-beginning-position))
+     (should (= (marker-position (mark-marker))
+                expected-mark-end-position)))))
+
 \f
 ;;; Navigation
 
@@ -1762,12 +1783,20 @@ python-nav-beginning-of-defun-1
         return wrapped_f
     return wwrap
 "
-   (python-tests-look-at "return wrap")
+   (python-tests-look-at "return wwrap")
    (should (= (save-excursion
                 (python-nav-beginning-of-defun)
                 (point))
               (save-excursion
-                (python-tests-look-at "def wrapped_f(*args):" -1)
+                (python-tests-look-at "def decoratorFunctionWithArguments" -1)
+                (beginning-of-line)
+                (point))))
+   (python-tests-look-at "return wrap" -1)
+   (should (= (save-excursion
+                (python-nav-beginning-of-defun)
+                (point))
+              (save-excursion
+                (python-tests-look-at "def wwrap(f):" -1)
                 (beginning-of-line)
                 (point))))
    (python-tests-look-at "def wrapped_f(*args):" -1)
@@ -1801,11 +1830,23 @@ python-nav-beginning-of-defun-2
         def a():
             pass
 
+        if True:
+            return a
+
     def c(self):
         pass
 "
    ;; Nested defuns, are handled with care.
    (python-tests-look-at "def c(self):")
+   (should (= (save-excursion
+                (python-nav-beginning-of-defun)
+                (point))
+              (save-excursion
+                (python-tests-look-at "def m(self):" -1)
+                (beginning-of-line)
+                (point))))
+   ;; Nested defuns shuld be skipped.
+   (python-tests-look-at "return a" -1)
    (should (= (save-excursion
                 (python-nav-beginning-of-defun)
                 (point))

bug#56105: 29.0.50; python-nav-beginning-of-defun does not handle nested functions properly

[Lars Ingebrigtsen]

kobarity <kobarity@gmail.com> writes:

>
> Attached is a patch to fix this issue. In this patch, min-indentation
> is introduced to keep track of the minimum indentation of the current
> function body to distinguish nested defuns from current defun.
> (python-info-looking-at-beginning-of-defun) in the initialization of
> min-indentation is a special case. In this case,
> python-nav-beginning-of-defun is expected to navigate to the previous
> defun of the same level, not the enclosing defun.

Thanks; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no