bug#71068: 30.0.50; Incorrect xref positions for eglot-execute

[Eshel Yaron]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi! Thanks for the report.
>
> On 19/05/2024 18:55, Eshel Yaron via Bug reports for GNU Emacs, the
> Swiss army knife of text editors wrote:
>> Using xref to find the definition of eglot-execute seems to yield
>> imprecise definition locations:
>> 1. emacs -Q
>> 2. (require 'eglot)
>> 3. M-. eglot-execute RET
>> This produces an*xref*  buffer that lists two definitions:
>> --8<---------------cut here---------------start------------->8---
>> .../lisp/progmodes/eglot.el
>>    (cl-defgeneric eglot-execute)
>>    (cl-defmethod eglot-execute (server action))
>> --8<---------------cut here---------------end--------------->8---
>> Following the first definition leads to the definition of
>> eglot-execute-command (which is different from eglot-execute, although
>> the former is defined right above the latter), while the second
>> definition leads to the top of the file.
>
> I've (hopefully) fixed the first problem just now in commit 2a12f39ffe8.

Thanks, that works well.

> Regarding the second one, looks like it's using a more advanced syntax
> that our functions haven't been taught about.
>
> Since the method has been defined through a :method property on
> defgeneric, I suppose cl--generic-search-method should be taught to
> search for such definitions too.

Yes, seems so.  Here's a lightly tested attempt at that:

diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index c08441ca37f..dc127552ed2 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -1066,24 +1066,32 @@ cl-find-method
 (defun cl--generic-search-method (met-name)
   "For `find-function-regexp-alist'.  Search for a `cl-defmethod'.
 MET-NAME is as returned by `cl--generic-load-hist-format'."
-  (let ((base-re (concat "(\\(?:cl-\\)?defmethod[ \t]+"
-                         (regexp-quote (format "%s" (car met-name)))
-                        "\\_>")))
-    (or
-     (re-search-forward
-      (concat base-re "[^&\"\n]*"
-              (mapconcat (lambda (qualifier)
-                           (regexp-quote (format "%S" qualifier)))
-                         (cadr met-name)
-                         "[ \t\n]*")
-              (mapconcat (lambda (specializer)
-                           (regexp-quote
-                            (format "%S" (if (consp specializer)
-                                             (nth 1 specializer) 
specializer))))
-                         (remq t (cddr met-name))
-                         "[ \t\n]*)[^&\"\n]*"))
-      nil t)
-     (re-search-forward base-re nil t))))
+  (let* ((name (format "%s" (car met-name)))
+         (base-re (concat "(\\(?:cl-\\)?defmethod[ \t]+"
+                          (regexp-quote name)
+                         "\\_>"))
+         (search (lambda (base &optional bound)
+                   (re-search-forward
+                    (concat base "[^&\"\n]*"
+                            (mapconcat (lambda (qualifier)
+                                         (regexp-quote (format "%S" 
qualifier)))
+                                       (cadr met-name)
+                                       "[ \t\n]*")
+                            (mapconcat (lambda (specializer)
+                                         (regexp-quote
+                                          (format "%S" (if (consp specializer)
+                                                           (nth 1 specializer)
+                                                         specializer))))
+                                       (remq t (cddr met-name))
+                                       "[ \t\n]*)[^&\"\n]*"))
+                    bound t))))
+    (or (and (re-search-forward (format cl--generic-find-defgeneric-regexp 
name)
+                                nil t)
+             (or (funcall search ":method[ \t\n]+"
+                          (save-excursion (end-of-defun) (point)))
+                 (not (goto-char (point-min)))))
+        (funcall search base-re)
+        (re-search-forward base-re nil t))))
 
 ;; WORKAROUND: This can't be a defconst due to bug#21237.
 (defvar cl--generic-find-defgeneric-regexp "(\\(?:cl-\\)?defgeneric[ 
\t]+%s\\_>")