Subject: Re: bug#78543: 29.2; checkdoc misses some CL (cl-lib) constructs.

From: Eli Zaretskii <[email-masked]>
Date: Sat, 31 May 2025 14:07:46 +0300

> From: Marco Antoniotti <[email-masked]>
> Date: Wed, 21 May 2025 22:38:48 +0200
> 
> The problem is with `checkdoc` not correctly handling some docstring
> in `cl-lib` style declarations.
> 
> I have a function like
> 
> ```
> (cl-defun funky-keys (&key ((:this that) 42) &aux (the-beast 666))
>     "Do something with THAT (passed via :THIS)"
>     ...
>     )
> ```
> 
> checkdoc insists on flagging :THIS and THE-BEAST as missing from the docstring.
> 
> Another one is the following.
> 
> ```
> (cl-defgeneric gf (a s d f)
>     (:documentation "Munge A, S, D, and F."))
> ```
> 
> checkdoc does not recognize the :documentation option (which is valid according to cl-lib).
> 
> This problem is still visible in 30.1

Sigh.  This is notoriously under-documented in the cl.info manual.
After reading and re-reading the node "Argument Lists" there, I
believe that we expect :this to be referenced as "THIS" (without the
leading colon) in the doc string, since argument references in doc
strings should be words, not symbols, and ':' is not a
word-constituent character.  Also, I believe that THE-BEAST should
indeed be mentioned in the doc string.  This:

  (cl-defun funky-keys (&key ((:this that) 42) &aux (the-beast 666))
      "Do something with THAT (passed via THIS), binding THE-BEAST to 666."
      nil
      )

passes the checkdoc tests, if we install the simple patch below.

But since I'm very far from being an expert of cl-defun and its
correct usage, I invite CL experts to chime in and provide their
opinions.

Here's the patch which I will install barring any objections:

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 355a0c5..f9fab0b 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1761,24 +1761,31 @@ checkdoc-this-string-valid-engine
 
 	     ;;   Addendum:  Make sure they appear in the doc in the same
 	     ;;              order that they are found in the arg list.
-	     (let ((args (nthcdr 4 fp))
-		   (last-pos 0)
-		   (found 1)
-		   (order (and (nth 3 fp) (car (nth 3 fp))))
-		   (nocheck (append '("&optional" "&rest" "&key" "&aux"
-                                      "&context" "&environment" "&whole"
-                                      "&body" "&allow-other-keys" "nil")
-                                    (nth 3 fp)))
+	     (let* ((args (nthcdr 4 fp))
+                    (this-arg (car args))
+                    (this-arg (if (string-prefix-p ":" this-arg)
+                                  (substring this-arg 1)
+                                this-arg))
+		    (last-pos 0)
+		    (found 1)
+		    (order (and (nth 3 fp) (car (nth 3 fp))))
+		    (nocheck (append '("&optional" "&rest" "&key" "&aux"
+                                       "&context" "&environment" "&whole"
+                                       "&body" "&allow-other-keys" "nil")
+                                     (nth 3 fp)))
 		   (inopts nil))
 	       (while (and args found (> found last-pos))
                  (if (or (member (car args) nocheck)
-                         (string-match "\\`_" (car args)))
+                         (string-match "\\`_" this-arg))
 		     (setq args (cdr args)
+                           this-arg (if (string-prefix-p ":" (car args))
+                                        (substring (car args) 1)
+                                      (car args))
 			   inopts t)
 		   (setq last-pos found
 			 found (save-excursion
 				 (re-search-forward
-				  (concat "\\<" (upcase (car args))
+				  (concat "\\<" (upcase this-arg)
 					  ;; Require whitespace OR
 					  ;; ITEMth<space> OR
 					  ;; ITEMs<space>
@@ -1791,7 +1798,7 @@ checkdoc-this-string-valid-engine
 			 ;; and see if the user wants to capitalize it.
 			 (if (save-excursion
 			       (re-search-forward
-				(concat "\\<\\(" (car args)
+				(concat "\\<\\(" this-arg
 					;; Require whitespace OR
 					;; ITEMth<space> OR
 					;; ITEMs<space>
@@ -1801,10 +1808,15 @@ checkdoc-this-string-valid-engine
 				  (match-beginning 1) (match-end 1)
 				  (format-message
                                    "If this is the argument `%s', it should appear as %s.  Fix?"
-				   (car args) (upcase (car args)))
-				  (upcase (car args)) t)
+				   this-arg (upcase this-arg))
+				  (upcase this-args) t)
 				 (setq found (match-beginning 1))))))
-		   (if found (setq args (cdr args)))))
+		   (if found (setq args
+                                   (cdr args)
+                                   this-arg (if (string-prefix-p ":"
+                                                                 (car args))
+                                                (substring (car args) 1)
+                                               (car args))))))
 	       (if (not found)
 		   ;; It wasn't found at all!  Offer to attach this new symbol
 		   ;; to the end of the documentation string.
@@ -1817,7 +1829,7 @@ checkdoc-this-string-valid-engine
 			 (goto-char e) (forward-char -1)
 			 (insert "\n"
 				 (if inopts "Optional a" "A")
-				 "rgument " (upcase (car args))
+				 "rgument " (upcase this-arg)
 				 " ")
 			 (insert (read-string "Describe: "))
 			 (if (not (save-excursion (forward-char -1)
@@ -1828,7 +1840,7 @@ checkdoc-this-string-valid-engine
                        (checkdoc-create-error
                         (format-message
                          "Argument `%s' should appear (as %s) in the doc string"
-                         (car args) (upcase (car args)))
+                         (car args) (upcase this-arg))
                         s (marker-position e))))
 		 (if (or (and order (eq order 'yes))
 			 (and (not order) checkdoc-arguments-in-order-flag))