GNU bug report logs - #46918
28.0.50; checkdoc fooled by qualified method

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46918 in the body.
You can then email your comments to 46918 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; checkdoc fooled by qualified method
Date: Thu, 04 Mar 2021 10:25:27 -0300

Starting from emacs -Q:

1. Type the following in the *scratch* buffer
(cl-defmethod foo ((a (eql test)))
  "Return A."
  a)

(cl-defmethod foo :around ((a (eql test)))
  "Return A."
  a)

2. M-x checkdoc

3. See how checkdoc doesn't complain about the first method, but does
complain about the second method, with the following message:

Error message:
  All variables and subroutines might as well have a documentation string

which is wrong.

Message #8 received at 46918 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: 46918 <at> debbugs.gnu.org
Subject: Re: bug#46918: 28.0.50; checkdoc fooled by qualified method
Date: Thu, 04 Mar 2021 10:27:10 -0300

Oops, forgot to add the bug report information:

In GNU Emacs 28.0.50 (build 12, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2021-03-04 built on tbb-desktop
Repository revision: ac42f719089a880bba418c95a7a16c4d7df392dd
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Ubuntu 18.04.5 LTS

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SOUND THREADS
TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: es_AR.UTF-8
  value of $LC_NUMERIC: es_AR.UTF-8
  value of $LC_TIME: es_AR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(checkdoc lisp-mnt thingatpt help-fns radix-tree help-mode pp shadow
sort mail-extr emacsbug message rmc puny dired dired-loaddefs rfc822 mml
mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map text-property-search time-date subr-x seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs cl-lib
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame minibuffer
cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
charscript charprop case-table epa-hook jka-cmpr-hook help simple abbrev
obarray cl-preloaded nadvice button loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget hashtable-print-readable backquote threads
dbusbind inotify dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 55199 12044)
 (symbols 48 6955 4)
 (strings 32 20238 1400)
 (string-bytes 1 660821)
 (vectors 16 13795)
 (vector-slots 8 181278 17264)
 (floats 8 32 43)
 (intervals 56 381 38)
 (buffers 992 14))

Message #11 received at 46918 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: 46918 <at> debbugs.gnu.org
Subject: Re: bug#46918: 28.0.50; checkdoc fooled by qualified method
Date: Thu, 04 Mar 2021 10:31:32 -0300

[Message part 1 (text/plain, inline)]

tags 46918 patch
quit


I attach a patch to fix this, together with new tests.  I wanted to keep
the first case simple, so I moved the handling of cl-defmethod to a
different case.

[0001-Make-checkdoc-work-with-qualified-methods.patch (text/x-patch, inline)]

From 3d6e813a42f3472c817cc512ccba60c70e04b64e Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda <at> gmail.com>
Date: Thu, 4 Mar 2021 08:34:58 -0300
Subject: [PATCH] Make checkdoc work with qualified methods

* lisp/emacs-lisp/checkdoc.el (checkdoc--next-docstring): Handle
cl-defmethod in a case of its own.  Check for the presence of
qualifiers, and skip them accordingly until the docstring.

