elisp--xref-find-definitions handle cl-defstuct default constructor

* lisp/progmodes/elisp-mode.el (elisp-xref-find): Add FIXME.
(elisp--xref-format-extra): Rename from elisp--xref-format-cl-defmethod.
(elisp--xref-find-definitions): Handle cl-defstuct default constructor.

* test/automated/elisp-mode-tests.el (xref-elisp-test-run): Split out
from xref-elisp-test for ease of debugging.
(xref-elisp-deftest): Rename from xref-elisp-test.
(find-defs-constructor): New test.
(find-defs-defgeneric-el): Match batch test config.
(compile): Required for find-defs compilation-minor-mode test.
(find-defs-defvar-el): Match code change.
(find-defs-face-el): Match code change.

* lisp/progmodes/xref.el (xref-find-function, xref-find-definitions):
Improve doc string.

3 files changed, 92 insertions, 69 deletions

diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 41ca57f668d..7ac5a5cb778 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -590,6 +590,10 @@ It can be quoted, or be inside a quoted form."
 
 (defun elisp-xref-find (action id)
   (require 'find-func)
+  ;; FIXME: use information in source near point to filter results:
+  ;; (dvc-log-edit ...) - exclude 'feature
+  ;; (require 'dvc-log-edit) - only 'feature
+  ;; Semantic may provide additional information
   (pcase action
     (`definitions
       (let ((sym (intern-soft id)))
@@ -606,7 +610,7 @@ It can be quoted, or be inside a quoted form."
     (put-text-property 4 6 'face 'font-lock-function-name-face str)
     str))
 
-(defconst elisp--xref-format-cl-defmethod
+(defconst elisp--xref-format-extra
   (let ((str "(%s %s %s)"))
     (put-text-property 1 3 'face 'font-lock-keyword-face str)
     (put-text-property 4 6 'face 'font-lock-function-name-face str)
@@ -675,7 +679,7 @@ otherwise build the summary from TYPE and SYMBOL."
 
     (when (fboundp symbol)
       (let ((file (find-lisp-object-file-name symbol (symbol-function symbol)))
-            generic)
+            generic doc)
         (when file
           (cond
            ((eq file 'C-source)
@@ -684,11 +688,26 @@ otherwise build the summary from TYPE and SYMBOL."
             ;; Second call will return "src/*.c" in file; handled by 't' case below.
             (push (elisp--xref-make-xref nil symbol (help-C-file-name (symbol-function symbol) 'subr)) xrefs))
 
+           ((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))
+            ;; `symbol' is a name for the default constructor created by
+            ;; cl-defstruct, so return the location of the cl-defstruct.
+            (let* ((type-name (match-string 1 doc))
+                   (type-symbol (intern type-name))
+                   (file (find-lisp-object-file-name type-symbol 'define-type))
+                   (summary (format elisp--xref-format-extra
+                                    'cl-defstruct
+                                    (concat "(" type-name)
+                                    (concat "(:constructor " (symbol-name symbol) "))"))))
+              (push (elisp--xref-make-xref 'define-type type-symbol file summary) xrefs)
+              ))
+
            ((setq generic (cl--generic symbol))
             (dolist (method (cl--generic-method-table generic))
               (let* ((info (cl--generic-method-info method))
                      (met-name (cons symbol (cl--generic-method-specializers method)))
-                     (descr (format elisp--xref-format-cl-defmethod 'cl-defmethod symbol (nth 1 info)))
+                     (descr (format elisp--xref-format-extra 'cl-defmethod symbol (nth 1 info)))
                      (file (find-lisp-object-file-name met-name 'cl-defmethod)))
                 (when file
                   (push (elisp--xref-make-xref 'cl-defmethod met-name file descr) xrefs))
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 68f6cfffd2a..b0a8eb7a316 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -202,8 +202,10 @@ LOCATION is an `xref-location'."
 It can be called in several ways:
 
  (definitions IDENTIFIER): Find definitions of IDENTIFIER.  The
-result must be a list of xref objects.  If no definitions can be
-found, return nil.
+result must be a list of xref objects.  If IDENTIFIER contains
+sufficient information to determine a unique definition, returns
+only that definition. If there are multiple possible definitions,
+return all of them.  If no definitions can be found, return nil.
 
  (references IDENTIFIER): Find references of IDENTIFIER.  The
 result must be a list of xref objects.  If no references can be
@@ -751,7 +753,14 @@ Return an alist of the form ((FILENAME . (XREF ...)) ...)."
 (defun xref-find-definitions (identifier)
   "Find the definition of the identifier at point.
 With prefix argument or when there's no identifier at point,
-prompt for it."
+prompt for it.
+
+If the backend has sufficient information to determine a unique
+definition for IDENTIFIER, it returns only that definition. If
+there are multiple possible definitions, it returns all of them.
+
+If the backend returns one definition, jump to it; otherwise,
+display the list in a buffer."
   (interactive (list (xref--read-identifier "Find definitions of: ")))
   (xref--find-definitions identifier nil))
 
diff --git a/test/automated/elisp-mode-tests.el b/test/automated/elisp-mode-tests.el
index 114b71cfc63..9b4014a8a55 100644
--- a/test/automated/elisp-mode-tests.el
+++ b/test/automated/elisp-mode-tests.el
@@ -176,28 +176,30 @@
       )))
 
 
-(defmacro xref-elisp-test (name computed-xrefs expected-xrefs)
+(defun xref-elisp-test-run (xrefs expecteds)
+  (while xrefs
+    (should (= (length xrefs) (length expecteds)))
+    (let ((xref (pop xrefs))
+          (expected (pop expecteds)))
+
+      (should (equal xref
+                     (or (when (consp expected) (car expected)) expected)))
+
+      (xref--goto-location (xref-item-location xref))
+      (should (looking-at (or (when (consp expected) (cdr expected))
+                              (xref-elisp-test-descr-to-target expected)))))
+    ))
+
+(defmacro xref-elisp-deftest (name computed-xrefs expected-xrefs)
   "Define an ert test for an xref-elisp feature.
 COMPUTED-XREFS and EXPECTED-XREFS are lists of xrefs, except if
 an element of EXPECTED-XREFS is a cons (XREF . TARGET), TARGET is
 matched to the found location; otherwise, match
 to (xref-elisp-test-descr-to-target xref)."
-  (declare (indent defun))
-  (declare (debug (symbolp "name")))
+  (declare (indent defun)
+           (debug (symbolp "name")))
   `(ert-deftest ,(intern (concat "xref-elisp-test-" (symbol-name name))) ()
-     (let ((xrefs ,computed-xrefs)
-           (expecteds ,expected-xrefs))
-       (while xrefs
-         (let ((xref (pop xrefs))
-               (expected (pop expecteds)))
-
-           (should (equal xref
-                          (or (when (consp expected) (car expected)) expected)))
-
-           (xref--goto-location (xref-item-location xref))
-           (should (looking-at (or (when (consp expected) (cdr expected))
-                                   (xref-elisp-test-descr-to-target expected)))))
-         ))
+     (xref-elisp-test-run ,computed-xrefs ,expected-xrefs)
      ))
 
 ;; When tests are run from the Makefile, 'default-directory' is $HOME,
@@ -212,7 +214,22 @@ to (xref-elisp-test-descr-to-target xref)."
 ;; FIXME: defalias-defun-c cmpl-prefix-entry-head
 ;; FIXME: defalias-defvar-el allout-mode-map
 
-(xref-elisp-test find-defs-defalias-defun-el
+(xref-elisp-deftest find-defs-constructor
+  (elisp--xref-find-definitions 'xref-make-elisp-location)
+  ;; 'xref-make-elisp-location' is just a name for the default
+  ;; constructor created by the cl-defstruct, so the location is the
+  ;; cl-defstruct location.
+  (list
+   (cons
+    (xref-make "(cl-defstruct (xref-elisp-location (:constructor xref-make-elisp-location)))"
+               (xref-make-elisp-location
+                'xref-elisp-location 'define-type
+                (expand-file-name "../../lisp/progmodes/elisp-mode.el" emacs-test-dir)))
+    ;; It's not worth adding another special case to `xref-elisp-test-descr-to-target' for this
+    "(cl-defstruct (xref-elisp-location")
+   ))
+
+(xref-elisp-deftest find-defs-defalias-defun-el
   (elisp--xref-find-definitions 'Buffer-menu-sort)
   (list
    (xref-make "(defalias Buffer-menu-sort)"
@@ -227,7 +244,7 @@ to (xref-elisp-test-descr-to-target xref)."
 
 ;; FIXME: defconst
 
-(xref-elisp-test find-defs-defgeneric-el
+(xref-elisp-deftest find-defs-defgeneric-el
   (elisp--xref-find-definitions 'xref-location-marker)
   (list
    (xref-make "(cl-defgeneric xref-location-marker)"
@@ -250,20 +267,14 @@ to (xref-elisp-test-descr-to-target xref)."
               (xref-make-elisp-location
                '(xref-location-marker xref-bogus-location) 'cl-defmethod
                (expand-file-name "../../lisp/progmodes/xref.el" emacs-test-dir)))
-   (xref-make "(cl-defmethod xref-location-marker ((l xref-etags-location)))"
-              (xref-make-elisp-location
-               '(xref-location-marker xref-etags-location) 'cl-defmethod
-               (expand-file-name "../../lisp/progmodes/etags.el" emacs-test-dir)))
+   ;; etags is not loaded at test time
    ))
 
-;; FIXME: constructor xref-make-elisp-location; location is
-;; cl-defstruct location. use :constructor in description.
-
-(xref-elisp-test find-defs-defgeneric-eval
+(xref-elisp-deftest find-defs-defgeneric-eval
   (elisp--xref-find-definitions (eval '(cl-defgeneric stephe-leake-cl-defgeneric ())))
   nil)
 
-(xref-elisp-test find-defs-defun-el
+(xref-elisp-deftest find-defs-defun-el
   (elisp--xref-find-definitions 'xref-find-definitions)
   (list
    (xref-make "(defun xref-find-definitions)"
@@ -271,11 +282,11 @@ to (xref-elisp-test-descr-to-target xref)."
                'xref-find-definitions nil
                (expand-file-name "../../lisp/progmodes/xref.el" emacs-test-dir)))))
 
-(xref-elisp-test find-defs-defun-eval
+(xref-elisp-deftest find-defs-defun-eval
   (elisp--xref-find-definitions (eval '(defun stephe-leake-defun ())))
   nil)
 
-(xref-elisp-test find-defs-defun-c
+(xref-elisp-deftest find-defs-defun-c
   (elisp--xref-find-definitions 'buffer-live-p)
   (list
    (xref-make "(defun buffer-live-p)"
@@ -283,7 +294,7 @@ to (xref-elisp-test-descr-to-target xref)."
 
 ;; FIXME: deftype
 
-(xref-elisp-test find-defs-defun-c-defvar-c
+(xref-elisp-deftest find-defs-defun-c-defvar-c
   (elisp-xref-find 'definitions "system-name")
   (list
    (xref-make "(defvar system-name)"
@@ -292,7 +303,7 @@ to (xref-elisp-test-descr-to-target xref)."
               (xref-make-elisp-location 'system-name nil "src/editfns.c")))
   )
 
-(xref-elisp-test find-defs-defun-el-defvar-c
+(xref-elisp-deftest find-defs-defun-el-defvar-c
   (elisp-xref-find 'definitions "abbrev-mode")
   ;; It's a minor mode, but the variable is defined in buffer.c
   (list
@@ -310,42 +321,34 @@ to (xref-elisp-test-descr-to-target xref)."
 ;; compilation-minor-mode". There is no way to tell that from the
 ;; symbol.  find-function-regexp-alist uses find-function-regexp for
 ;; this, but that matches too many things for use in this test.
-(xref-elisp-test find-defs-defun-defvar-el
+(require 'compile) ;; not loaded by default at test time
+(xref-elisp-deftest find-defs-defun-defvar-el
   (elisp--xref-find-definitions 'compilation-minor-mode)
   (list
    (cons
-    (xref-make "(defun compilation-minor-mode)"
-               (xref-make-elisp-location
-                'compilation-minor-mode nil
-                (expand-file-name "../../lisp/progmodes/compile.el" emacs-test-dir)))
-    "(define-minor-mode compilation-minor-mode")
-   (cons
     (xref-make "(defvar compilation-minor-mode)"
               (xref-make-elisp-location
                'compilation-minor-mode 'defvar
                (expand-file-name "../../lisp/progmodes/compile.el" emacs-test-dir)))
     "(define-minor-mode compilation-minor-mode")
-   )
-  )
+   (cons
+    (xref-make "(defun compilation-minor-mode)"
+               (xref-make-elisp-location
+                'compilation-minor-mode nil
+                (expand-file-name "../../lisp/progmodes/compile.el" emacs-test-dir)))
+    "(define-minor-mode compilation-minor-mode")
+   ))
 
-(xref-elisp-test find-defs-defvar-el
+(xref-elisp-deftest find-defs-defvar-el
   (elisp--xref-find-definitions 'xref--marker-ring)
-  ;; This is a defconst, which creates an alias and a variable.
-  ;; FIXME: try not to show the alias in this case
   (list
    (xref-make "(defvar xref--marker-ring)"
               (xref-make-elisp-location
                'xref--marker-ring 'defvar
                (expand-file-name "../../lisp/progmodes/xref.el" emacs-test-dir)))
-   (cons
-    (xref-make "(defalias xref--marker-ring)"
-               (xref-make-elisp-location
-                'xref--marker-ring 'defalias
-                (expand-file-name "../../lisp/progmodes/xref.elc" emacs-test-dir)))
-    "(defvar xref--marker-ring")
     ))
 
-(xref-elisp-test find-defs-defvar-c
+(xref-elisp-deftest find-defs-defvar-c
   (elisp--xref-find-definitions 'default-directory)
   (list
    (cons
@@ -354,15 +357,13 @@ to (xref-elisp-test-descr-to-target xref)."
     ;; IMPROVEME: we might be able to compute this target
     "DEFVAR_PER_BUFFER (\"default-directory\"")))
 
-(xref-elisp-test find-defs-defvar-eval
+(xref-elisp-deftest find-defs-defvar-eval
   (elisp--xref-find-definitions (eval '(defvar stephe-leake-defvar nil)))
   nil)
 
-(xref-elisp-test find-defs-face-el
+(xref-elisp-deftest find-defs-face-el
   (elisp--xref-find-definitions 'font-lock-keyword-face)
   ;; 'font-lock-keyword-face is both a face and a var
-  ;; defface creates both a face and an alias
-  ;; FIXME: try to not show the alias in this case
   (list
    (xref-make "(defvar font-lock-keyword-face)"
               (xref-make-elisp-location
@@ -372,19 +373,13 @@ to (xref-elisp-test-descr-to-target xref)."
               (xref-make-elisp-location
                'font-lock-keyword-face 'defface
                (expand-file-name "../../lisp/font-lock.el" emacs-test-dir)))
-   (cons
-    (xref-make "(defalias font-lock-keyword-face)"
-              (xref-make-elisp-location
-               'font-lock-keyword-face 'defalias
-               (expand-file-name "../../lisp/font-lock.elc" emacs-test-dir)))
-    "(defface font-lock-keyword-face")
    ))
 
-(xref-elisp-test find-defs-face-eval
+(xref-elisp-deftest find-defs-face-eval
   (elisp--xref-find-definitions (eval '(defface stephe-leake-defface nil "")))
   nil)
 
-(xref-elisp-test find-defs-feature-el
+(xref-elisp-deftest find-defs-feature-el
   (elisp--xref-find-definitions 'xref)
   (list
    (xref-make "(feature xref)"
@@ -392,7 +387,7 @@ to (xref-elisp-test-descr-to-target xref)."
                'xref 'feature
                (expand-file-name "../../lisp/progmodes/xref.el" emacs-test-dir)))))
 
-(xref-elisp-test find-defs-feature-eval
+(xref-elisp-deftest find-defs-feature-eval
   (elisp--xref-find-definitions (eval '(provide 'stephe-leake-feature)))
   nil)