Package: emacs;
Reported by: Marco Antoniotti <marcoxa <at> gmail.com>
Date: Wed, 21 May 2025 20:40:01 UTC
Severity: normal
Found in version 29.2
Done: Eli Zaretskii <eliz <at> gnu.org>
Message #8 received at 78543 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Marco Antoniotti <marcoxa <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 78543 <at> debbugs.gnu.org Subject: Re: bug#78543: 29.2; checkdoc misses some CL (cl-lib) constructs. Date: Sat, 31 May 2025 14:07:46 +0300
> From: Marco Antoniotti <marcoxa <at> gmail.com>
> 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))
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.