Re: Adding support for xref jumping to headers/interfaces

[Dmitry Gutov <dgutov@yandex.ru>]

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

On 17/05/2023 00:10, Spencer Baugh wrote:
> An option I didn't mention originally: Perhaps xref-find-definitions
> could just show both implementation and interface.  That would remove
> the need for any new keybindings.  The UI might be worse but perhaps
> it could be improved.
> 
> This is inspired by this comment:
> https://github.com/joaotavora/eglot/issues/302#issuecomment-534202561
> 
> The relevant excerpt:
>> By the way, on a tangent, I think it's a mistake on LSP's part to
>> import C++/Java's ecosystem of possible symbol loci. In Common Lisp,
>> there are many different types of construct associated with a symbol
>> and they are all "definitions". A good IDE like SLIME will use
>> something akin to xref-find-definitions (indeed xref is modelled after
>> SLIME) and categorize them properly in the result buffer if there is
>> more than one. So the command should have been textDocument/definition
>> with some kind of subtype parameter, or at least this is where xref
>> should evolve to, i.e. xref should keep the single command
>> xref-find-definitions and maybe augment it with some "definition-type"
>> parameter.

Perhaps it /should/ (xref-find-definitions to include both 
implementation and interface). And include the typeDefinition entries 
for langs that have those. Etc.

Would the UI be worse? Let's try to find out.

Attached is the patch along the lines that we discussed: a new command 
xref-find-extra currently bound to M-' just for the sake of this 
experiment (eliminating the need to set up define-key to try out the 
"fast" workflow). It uses the symbol at point or asks for it, then polls 
the backend for available kinds, does completing-read and asks the 
backend for definitions of that kind. And shows the list or jumps to the 
unique one.

To have something to test, I implemented this for Elisp. However, all 
known kinds are already printed by the regular xref-find-definitions 
output in that backend. So the result turned out interesting, but a 
little impractical: with Elisp, we already make the choice between the 
kinds in the Xref output buffer, when that buffer is shown at all (as 
Eli pointed out, actually). So... we could proceed in that direction and 
recommend that all kinds of definitions would be added there.

But let's try to answer the question: which workflows would the attached 
approach work better for? I could give one example: in Java, you 
sometimes (often?) want to find the locations where the current 
definition near point is overridden by subclasses. We could call it 
"find overrides", for example. But it does not just filter by kind (e.g. 
method overrides), it also filters by class hierarchy, i.e. showing only 
definitions in the subclasses of the current class, or interface.

Examples of such searches:
 
https://stackoverflow.com/questions/11849631/how-to-find-the-overridden-method-in-eclipse
   https://stackoverflow.com/questions/1143267/finding-overriding-methods

Is this still something people care about? Does jdt-ls even support this?

Or perhaps the main value would be in actually having separate commands 
which could be bound to a submap with faster key sequences and/or to the 
context menu (with context-menu-mode on). Then our solution should be 
more in line with either defining a number of additional named commands 
(for mode universal kinds) and/or making it easier to define new such 
commands for users/3rd-party packages/etc.

Please try out the attached (with implementation for Eglot hopefully 
coming soon) and let me know your thoughts (you all).

[-- Attachment #2: xref-find-extras.diff --]
[-- Type: text/x-patch, Size: 8315 bytes --]

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index ff90a744ea3..ead8b618599 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1037,6 +1037,26 @@ xref-backend-definitions
                 (elisp--xref-filter-definitions defs 'any sym))
           (elisp--xref-filter-definitions defs namespace sym))))))
 
