Subject: bug#76372: 31.0.50; False () in ert-font-lock macro signatures

From: "Basil L. Contovounesios" <[email-masked]>
Date: Mon, 17 Feb 2025 21:37:06 +0100

0. emacs -Q
1. C-h f ert-font-lock-deftest RET

*Help* now reports the macro's calling convention as:

  (ert-font-lock-deftest NAME () [DOCSTRING] ...)

but the second () argument is not actually supported by the macro, whose
arglist reads:

  (name &rest docstring-keys-mode-and-str)

Here, ert-font-lock--parse-macro-args expects
docstring-keys-mode-and-str to start with an (optional) string,
not the empty arglist '().

The symptom of this mismatch is that calls to ert-font-lock-deftest are
misindented (malindented? :) and misfontified.  For example, in
ert-font-lock-tests.el:

  (ert-font-lock-deftest test-macro-test--correct-highlighting
      emacs-lisp-mode
    "
  (defun fun ())
  ;; ^ font-lock-keyword-face
  ;;      ^ font-lock-function-name-face")

Here, the string literal has font-lock-doc-face instead of
font-lock-string-face.

  (ert-font-lock-deftest test-macro-test--docstring
      "A test with a docstring."
    emacs-lisp-mode
    "
  (defun fun ())
  ;; ^ font-lock-keyword-face"
    )

Here, both strings have font-lock-string-face, whereas the first should
have font-lock-doc-face.

Finally, if a test actually followed ert-font-lock-deftest's docstring
in including an empty arglist (), then ert-describe-test would claim
that it is not documented.

The attached patch tries to address these issues.
WDYT?

Thanks,
-- 
Basil

>From 6487a8876fcd66ee38bd67b02994a2ad2b635886 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Sun, 16 Feb 2025 16:29:13 +0100
Subject: [PATCH] Fix ert-font-lock macro signatures

* doc/misc/ert.texi (Syntax Highlighting Tests):
* test/lisp/emacs-lisp/ert-font-lock-tests.el
(test-line-comment-p--emacs-lisp, test-line-comment-p--shell-script)
(test-line-comment-p--javascript, test-line-comment-p--python)
(test-line-comment-p--c, test-macro-test--correct-highlighting)
(test-macro-test--docstring, test-macro-test--failing)
(test-macro-test--file, test-macro-test--file-no-asserts)
(test-macro-test--file-failing): Reindent macro calls.
(with-temp-buffer-str-mode): Evaluate macro arguments left-to-right.
(ert-font-lock--wrap-begin-end): Use rx for more robust composition.
(test-line-comment-p--php): Require that php-mode is callable, not
already loaded.

* lisp/emacs-lisp/ert-font-lock.el: Remove redundant requires.
(ert-font-lock--parse-macro-args): First return value 'doc' being
nil already indicates omission of a docstring, so remove redundant
second return value doc-p.  All users updated.  Fix error messages.
(ert-font-lock-deftest, ert-font-lock-deftest-file): NAME is not
followed by an empty list like in ert-deftest, so the optional
DOCSTRING is actually the second argument.  Adapt calling convention
in docstring, and debug, doc-string, and indent properties
accordingly.  Fix docstring grammar, document MAJOR-MODE, and avoid
referring to a file name as a path.
(ert-font-lock--check-faces): Simplify with ensure-list.
---
 doc/misc/ert.texi                           | 10 +--
 lisp/emacs-lisp/ert-font-lock.el            | 77 ++++++++---------
 test/lisp/emacs-lisp/ert-font-lock-tests.el | 94 ++++++++++-----------
 3 files changed, 87 insertions(+), 94 deletions(-)

diff --git a/doc/misc/ert.texi b/doc/misc/ert.texi
index 566fa03bf3f..8f9e4fff415 100644
--- a/doc/misc/ert.texi
+++ b/doc/misc/ert.texi
@@ -962,7 +962,7 @@ Syntax Highlighting Tests
 comments directly and can be read either from inline strings or files.
 The parser expects the input string to contain at least one assertion.
 
-Test assertion parser extracts tests from comment-only lines.  Every
+The test assertion parser extracts tests from comment-only lines.  Every
 comment assertion line starts either with a caret (@samp{^}) or an arrow
 (@samp{<-}).  A single caret/arrow or carets should be followed
 immediately by the name of a face or a list of faces to be checked
@@ -984,7 +984,7 @@ Syntax Highlighting Tests
 @end example
 
 Both symbol-only @code{:face} property values and assertion face values
-are normalized to single element lists so assertions below are
+are normalized to single element lists so the assertions below are
 equivalent:
 
 @example
@@ -1061,7 +1061,7 @@ Syntax Highlighting Tests
 
 @lisp
 (ert-font-lock-deftest test-macro-test--inline
-    emacs-lisp-mode
+  emacs-lisp-mode
   "
 (defun fun ())
 ;; ^ font-lock-keyword-face
@@ -1075,13 +1075,13 @@ Syntax Highlighting Tests
 
 @lisp
 (ert-font-lock-deftest-file test-macro-test--file
-    "Test reading correct assertions from a file"
+  "Test reading correct assertions from a file."
   javascript-mode
   "correct.js")
 @end lisp
 
 The @code{ert-font-lock-deftest} and @code{ert-font-lock-deftest-file}
-macros accept the same keyword parameters as @code{ert-deftest} i.e.,
+macros accept the same keyword arguments as @code{ert-deftest}, i.e.,
 @code{:tag} and @code{:expected-result}.

diff --git a/lisp/emacs-lisp/ert-font-lock.el b/lisp/emacs-lisp/ert-font-lock.el
index e584eb679fd..0f8e90d747c 100644
--- a/lisp/emacs-lisp/ert-font-lock.el
+++ b/lisp/emacs-lisp/ert-font-lock.el
@@ -27,8 +27,8 @@
 ;; highlighting provided by font-lock.
 ;;
 ;; ert-font-lock entry points are functions
-;; `ert-font-lock-test-string' and `ert-font-lock-test-file' and
-;; convenience macros: `ert-font-lock-deftest' and
+;; `ert-font-lock-test-string' and `ert-font-lock-test-file', and
+;; convenience macros `ert-font-lock-deftest' and
 ;; `ert-font-lock-deftest-file'.
 ;;
 ;; See unit tests in ert-font-lock-tests.el for usage examples.
@@ -37,8 +37,6 @@
 
 (require 'ert)
 (require 'ert-x)
-(require 'newcomment)
-(require 'pcase)
 
 (defconst ert-font-lock--face-symbol-re
   (rx (+ (or alphanumeric "-" "_" "." "/")))
@@ -100,98 +98,95 @@ ert-font-lock--test-body-file
 
 (defun ert-font-lock--parse-macro-args (doc-keys-mode-arg)
   "Parse DOC-KEYS-MODE-ARG macro argument list."
-  (let (doc doc-p mode arg)
+  (let (doc mode arg)
 
     (when (stringp (car doc-keys-mode-arg))
-      (setq doc (pop doc-keys-mode-arg)
-            doc-p t))
+      (setq doc (pop doc-keys-mode-arg)))
 
     (pcase-let
         ((`(,keys ,mode-arg)
           (ert--parse-keys-and-body doc-keys-mode-arg)))
 
       (unless (symbolp (car mode-arg))
-        (error "A major mode symbol expected: %S" (car mode-arg)))
+        (error "Expected a major mode symbol: %S" (car mode-arg)))
       (setq mode (pop mode-arg))
 
       (unless (stringp (car mode-arg))
-        (error "A string or file with assertions expected: %S" (car mode-arg)))
+        (error "Expected a string or file with assertions: %S" (car mode-arg)))
       (setq arg (pop mode-arg))
 
-      (list doc doc-p keys mode arg))))
+      (list doc keys mode arg))))
 
 ;;;###autoload
 (defmacro ert-font-lock-deftest (name &rest docstring-keys-mode-and-str)
   "Define test NAME (a symbol) using assertions from TEST-STR.
 
-Other than MAJOR-MODE and TEST-STR parameters, this macro accepts
-the same parameters and keywords as `ert-deftest' and is intended
-to be used through `ert'.
+The MAJOR-MODE symbol determines the syntax and font lock of TEST-STR.
 
-\(fn NAME () [DOCSTRING] [:expected-result RESULT-TYPE] \
+Except for the MAJOR-MODE and TEST-STR parameters, this macro accepts
+the same arguments and keywords as `ert-deftest' and is intended to be
+used through `ert'.
+
+\(fn NAME [DOCSTRING] [:expected-result RESULT-TYPE] \
 [:tags \\='(TAG...)] MAJOR-MODE TEST-STR)"
   (declare (debug (&define [&name "test@" symbolp]
-                           sexp [&optional stringp]
+                           [&optional stringp]
                            [&rest keywordp sexp]
                            symbolp
                            stringp))
-           (doc-string 3)
-           (indent 2))
-  (pcase-let ((`(,documentation
-                 ,documentation-supplied-p
-                 ,keys ,mode ,arg)
+           (doc-string 2)
+           (indent 1))
+  (pcase-let ((`(,documentation ,keys ,mode ,arg)
                (ert-font-lock--parse-macro-args docstring-keys-mode-and-str)))
 
     `(ert-set-test ',name
                    (make-ert-test
                     :name ',name
-                    ,@(when documentation-supplied-p
+                    ,@(when documentation
                         `(:documentation ,documentation))
                     ,@(when (map-contains-key keys :expected-result)
                         `(:expected-result-type ,(map-elt keys :expected-result)))
                     ,@(when (map-contains-key keys :tags)
                         `(:tags ,(map-elt keys :tags)))
-                    :body (lambda () (ert-font-lock--test-body-str ',mode ,arg ',name))
-
+                    :body (lambda ()
+                            (ert-font-lock--test-body-str ',mode ,arg ',name))
                     :file-name ,(or (macroexp-file-name) buffer-file-name)))))
 
 ;;;###autoload
 (defmacro ert-font-lock-deftest-file (name &rest docstring-keys-mode-and-file)
   "Define test NAME (a symbol) using assertions from FILE.
 
-FILE - path to a file with assertions in ERT resource director as
-return by `ert-resource-directory'.
+FILE names a file with assertions in the ERT resource directory, as
+returned by `ert-resource-directory'.  The MAJOR-MODE symbol determines
+the syntax and font lock of FILE's contents.
 
-Other than MAJOR-MODE and FILE parameters, this macro accepts the
-same parameters and keywords as `ert-deftest' and is intended to
-be used through `ert'.
+Except for the MAJOR-MODE and FILE parameters, this macro accepts the
+same arguments and keywords as `ert-deftest' and is intended to be used
+through `ert'.
 
-\(fn NAME () [DOCSTRING] [:expected-result RESULT-TYPE] \
+\(fn NAME [DOCSTRING] [:expected-result RESULT-TYPE] \
 [:tags \\='(TAG...)] MAJOR-MODE FILE)"
   (declare (debug (&define [&name "test@" symbolp]
-                           sexp [&optional stringp]
+                           [&optional stringp]
                            [&rest keywordp sexp]
                            symbolp
                            stringp))
-           (doc-string 3)
-           (indent 2))
-
-  (pcase-let ((`(,documentation
-                 ,documentation-supplied-p
-                 ,keys ,mode ,arg)
+           (doc-string 2)
+           (indent 1))
+  (pcase-let ((`(,documentation ,keys ,mode ,arg)
                (ert-font-lock--parse-macro-args docstring-keys-mode-and-file)))
 
     `(ert-set-test ',name
                    (make-ert-test
                     :name ',name
-                    ,@(when documentation-supplied-p
+                    ,@(when documentation
                         `(:documentation ,documentation))
                     ,@(when (map-contains-key keys :expected-result)
                         `(:expected-result-type ,(map-elt keys :expected-result)))
                     ,@(when (map-contains-key keys :tags)
                         `(:tags ,(map-elt keys :tags)))
                     :body (lambda () (ert-font-lock--test-body-file
-                                 ',mode (ert-resource-file ,arg) ',name))
+                                      ',mode (ert-resource-file ,arg) ',name))
                     :file-name ,(or (macroexp-file-name) buffer-file-name)))))
 
 (defun ert-font-lock--in-comment-p ()
@@ -354,10 +349,8 @@ ert-font-lock--check-faces
 
       ;; normalize both expected and resulting face - these can be
       ;; either symbols, nils or lists of symbols
-      (when (not (listp actual-face))
-        (setq actual-face (list actual-face)))
-      (when (not (listp expected-face))
-        (setq expected-face (list expected-face)))
+      (setq actual-face (ensure-list actual-face))
+      (setq expected-face (ensure-list expected-face))
 
       ;; fail when lists are not 'equal and the assertion is *not negated*
       (when (and (not negation) (not (equal actual-face expected-face)))
diff --git a/test/lisp/emacs-lisp/ert-font-lock-tests.el b/test/lisp/emacs-lisp/ert-font-lock-tests.el
index e410cf5fa13..194a79f32b7 100644
--- a/test/lisp/emacs-lisp/ert-font-lock-tests.el
+++ b/test/lisp/emacs-lisp/ert-font-lock-tests.el
@@ -39,13 +39,13 @@ with-temp-buffer-str-mode
   "Create a buffer with STR contents and MODE. "
   (declare (indent 1) (debug t))
   `(with-temp-buffer
-     (insert ,str)
      (,mode)
+     (insert ,str)
      (goto-char (point-min))
      ,@body))
 
 (defun ert-font-lock--wrap-begin-end (re)
-  (concat "^" re "$"))
+  (rx bol (regexp re) eol))
 
 ;;; Regexp tests
 ;;;
@@ -97,89 +97,89 @@ test-line-comment-p--fundamental
 
 (ert-deftest test-line-comment-p--emacs-lisp ()
   (with-temp-buffer-str-mode emacs-lisp-mode
-                             "not comment
+    "not comment
 ;; comment
 "
-                             (should-not (ert-font-lock--line-comment-p))
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))))
+    (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))))
 
 (ert-deftest test-line-comment-p--shell-script ()
   (with-temp-buffer-str-mode shell-script-mode
-                             "echo Not a comment
+    "echo Not a comment
 # comment
 "
-                             (should-not (ert-font-lock--line-comment-p))
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))))
+    (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))))
 
 (declare-function php-mode "php-mode")
 (ert-deftest test-line-comment-p--php ()
-  (skip-unless (featurep 'php-mode))
+  (skip-unless (fboundp 'php-mode))
 
   (with-temp-buffer-str-mode php-mode
-                             "echo 'Not a comment'
+    "echo 'Not a comment'
 // comment
 /* comment */
 "
-                             (should-not (ert-font-lock--line-comment-p))
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))))
+    (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))))

 (ert-deftest test-line-comment-p--javascript ()
   (with-temp-buffer-str-mode javascript-mode
-                             "// comment
+    "// comment
 
    // comment, after a blank line
 
 var abc = function(d) {};
 "
-                             (should (ert-font-lock--line-comment-p))
+    (should (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))))
 
 (ert-deftest test-line-comment-p--python ()
 
   (with-temp-buffer-str-mode python-mode
-                             "# comment
+    "# comment
 
    # comment
 print(\"Hello, world!\")"
-                             (should (ert-font-lock--line-comment-p))
+    (should (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should-not (ert-font-lock--line-comment-p))))
+    (forward-line)
+    (should-not (ert-font-lock--line-comment-p))))
 
 (ert-deftest test-line-comment-p--c ()
 
   (with-temp-buffer-str-mode c-mode
-                             "// comment
+    "// comment
 /* also comment */"
-                             (should (ert-font-lock--line-comment-p))
+    (should (ert-font-lock--line-comment-p))
 
-                             (forward-line)
-                             (should (ert-font-lock--line-comment-p))))
+    (forward-line)
+    (should (ert-font-lock--line-comment-p))))
 
 (ert-deftest test-parse-comments--no-assertion-error ()
   (let* ((str "
@@ -568,14 +568,14 @@ test-font-lock-test-file--wrong
 ;;
 
 (ert-font-lock-deftest test-macro-test--correct-highlighting
-    emacs-lisp-mode
+  emacs-lisp-mode
   "
 (defun fun ())
 ;; ^ font-lock-keyword-face
 ;;      ^ font-lock-function-name-face")
 
 (ert-font-lock-deftest test-macro-test--docstring
-    "A test with a docstring."
+  "A test with a docstring."
   emacs-lisp-mode
   "
 (defun fun ())
@@ -583,7 +583,7 @@ fun
   )
 
 (ert-font-lock-deftest test-macro-test--failing
-    "A failing test."
+  "A failing test."
   :expected-result :failed
   emacs-lisp-mode
   "
@@ -591,18 +591,18 @@ fun
 ;; ^ wrong-face")
 
 (ert-font-lock-deftest-file test-macro-test--file
-    "Test reading correct assertions from a file"
+  "Test reading correct assertions from a file"
   javascript-mode
   "correct.js")
 
 (ert-font-lock-deftest-file test-macro-test--file-no-asserts
-    "Check failing on files without assertions"
+  "Check failing on files without assertions."
   :expected-result :failed
   javascript-mode
   "no-asserts.js")
 
 (ert-font-lock-deftest-file test-macro-test--file-failing
-    "Test reading wrong assertions from a file"
+  "Test reading wrong assertions from a file."
   :expected-result :failed
   javascript-mode
   "broken.js")
-- 
2.47.2

In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.2, Xaw3d scroll bars) of 2025-02-16 built on tais
Repository revision: fc189cb83cf7284344a737e75b3ee56487b1c386
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101015
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure 'CFLAGS=-Og -ggdb3' -C --prefix=/home/blc/.local
 --enable-checking=structs --without-native-compilation
 --with-file-notification --with-x-toolkit=lucid --with-x'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XAW3D XDBE XIM XINERAMA XINPUT2
XPM XRANDR LUCID ZLIB

Important settings:
  value of $LANG: en_IE.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
  show-paren-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
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils cl-extra shortdoc
text-property-search comp-common rx ert-font-lock pcase ert-x subr-x ert
map pp ewoc debug backtrace find-func thingatpt help-fns byte-opt gv
bytecomp byte-compile radix-tree help-mode cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-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 font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty move-toolbar
make-network-process tty-child-frames emacs)

Memory information:
((conses 16 75191 9165) (symbols 48 7566 0) (strings 32 22214 1377)
 (string-bytes 1 522717) (vectors 16 13949)
 (vector-slots 8 148704 7756) (floats 8 87 2) (intervals 56 318 7)
 (buffers 992 11))