* test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-cl-defmethod-qualified-ok)
(checkdoc-cl-defmethod-with-extra-qualifier-ok)
(checkdoc-cl-defmethod-with-extra-and-nil-args-ok): Add tests for the fix.
---
 lisp/emacs-lisp/checkdoc.el            | 21 +++++++++++++++++++-
 test/lisp/emacs-lisp/checkdoc-tests.el | 27 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 75aefdc7ba..213ab43184 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -932,7 +932,7 @@ checkdoc--next-docstring
                            ;; definition ends prematurely.
                            (end-of-file)))
     (`(,(or 'defun 'defvar 'defcustom 'defmacro 'defconst 'defsubst 'defadvice
-            'cl-defun 'cl-defgeneric 'cl-defmethod 'cl-defmacro)
+            'cl-defun 'cl-defgeneric 'cl-defmacro)
        ,(pred symbolp)
        ;; Require an initializer, i.e. ignore single-argument `defvar'
        ;; forms, which never have a doc string.
@@ -942,6 +942,25 @@ checkdoc--next-docstring
      ;; initializer or argument list.
      (forward-sexp 3)
      (skip-chars-forward " \n\t")
+     t)
+    (`(,'cl-defmethod
+        ,(pred symbolp)
+        . ,rest)
+     (down-list)
+     (forward-sexp (pcase (car rest)
+                     ;; No qualifier, so skip like we would have skipped in
+                     ;; the first clause of the outer `pcase'.
+                     ((pred listp) 3)
+                     (':extra
+                      ;; Skip the :extra qualifier together with its string too.
+                      ;; Skip any additional qualifier.
+                      (if (memq (nth 2 rest) '(:around :before :after))
+                                  6
+                                5))
+                     ;; Skip :before, :after or :around qualifier too.
+                     ((or ':around ':before ':after)
+                      4)))
+     (skip-chars-forward " \n\t")
      t)))
 
 ;;;###autoload
diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el
index 93015fbb10..7a7aa9fb3c 100644
--- a/test/lisp/emacs-lisp/checkdoc-tests.el
+++ b/test/lisp/emacs-lisp/checkdoc-tests.el
@@ -52,6 +52,33 @@ checkdoc-cl-defmethod-with-types-ok
     (insert "(cl-defmethod foo ((a (eql smthg)) (b list)) \"Return A+B.\")")
     (checkdoc-defun)))
 
+(ert-deftest checkdoc-cl-defmethod-qualified-ok ()
+  "Checkdoc should be happy with a `cl-defmethod' using qualifiers."
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(cl-defmethod test :around ((a (eql smthg))) \"Return A.\")")
+    (checkdoc-defun)))
+
+(ert-deftest checkdoc-cl-defmethod-with-extra-qualifier-ok ()
+  "Checkdoc should be happy with a :extra qualified `cl-defmethod'."
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(cl-defmethod foo :extra \"foo\" ((a (eql smthg))) \"Return A.\")")
+    (checkdoc-defun))
+
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert
+     "(cl-defmethod foo :extra \"foo\" :after ((a (eql smthg))) \"Return A.\")")
+    (checkdoc-defun)))
+
+(ert-deftest checkdoc-cl-defmethod-with-extra-qualifier-and-nil-args-ok ()
+  "Checkdoc should be happy with a 0-arity :extra qualified `cl-defmethod'."
+  (with-temp-buffer
+    (emacs-lisp-mode)
+    (insert "(cl-defmethod foo :extra \"foo\" () \"Return A.\")")
+    (checkdoc-defun)))
+
 (ert-deftest checkdoc-cl-defun-with-key-ok ()
   "Checkdoc should be happy with a cl-defun using &key."
   (with-temp-buffer
-- 
2.30.0

Message #16 received at 46918 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 46918 <at> debbugs.gnu.org
Subject: Re: bug#46918: 28.0.50; checkdoc fooled by qualified method
Date: Thu, 04 Mar 2021 21:18:18 +0100

Mauro Aranda <maurooaranda <at> gmail.com> writes:

> I attach a patch to fix this, together with new tests.  I wanted to keep
> the first case simple, so I moved the handling of cl-defmethod to a
> different case.

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #19 received at 46918 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46918 <at> debbugs.gnu.org
Subject: Re: bug#46918: 28.0.50; checkdoc fooled by qualified method
Date: Thu, 04 Mar 2021 17:35:20 -0300

close 46918 28.1
quit


Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>> I attach a patch to fix this, together with new tests.  I wanted to keep
>> the first case simple, so I moved the handling of cl-defmethod to a
>> different case.
>
> Looks good to me.

Thanks, I pushed the fix to master.  It should be tweaked if the :extra
qualifier is allowed to come last (Bug#46917), but it should be easy to
do that.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.