+(cl-defmethod xref-backend-extra-kinds ((_backend (eql 'elisp)) identifier)
+  (require 'find-func)
+  (let ((sym (intern-soft identifier)))
+    (when sym
+      (let* ((pos (get-text-property 0 'pos identifier))
+             (namespace (if pos
+                            (elisp--xref-infer-namespace pos)
+                          'any))
+             (defs (elisp--xref-find-definitions sym)))
+        (pcase-exhaustive namespace
+          ('function '(function defalias define-type
+                       cl-defgeneric cl-defmethod))
+          ('variable '(defvar))
+          ('face '(defface))
+          ('feature '(feature)))
+        (if (eq namespace 'maybe-variable)
+            (or (elisp--xref-filter-definitions defs 'variable sym)
+                (elisp--xref-filter-definitions defs 'any sym))
+          (elisp--xref-filter-definitions defs namespace sym))))))
+
 (defun elisp--xref-filter-definitions (definitions namespace symbol)
   (if (eq namespace 'any)
       (if (memq symbol minor-mode-list)
@@ -1212,6 +1232,66 @@ elisp--xref-find-definitions
 
     xrefs))
 
+(cl-defmethod xref-backend-extra-kinds ((_backend (eql 'elisp)) identifier)
+  ;; The file name is not known when `symbol' is defined via interactive eval.
+  (let ((symbol (intern-soft identifier))
+        kinds)
+    ;; alphabetical by result type symbol
+
+    ;; FIXME: advised function; list of advice functions
+    ;; FIXME: aliased variable
+
+    ;; Coding system symbols do not appear in ‘load-history’,
+    ;; so we can’t get a location for them.
+    (when (and (symbolp symbol)
+               (symbol-function symbol)
+               (symbolp (symbol-function symbol)))
+      (push "defalias" kinds))
+
+    (when (facep symbol)
+      (push "face" kinds))
+
+    (when (fboundp symbol)
+      (let ((file (find-lisp-object-file-name symbol (symbol-function symbol)))
+            doc)
+        (when file
+          (cond
+           ((eq file 'C-source)
+            (push "function" kinds))
+           ((and (setq doc (documentation symbol t))
+                 ;; This doc string is defined in cl-macs.el cl-defstruct
+                 (string-match "Constructor for objects of type `\\(.*\\)'" doc))
+            (push "constructor" kinds))
+           ((cl--generic symbol)
+            (push "generic" kinds))
+           (t
+            (push "function" kinds))))))
+    (when (boundp symbol)
+      (push "variable" kinds))
+    (when (featurep symbol)
+      (push "feature" kinds))
+    (nreverse kinds)))
+
+(cl-defmethod xref-backend-extra-defs ((_backend (eql 'elisp)) identifier kind)
+  (require 'find-func)
+  (let ((sym (intern-soft identifier)))
+    (when sym
+      (let* ((defs (elisp--xref-find-definitions sym))
+             (expected-kind
+              (assoc-default kind
+                             '(("defalias" . defalias)
+                               ("face" . defface)
+                               ("function" . nil)
+                               ("variable" . defvar)
+                               ("constructor" . define-type)
+                               ("generic" . generic)))))
+        (cl-loop for d in defs
+                 for def-kind = (xref-elisp-location-type (xref-item-location d))
+                 when (if (eq expected-kind 'generic)
+                          (memq def-kind '(cl-defgeneric cl-defmethod))
+                        (eq def-kind expected-kind))
+                 collect d)))))
+
 (declare-function xref-apropos-regexp "xref" (pattern))
 
 (cl-defmethod xref-backend-apropos ((_backend (eql 'elisp)) pattern)
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 81618428bf3..e1e3862256c 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -314,6 +314,17 @@ xref-backend-identifier-completion-ignore-case
   "Return t if case is not significant in identifier completion."
   completion-ignore-case)
 
+(cl-defgeneric xref-backend-extra-kinds (_backend _identifier)
+  "Return the other definition types BACKEND could show for IDENTIFIER."
+  (user-error "Extra definitions not supported by the backend"))
+
+(cl-defgeneric xref-backend-extra-defs (_backend _identifier _kind)
+  "Find definitions of extra KIND for IDENTIFIER.
+
+The result must be a list of xref objects.  Refer to
+`xref-backend-definitions' for other details."
+  nil)
+
 \f
 ;;; misc utilities
 (defun xref--alistify (list key)
@@ -364,7 +375,8 @@ xref-marker-ring-length
 
 (defcustom xref-prompt-for-identifier '(not xref-find-definitions
                                             xref-find-definitions-other-window
-                                            xref-find-definitions-other-frame)
+                                            xref-find-definitions-other-frame
+                                            xref-find-extra)
   "If non-nil, prompt for the identifier to find.
 
 When t, always prompt for the identifier name.
@@ -1569,11 +1581,11 @@ xref--find-definitions
    (xref--create-fetcher id 'definitions id)
    display-action))
 
-(defun xref--create-fetcher (input kind arg)
+(defun xref--create-fetcher (input kind &rest args)
   "Return an xref list fetcher function.
 
 It revisits the saved position and delegates the finding logic to
-the xref backend method indicated by KIND and passes ARG to it."
+the xref backend method indicated by KIND and passes ARGS to it."
   (let* ((orig-buffer (current-buffer))
          (orig-position (point))
          (backend (xref-find-backend))
@@ -1589,7 +1601,7 @@ xref--create-fetcher
         (when (buffer-live-p orig-buffer)
           (set-buffer orig-buffer)
           (ignore-errors (goto-char orig-position)))
-        (let ((xrefs (funcall method backend arg)))
+        (let ((xrefs (apply method backend args)))
           (unless xrefs
             (xref--not-found-error kind input))
           xrefs)))))
@@ -1624,6 +1636,35 @@ xref-find-definitions-other-frame
   (interactive (list (xref--read-identifier "Find definitions of: ")))
   (xref--find-definitions identifier 'frame))
 
+;;;###autoload
+(defun xref-find-extra (identifier)
+  "Find some specific kind of definition of the identifier at point.
+With prefix argument or when there's no identifier at point,
+prompt for the identifier.
+
+If only one location is found, display it in the selected window.
+Otherwise, display the list of the possible definitions in a
+buffer where the user can select from the list.
+
+Use \\[xref-go-back] to return back to where you invoked this command."
+  (interactive (list
+                ;; XXX: Choose kind of "extra" first? That would fail
+                ;; to take advantage of the symbol-at-point, though.
+                (xref--read-identifier "Find definitions of: ")))
+  (let* ((kinds (xref-backend-extra-kinds (xref-find-backend) identifier))
+         ;; FIXME: We should probably skip asking when there's just
+         ;; one available kind, but let's keep completing-read while
+         ;; collecting the initial feedback about the interface.
+         (kind ;; (if (cdr kinds)
+          (completing-read "Definition kind: " kinds nil t nil nil (car kinds))
+          ;; (car kinds)
+          ;; )
+          ))
+    (unless kind (user-error "No supported kinds"))
+    (xref--show-defs
+     (xref--create-fetcher identifier 'extra-defs identifier kind)
+     nil)))
+
 ;;;###autoload
 (defun xref-find-references (identifier)
   "Find references to the identifier at point.
@@ -1724,6 +1765,7 @@ xref-apropos-regexp
 ;;;###autoload (define-key esc-map [?\C-,] #'xref-go-forward)
 ;;;###autoload (define-key esc-map "?" #'xref-find-references)
 ;;;###autoload (define-key esc-map [?\C-.] #'xref-find-apropos)
+;;;###autoload (define-key esc-map "'" #'xref-find-extra)
 ;;;###autoload (define-key ctl-x-4-map "." #'xref-find-definitions-other-window)
 ;;;###autoload (define-key ctl-x-5-map "." #'xref-find-definitions-other-frame)