Return nil from more ERC response handlers

* etc/ERC-NEWS: Mention that certain aberrant response handlers now
return nil.
* lisp/erc/erc-backend.el (define-erc-response-handler): Mention that
body should explicitly return nil.
(erc-server-PART)
(erc-server-PING): Return nil.
* lisp/erc/erc-sasl.el (erc-sasl--destroy): Return nil.
* lisp/erc/erc.el (erc-display-message): Mention in doc string that
the return value is undefined.
(erc-kill-channel-hook): Fix package-version.
* test/lisp/erc/erc-networks-tests.el (erc-networks--set-name): Ensure
`erc--route-insertion' returns nil because this influences whether
response-handler hooks continue running.
* test/lisp/erc/erc-sasl-tests.el
(erc-sasl-create-client-ecdsa): Fix regression that made test
unusable, although it's still relatively useless and therefore skipped
by default.
* test/lisp/erc/erc-services-tests.el
(erc-services-tests--auth-source-standard)
(erc-services-tests--auth-source-announced): Clarify annotations.
* test/lisp/erc/erc-tests.el (erc-message): Don't return non-nil in
mocked `erc-display-message'.
(erc-send-modify-hook): Shadow `erc-send-modify-hook' because
`erc-stamp--date-mode' modifies it locally.

etc/ERC-NEWS

@@ -685,6 +685,14 @@ The option 'erc-format-nick-function' has been renamed to
 actual role.  So too has the related function 'erc-format-nick', which
 is now 'erc-determine-speaker-from user'.
 
+*** All default response handlers return nil.
+Actually, this isn't yet true, but ERC is moving in this direction.
+The goal is to guarantee that trailing members of response hooks, like
+'erc-server-005-functions', have an opportunity to run after the
+default handler.  For now, certain default handlers that may have
+previously returned non-nil, like 'erc-server-PONG' and
+'erc-server-904', have been updated to return nil in all cases.
+
 *** A template-based approach to formatting inserted chat messages.
 Predicting and influencing how ERC formats messages containing a
 leading "<speaker>" has never been straightforward.  The characters

lisp/erc/erc-backend.el

@@ -1566,13 +1566,23 @@ This creates:
    `erc-server-NAME'.
  - a function `erc-server-NAME' with body FN-BODY.
 
+\(Note that here, NAME merely refers to the parameter NAME rather than
+an actual IRC response or server-sent command.)
+
 If ALIASES is non-nil, each alias in ALIASES is `defalias'ed to
 `erc-server-NAME'.
 Alias hook variables are created as `erc-server-ALIAS-functions' and
 initialized to the same default value as `erc-server-NAME-functions'.
 
