Re: a property "definition-type" would help find macro-defined tests

[Stephen Gildea]

Eshel Yaron <me@eshelyaron.com> wrote:

>   Yeah, but the first form is not enough, since in the current
>   implementation you also need to extend find-function-regexp-alist,
>   right?

That is a compelling argument.  I'm pleased to see a design that doesn't
require modifying the global find-function-regexp-alist.  I'm sold.

Here's a complete patch implementing your design.

For the macros that use this functionality, I provide a
convenience function find-function-update-type-alist.

I also fix ERT to always look up its tests with type
'ert--test'; there was a bug where 'ert-describe-test' doesn't
set the type.  With 'definition-type' this didn't matter,
because we overrode the type.  But now that we translate, ERT
has to give us a correct starting type.

 < Stephen

diff --git a/etc/NEWS b/etc/NEWS
index 2f04204ad94..059d765fb44 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1166,11 +1166,14 @@ It offers a more concise way to create a completion 
table with metadata.
 ** 'all-completions' and 'unintern' no longer support old calling conventions.
 
 +++
-** New symbol property 'definition-type' used by 'find-function' and friends.
+** New symbol property 'find-function-type-alist' used by 'find-function' etc.
 Macros that define an object in a way that makes the object's name and
-the macro call site defining the object hard to associate can put the
-property 'definition-type' on the object's name to provide instructions
-for finding the definition.
+the macro call site defining the object hard to associate can add an
+entry to the property 'find-function-type-alist' on the object's name to
+provide instructions for finding the definition.
+
+New convenience function 'find-function-update-type-alist' offers a
+concise way to update a symbol's 'find-function-type-alist' property.
 
 
 * Changes in Emacs 31.1 on Non-Free Operating Systems
diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el
index 643b6aba2a6..f44f48515da 100644
--- a/lisp/emacs-lisp/find-func.el
+++ b/lisp/emacs-lisp/find-func.el
@@ -152,7 +152,11 @@ find-function-regexp-alist
 to be used to substitute the desired symbol name into the regexp.
 Instead of regexp variable, types can be mapped to functions as well,
 in which case the function is called with one argument (the object
-we're looking for) and it should search for it.")
+we're looking for) and it should search for it.
+
+Symbols can have their own version of this alist on
+the property `find-function-type-alist'.
+See the function `find-function-update-type-alist'.")
 (put 'find-function-regexp-alist 'risky-local-variable t)
 
 (define-obsolete-variable-alias 'find-function-source-path
@@ -402,9 +406,9 @@ find-function-search-for-symbol
 
 If TYPE is nil, look for a function definition,
 otherwise, TYPE specifies the kind of definition.
-If SYMBOL has a property `definition-type',
-the property value is used instead of TYPE.
-TYPE is interpreted via `find-function-regexp-alist'.
+TYPE is looked up in SYMBOL's property `find-function-type-alist'
+(which can be maintained with `find-function-update-type-alist')
+or the variable `find-function-regexp-alist'.
 
 The search is done in the source for library LIBRARY."
   (if (null library)
@@ -413,8 +417,6 @@ find-function-search-for-symbol
   ;; that defines something else.
   (while (and (symbolp symbol) (get symbol 'definition-name))
     (setq symbol (get symbol 'definition-name)))
-  (setq type (or (get symbol 'definition-type)
-                 type))
   (if (string-match "\\`src/\\(.*\\.\\(c\\|m\\)\\)\\'" library)
       (find-function-C-source symbol (match-string 1 library) type)
     (when (string-match "\\.el\\(c\\)\\'" library)
@@ -424,7 +426,9 @@ find-function-search-for-symbol
     (when (string-match "\\.emacs\\(.el\\)\\'" library)
       (setq library (substring library 0 (match-beginning 1))))
     (let* ((filename (find-library-name library))
-          (regexp-symbol (cdr (assq type find-function-regexp-alist))))
+          (regexp-symbol
+            (or (alist-get type (get symbol 'find-function-type-alist))
+                (alist-get type find-function-regexp-alist))))
       (with-current-buffer (find-file-noselect filename)
        (let ((regexp (if (functionp regexp-symbol) regexp-symbol
                         (format (symbol-value regexp-symbol)
@@ -466,6 +470,13 @@ find-function-search-for-symbol
                       (find-function--search-by-expanding-macros
                        (current-buffer) symbol type))))))))))
 
+;;;###autoload
+(defun find-function-update-type-alist (symbol type variable)
+  "Update SYMBOL property `find-function-type-alist' with (TYPE . VARIABLE).
+Property `find-function-type-alist' is a symbol-specific version
+of variable `find-function-regexp-alist' and has the same format."
+  (setf (alist-get type (get symbol 'find-function-type-alist)) variable))
+
 (defun find-function--try-macroexpand (form)
   "Try to macroexpand FORM in full or partially.
 This is a best-effort operation in which if macroexpansion fails,
diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index f25ba8a529c..3106b0dabca 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -2467,7 +2467,9 @@ ert-results-find-test-at-point-other-window
 
 (defun ert--test-name-button-action (button)
   "Find the definition of the test BUTTON belongs to, in another window."
-  (let ((name (button-get button 'ert-test-name)))
+  ;; work with either ert-insert-test-name-button or help-xref-button
+  (let ((name (or (button-get button 'ert-test-name)
+                  (car (button-get button 'help-args)))))
     (ert-find-test-other-window name)))
 
 (defun ert--ewoc-position (ewoc node)
@@ -2814,7 +2816,8 @@ ert-describe-test
                                       (file-name-nondirectory file-name)))
               (save-excursion
                 (re-search-backward (substitute-command-keys "`\\([^`']+\\)'"))
-                (help-xref-button 1 'help-function-def test-name file-name)))
+                (help-xref-button 1 'ert--test-name-button
+                                  test-name file-name)))
             (insert ".")
             (fill-region-as-paragraph (point-min) (point))
             (insert "\n\n")
diff --git a/doc/lispref/symbols.texi b/doc/lispref/symbols.texi
index 508ee13a244..058ed9fda14 100644
--- a/doc/lispref/symbols.texi
+++ b/doc/lispref/symbols.texi
@@ -536,9 +536,9 @@ Standard Properties
 related functions.  @xref{Variable Definitions}.
 
 @cindex @code{definition-name} (symbol property)
-@cindex @code{definition-type} (symbol property)
+@cindex @code{find-function-type-alist} (symbol property)
 @item definition-name
-@itemx definition-type
+@itemx find-function-type-alist
 These properties help find the definition of a symbol in the source
 code when it might be hard to find the definition by textual search
 of the source file.
@@ -563,16 +563,19 @@ Standard Properties
 macro to generate calls to @code{ert-deftest}
 (@pxref{,,,ert, ERT: Emacs Lisp Regression Testing}) where the code
 is boiler plate and only varying data need to be passed in.
-In such cases, the @code{definition-type} property of the symbol can
-be a symbol that has an entry in @code{find-function-regexp-alist}
+In such cases, the @code{find-function-type-alist} property of the
+symbol can be an alist that augments @code{find-function-regexp-alist}
 telling how to find the definition of symbols of this type.
 
 In the example of a macro defining calls to @code{ert-deftest},
-the macro could put the property @code{definition-type} on each
-test defined.  The file defining the macro would also define a
-definition-finding function or regexp and add it to
-@code{find-function-regexp-alist} after that variable is loaded.
-Here is an example using a function to find the definition:
+the macro could put the property @code{find-function-type-alist} on each
+test defined, associating @code{ert--test} (the internal type of ERT
+tests) with the name of a regexp or function that can find the correct
+macro call.  The file defining the macro would also have to provide that
+definition-finding function or regexp.
+Here is an example using a function to find the definition.
+The example updates the property using convenience function
+@code{find-function-update-type-alist}.
 
 @example
 @group
@@ -581,10 +584,11 @@ Standard Properties
   (declare (debug (&rest sexp)))
   (let ((test-name (intern (concat ...))))
     `(progn
-      (put ',test-name 'definition-type 'foo-test-type)
-      (ert-deftest ,test-name ()
-        ,(concat "Test foo with " ...)
-        ...))))
+       (find-function-update-type-alist
+        ',test-name 'ert--test 'foo-find-test-def-function)
+       (ert-deftest ,test-name ()
+         ,(concat "Test foo with " ...)
+         ...))))
 @end group
 
 @group
@@ -598,13 +602,6 @@ Standard Properties
         (goto-char (point-min))
         (re-search-forward regexp nil t)))))
 @end group
-
-@group
-(with-eval-after-load "find-func"
-  (add-to-list
-   'find-function-regexp-alist
-   '(foo-test-type . foo-find-test-def-function)))
-@end group
 @end example
 
 @item disabled
diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index bad3c926b27..2b5847d2f64 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -754,7 +754,7 @@ Defining Functions
 usually looks very different from the usual static calls to
 @code{defun}.  You can make the job of finding the code that
 generates such functions easier by using the @code{definition-name}
-or @code{definition-type} property, @pxref{Standard Properties}.
+or @code{find-function-type-alist} property, @pxref{Standard Properties}.
 
 @cindex override existing functions
 @cindex redefine existing functions
diff --git a/doc/lispref/tips.texi b/doc/lispref/tips.texi
index c2fba3cba8d..1bf52886971 100644
--- a/doc/lispref/tips.texi
+++ b/doc/lispref/tips.texi
@@ -228,7 +228,7 @@ Coding Conventions
 itself, since that would confuse these tools.
 If your macro cannot be written in this style, the macro can still
 help these tools find the defining call by putting the property
-@code{definition-name} or @code{definition-type} on the name.
+@code{definition-name} or @code{find-function-type-alist} on the name.
 @xref{Standard Properties}.
 
 @item
diff --git a/doc/misc/ert.texi b/doc/misc/ert.texi
index c8aac971ec7..0e03afc98ff 100644
--- a/doc/misc/ert.texi
+++ b/doc/misc/ert.texi
@@ -524,7 +524,7 @@ How to Write Tests
 
 If the test definition is generated by a macro, the macro may want to
 help ERT find the defining call to the macro by putting the property
-@code{definition-type} on the test name.
+@code{find-function-type-alist} on the test name.
 @xref{Standard Properties,,,elisp, GNU Emacs Lisp Reference Manual}.