bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

[npostavs]

npostavs@users.sourceforge.net writes:

>     (defun foo (&rest args)
>       "Do foo.
>
>     \(fn ARG1 ARG2)")
>
>     (help-function-arglist 'foo) ;=> (&rest args)

Here's a patch which puts the check of the docstring before the lambda
list.  I'm not sure what to do with the PRESERVE-NAMES argument of
`help-function-arglist' though.

>From 0a84e9441d94485ba20c2e44cb9062a7e4259968 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 2 Jun 2017 22:34:32 -0400
Subject: [PATCH v1] Prefer docstring as source of arglist with names
 (Bug#26270)

* lisp/help.el (help-arglist-from-docstring): New function, extracted
from `help-function-arglist'.
(help-function-arglist): Use it before looking for a function's lambda
list.
---
 lisp/help.el | 61 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/lisp/help.el b/lisp/help.el
index 361ab2a01e..4d98c1dc81 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1417,6 +1417,23 @@ (defun help-add-fundoc-usage (docstring arglist)
                   (error "Unrecognized usage format"))
              (help--make-usage-docstring 'fn arglist)))))
 
+(defun help-arglist-from-docstring (doc)
+  "Return argument list parsed from the docstring, DOC.
+DOC should end with \"(fn ARGLIST)\", see Info node `(elisp)
+Function Documentation'."
+  (let* ((docargs (if doc (car (help-split-fundoc doc nil))))
+         (arglist (if docargs (cdr (read (downcase docargs)))))
+         (valid t))
+    ;; Check validity.
+    (dolist (arg arglist)
+      (unless (and (symbolp arg)
+                   (let ((name (symbol-name arg)))
+                     (if (eq (aref name 0) ?&)
+                         (memq arg '(&rest &optional))
+                       (not (string-match "\\." name)))))
+        (setq valid nil)))
+    (when valid arglist)))
+
 (defun help-function-arglist (def &optional preserve-names)
   "Return a formal argument list for the function DEF.
 If PRESERVE-NAMES is non-nil, return a formal arglist that uses
@@ -1425,40 +1442,28 @@ (defun help-function-arglist (def &optional 
preserve-names)
   (if (and (symbolp def) (fboundp def)) (setq def (indirect-function def)))
   ;; If definition is a macro, find the function inside it.
   (if (eq (car-safe def) 'macro) (setq def (cdr def)))
+  ;; FIXME: What does PRESERVE-NAMES really mean??
   (cond
+   ((help-arglist-from-docstring
+     (ignore-errors (documentation def))))
    ((and (byte-code-function-p def) (listp (aref def 0))) (aref def 0))
    ((eq (car-safe def) 'lambda) (nth 1 def))
    ((eq (car-safe def) 'closure) (nth 2 def))
    ((or (and (byte-code-function-p def) (integerp (aref def 0)))
         (subrp def) (module-function-p def))
-    (or (when preserve-names
-          (let* ((doc (condition-case nil (documentation def) (error nil)))
-                 (docargs (if doc (car (help-split-fundoc doc nil))))
-                 (arglist (if docargs
-                              (cdar (read-from-string (downcase docargs)))))
-                 (valid t))
-            ;; Check validity.
-            (dolist (arg arglist)
-              (unless (and (symbolp arg)
-                           (let ((name (symbol-name arg)))
-                             (if (eq (aref name 0) ?&)
-                                 (memq arg '(&rest &optional))
-                               (not (string-match "\\." name)))))
-                (setq valid nil)))
-            (when valid arglist)))
-        (let* ((arity (func-arity def))
-               (max (cdr arity))
-               (min (car arity))
-               (arglist ()))
-          (dotimes (i min)
-            (push (intern (concat "arg" (number-to-string (1+ i)))) arglist))
-          (when (and (integerp max) (> max min))
-            (push '&optional arglist)
-            (dotimes (i (- max min))
-              (push (intern (concat "arg" (number-to-string (+ 1 i min))))
-                    arglist)))
-          (unless (integerp max) (push '&rest arglist) (push 'rest arglist))
-          (nreverse arglist))))
+    (let* ((arity (func-arity def))
+           (max (cdr arity))
+           (min (car arity))
+           (arglist ()))
+      (dotimes (i min)
+        (push (intern (concat "arg" (number-to-string (1+ i)))) arglist))
+      (when (and (integerp max) (> max min))
+        (push '&optional arglist)
+        (dotimes (i (- max min))
+          (push (intern (concat "arg" (number-to-string (+ 1 i min))))
+                arglist)))
+      (unless (integerp max) (push '&rest arglist) (push 'rest arglist))
+      (nreverse arglist)))
    ((and (autoloadp def) (not (eq (nth 4 def) 'keymap)))
     "[Arg list not available until function definition is loaded.]")
    (t t)))
-- 
2.11.1