-FN-BODY is the body of `erc-server-NAME' it may refer to the two
-function arguments PROC and PARSED.
+ERC uses FN-BODY as the body of the default response handler
+`erc-server-NAME', which handles all incoming IRC \"NAME\" responses,
+unless overridden (see below).  ERC calls the function with two
+arguments, PROC and PARSED, whose symbols (lowercase) are bound to the
+current `erc-server-process' and `erc-response' instance within FN-BODY.
+Implementers should take care not to shadow them inadvertently.  In all
+cases, FN-BODY should return nil to allow third parties to run code
+after `erc-server-NAME' returns.  For historical reasons, ERC does not
+currently enforce this, however future versions very well may.
 
 If EXTRA-FN-DOC is non-nil, it is inserted at the beginning of the
 defined function's docstring.
@@ -1902,7 +1912,8 @@ add things to `%s' instead."
         (when (and erc-kill-buffer-on-part buffer)
           (defvar erc-killing-buffer-on-part-p)
           (let ((erc-killing-buffer-on-part-p t))
-            (kill-buffer buffer)))))))
+            (kill-buffer buffer))))))
+  nil)
 
 (define-erc-response-handler (PING)
   "Handle ping messages." nil
@@ -1914,7 +1925,8 @@ add things to `%s' instead."
       (erc-display-message
        parsed 'error proc
        'PING ?s (erc-time-diff erc-server-last-ping-time (erc-current-time))))
-    (setq erc-server-last-ping-time (erc-current-time))))
+    (setq erc-server-last-ping-time (erc-current-time)))
+  nil)
 
 (define-erc-response-handler (PONG)
   "Handle pong messages." nil

lisp/erc/erc-sasl.el

@@ -373,7 +373,8 @@ This doesn't solicit or validate a suite of supported mechanisms."
   "Destroy process PROC and warn user that their settings are likely faulty."
   (delete-process proc)
   (erc--lwarn 'erc-sasl :error
-              "Disconnected from %s; please review SASL settings" proc))
+              "Disconnected from %s; please review SASL settings" proc)
+  nil)
 
 (define-erc-response-handler (902)
   "Handle an ERR_NICKLOCKED response." nil

lisp/erc/erc.el

@@ -3987,7 +3987,9 @@ As of ERC 5.6, assume third-party code will use this function
 instead of lower-level ones, like `erc-insert-line', to insert
 arbitrary informative messages as if sent by the server.  That
 is, tell modules to treat a \"local\" message for which PARSED is
-nil like any other server-sent message."
+nil like any other server-sent message.  Finally, expect users to
+treat the return value of this function as undefined even though
+various default response handlers may appear to presume nil."
   (let* ((erc--msg-props
           (or erc--msg-props
               (let ((table (make-hash-table))
@@ -9626,7 +9628,7 @@ See also `format-spec'."
     erc-networks-shrink-ids-and-buffer-names
     erc-networks-rename-surviving-target-buffer)
   "Invoked whenever a channel-buffer is killed via `kill-buffer'."
-  :package-version '(ERC . "5.5")
+  :package-version '(ERC . "5.6") ; FIXME sync on release
   :group 'erc-hooks
   :type 'hook)
 

test/lisp/erc/erc-networks-tests.el

@@ -1199,7 +1199,7 @@
       (erc-mode)
 
       (cl-letf (((symbol-function 'erc--route-insertion)
-                 (lambda (&rest r) (push r calls))))
+                 (lambda (&rest r) (ignore (push r calls)))))
 
         (ert-info ("Signals when `erc-server-announced-name' unset")
           (should-error (erc-networks--set-name nil (make-erc-response)))

test/lisp/erc/erc-sasl-tests.el

@@ -319,16 +319,27 @@ IRX9cyi2wdYg9mUUYyh9GKdBCYHGUJAiCA==
   :tags '(:unstable)
   ;; This is currently useless because it just roundtrips shelling out
   ;; to pkeyutl.
-  (ert-skip "Placeholder")
+  (ert-skip "Placeholder for manual debugging")
   (unless (executable-find "openssl")
     (ert-skip "System lacks openssl"))
+
   (ert-with-temp-file keyfile
     :prefix "ecdsa_key"
     :suffix ".pem"
     :text erc-sasl-tests-ecdsa-key-file
-    (let* ((erc-server-current-nick "jilles")
-           (erc-sasl--options `((password . ,keyfile)))
-           (client (erc-sasl--create-client 'ecdsa-nist256p-challenge))
+
+    (erc-mode)
+    (erc--initialize-markers (point) nil)
+    (setq erc-server-process (make-process :name "sleep"
+                                           :buffer (current-buffer)
+                                           :command '("sleep" "1")
+                                           :noquery t)
+          erc-session-username "jilles")
+    (let ((erc-sasl-mechanism 'ecdsa-nist256p-challenge)
+          (erc-sasl-password keyfile))
+      (erc-sasl-mode +1))
+
+    (let* ((client (erc-sasl--state-client erc-sasl--state))
            (step (sasl-next-step client nil)))
       (ert-info ("Client's initial request")
         (should (equal (format "%S" [erc-sasl--ecdsa-first "jilles"])

test/lisp/erc/erc-services-tests.el

@@ -70,7 +70,7 @@
 (defun erc-services-tests--auth-source-standard (search)
   (setq search (erc-services-tests--wrap-search search))
 
-  (ert-info ("Session wins")
+  (ert-info ("Session ID wins")
     (let ((erc-session-server "irc.gnu.org")
           (erc-server-announced-name "my.gnu.org")
           (erc-session-port 6697)
@@ -92,9 +92,14 @@
     (let ((erc-session-server "irc.gnu.org")
           (erc-server-announced-name "my.gnu.org")
           (erc-session-port 6697)
-          erc-network
           (erc-networks--id (erc-networks--id-create nil)))
-      (should (string= (funcall search :user "#chan") "baz")))))
+      (should (string= (funcall search :user "#chan") "baz"))))
+
+  (ert-info ("Dialed wins")
+    (let ((erc-session-server "irc.gnu.org")
+          (erc-session-port 6697)
+          (erc-networks--id (erc-networks--id-create nil)))
+      (should (string= (funcall search :user "#chan") "bar")))))
 
 (defun erc-services-tests--auth-source-announced (search)
   (setq search (erc-services-tests--wrap-search search))
@@ -102,7 +107,8 @@
          (erc-server-parameters '(("CHANTYPES" . "&#")))
          (erc--target (erc--target-from-string "&chan")))
 
-    (ert-info ("Announced prioritized")
+    ;; Pretend #chan is just some account name and not a channel.
+    (ert-info ("Host priorities reversed when target is local")
 
       (ert-info ("Announced wins")
         (let* ((erc-session-server "irc.gnu.org")
@@ -113,7 +119,7 @@
                (erc-networks--id (erc-networks--id-create nil)))
           (should (string= (funcall search :user "#chan") "baz"))))
 
-      (ert-info ("Peer next")
+      (ert-info ("Dialed next")
         (let* ((erc-server-announced-name "irc.gnu.org")
                (erc-session-port 6697)
                (erc-network 'GNU.chat)

test/lisp/erc/erc-tests.el

@@ -330,6 +330,7 @@
                                (cl-incf counter))))
          erc-accidental-paste-threshold-seconds
          erc-insert-modify-hook
+         erc-send-modify-hook
          (erc-last-input-time 0)
          (erc-modules (remq 'stamp erc-modules))
          (erc-send-input-line-function #'ignore)
@@ -1268,6 +1269,7 @@
       (should-not (erc--valid-local-channel-p "#chan"))
       (should (erc--valid-local-channel-p "&local")))))
 
+;; FIXME remove this because it serves no purpose.  See bug#71178.
 (ert-deftest erc--restore-initialize-priors ()
   (unless (>= emacs-major-version 28)
     (ert-skip "Lisp nesting exceeds `max-lisp-eval-depth'"))
@@ -2533,7 +2535,7 @@
         erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
     (cl-letf (((symbol-function 'erc-display-message)
                (lambda (_ _ _ msg &rest args)
-                 (push (apply #'erc-format-message msg args) calls)))
+                 (ignore (push (apply #'erc-format-message msg args) calls))))
               ((symbol-function 'erc-server-send)
                (lambda (line _) (push line calls)))
               ((symbol-function 'erc-server-buffer)