bug#67523: check-declare doesn't account for shorthands

[João Távora]

On Wed, Nov 29, 2023 at 10:35 AM João Távora <joaotavora@gmail.com> wrote:
>
> On Wed, Nov 29, 2023 at 9:56 AM João Távora <joaotavora@gmail.com> wrote:
> >
> > On Wed, Nov 29, 2023 at 9:12 AM Joseph Turner <joseph@ushin.org> wrote:
> >
> > > A potential solution could be to convert the longhand symbol into its
> > > shorthand form and pass that into re-search-forward.  This is tricky
> > > since there may be multiple different shorthands which could yield the
> > > same longhand form.  It might be more feasible to run re-search-forward
> > > on a known common suffix portion of the symbol name, then with point on
> > > the suspected definition, run `intern-soft' to get the full symbol name.
> >
> > No, this is brittle.  Check-declare, if it's to be useful (is it?)
> > is probably meant to be as precise as possible.
> >
> > > A workaround is to not use shorthands in function definitions.
> >
> > That's letting the bad guys win :-)
> >
> > > Thoughts?
> >
> > As usual, my thoughts are that tools that read Lisp code
> > should use the Lisp reader, not regular expressions.
> >
> > Here, check-declare should just walk the whole file.
>
> Or maybe just this 100% guaranteed untested patch would work:

Sorry, that was 100% untested indeed.  This patch seems to work:

diff --git a/lisp/emacs-lisp/check-declare.el b/lisp/emacs-lisp/check-declare.el
index c887d95210c..bc3844ca9be 100644
--- a/lisp/emacs-lisp/check-declare.el
+++ b/lisp/emacs-lisp/check-declare.el
@@ -145,21 +145,26 @@ check-declare-verify
     (if (file-regular-p fnfile)
         (with-temp-buffer
           (insert-file-contents fnfile)
+          (unless cflag
+            ;; for syntax and shorthands
+            (lisp-data-mode)
+            (hack-local-variables))
           ;; defsubst's don't _have_ to be known at compile time.
-          (setq re (format (if cflag
-                               "^[ \t]*\\(DEFUN\\)[ \t]*([ \t]*\"%s\""
-                             "^[ \t]*(\\(fset[ \t]+'\\|\
+          (setq re (if cflag
+                       (format "^[ \t]*\\(DEFUN\\)[ \t]*([ \t]*\"%s\""
+                               (regexp-opt (mapcar 'cadr fnlist) t))
+                     "^[ \t]*(\\(fset[ \t]+'\\|\
 cl-def\\(?:generic\\|method\\|un\\)\\|\
 def\\(?:un\\|subst\\|foo\\|method\\|class\\|\
 ine-\\(?:derived\\|generic\\|\\(?:global\\(?:ized\\)?-\\)?minor\\)-mode\\|\
 \\(?:ine-obsolete-function-\\)?alias[ \t]+'\\|\
 ine-overloadable-function\\)\\)\
-[ \t]*%s\\([ \t;]+\\|$\\)")
-                           (regexp-opt (mapcar 'cadr fnlist) t)))
+[ \t]*\\(\\(?:\\sw\\|\\s_\\)+\\)\\([ \t;]+\\|$\\)"))
           (while (re-search-forward re nil t)
             (skip-chars-forward " \t\n")
-            (setq fn (match-string 2)
-                  type (match-string 1)
+            (setq fn (symbol-name (car (read-from-string (match-string 2)))))
+            (when (member fn (mapcar 'cadr fnlist))
+            (setq type (match-string 1)
                   ;; (min . max) for a fixed number of arguments, or
                   ;; arglists with optional elements.
                   ;; (min) for arglists with &rest.
@@ -202,7 +207,7 @@ check-declare-verify
                             (t
                              'err))
                   ;; alist of functions and arglist signatures.
-                  siglist (cons (cons fn sig) siglist)))))
+                  siglist (cons (cons fn sig) siglist))))))
     (dolist (e fnlist)
       (setq arglist (nth 2 e)
             type