From: Helmut Eller <eller.helmut@gmail.com>
To: 65030@debbugs.gnu.org
Subject: bug#65030: 30.0.50; Check keyword args of make-process
Date: Thu, 03 Aug 2023 08:47:44 +0200 [thread overview]
Message-ID: <m2zg38r59r.fsf@gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 608 bytes --]
The functions make-process and make-network-process have many keyword
arguments and it's somewhat easy to misspell some of them. E.g. using
:coding-system instead of :coding. These functions don't detect such
mistakes at runtime. What would people think about adding some checks
as a compiler macro as with the patch below?
I didn't know where to put this, so I just left it in bytecomp.el.
Perhaps the advertised-calling-convention declaration could do this, but
since keyword arguments seem to be generally discouraged, a special case
for make-process and make-network-process maybe simpler.
Helmut
[-- Attachment #2: 0001-Check-keyword-args-of-make-process.patch --]
[-- Type: text/x-diff, Size: 6874 bytes --]
From 758ba9b8b26333fc60ca4693616c5f2b355d4fcc Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut@gmail.com>
Date: Thu, 3 Aug 2023 08:33:40 +0200
Subject: [PATCH] Check keyword args of make-process
The functions make-process and make-network-process have many
keyword args and it's easy to misspell some of them.
Use a compiler macro to warn about some possible mistakes.
* lisp/emacs-lisp/bytecomp.el (bytecomp--check-keyword-args): New
helper.
(make-process, make-network-process): Define a compiler macro that
performs some checks but doesn't anything else.
* test/lisp/emacs-lisp/bytecomp-tests.el: Add some tests.
* test/lisp/emacs-lisp/bytecomp-resources/:
(warn-make-process-missing-keyword-arg.el,
warn-make-process-missing-keyword-value.el,
warn-make-process-repeated-keyword-arg.el,
warn-make-process-unknown-keyword-arg.el): New test files
---
lisp/emacs-lisp/bytecomp.el | 61 +++++++++++++++++++
.../warn-make-process-missing-keyword-arg.el | 3 +
...warn-make-process-missing-keyword-value.el | 3 +
.../warn-make-process-repeated-keyword-arg.el | 3 +
.../warn-make-process-unknown-keyword-arg.el | 4 ++
test/lisp/emacs-lisp/bytecomp-tests.el | 16 +++++
6 files changed, 90 insertions(+)
create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 5b1d958e6c2..d3a434b5593 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -5782,6 +5782,67 @@ bytecomp--backward-word
form ; arity error
`(forward-word (- (or ,arg 1)))))
+(defun bytecomp--check-keyword-args (form arglist allowed-keys required-keys)
+ (let ((fun (car form)))
+ (cl-flet ((missing (form keyword)
+ (byte-compile-warn-x
+ form
+ "`%S´ called without required keyword argument %S"
+ fun keyword))
+ (unrecognized (form keyword)
+ (byte-compile-warn-x
+ form
+ "`%S´ called with unknown keyword argument %S"
+ fun keyword))
+ (duplicate (form keyword)
+ (byte-compile-warn-x
+ form
+ "`%S´ called with repeated keyword argument %S"
+ fun keyword))
+ (missing-val (form keyword)
+ (byte-compile-warn-x
+ form
+ "missing value for keyword argument %S"
+ keyword)))
+ (let* ((seen '())
+ (l arglist))
+ (while (consp l)
+ (let ((key (car l)))
+ (cond ((and (keywordp key) (memq key allowed-keys))
+ (cond ((memq key seen)
+ (duplicate l key))
+ (t
+ (push key seen))))
+ (t (unrecognized l key)))
+ (when (null (cdr l))
+ (missing-val l key)))
+ (setq l (cddr l)))
+ (dolist (key required-keys)
+ (unless (memq key seen)
+ (missing form key))))))
+ form)
+
+(put 'make-process 'compiler-macro
+ #'(lambda (form &rest args)
+ (bytecomp--check-keyword-args
+ form args
+ '(:name
+ :buffer :command :coding :noquery :stop :connection-type
+ :filter :sentinel :stderr :file-handler)
+ '(:name :command))))
+
+(put 'make-network-process 'compiler-macro
+ #'(lambda (form &rest args)
+ (bytecomp--check-keyword-args
+ form args
+ '(:name
+ :buffer :host :service :type :family :local :remote :coding
+ :nowait :noquery :stop :filter :filter-multibyte :sentinel
+ :log :plist :tls-parameters :server :broadcast :dontroute
+ :keepalive :linger :oobinline :priority :reuseaddr :bindtodevice
+ :use-external-socket)
+ '(:name :service))))
+
(provide 'byte-compile)
(provide 'bytecomp)
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
new file mode 100644
index 00000000000..9369e78ff54
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-arg.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+ (make-process :name "ls"))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
new file mode 100644
index 00000000000..4226349afef
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-missing-keyword-value.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+ (make-process :name "ls" :command))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
new file mode 100644
index 00000000000..18250f14ee9
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-repeated-keyword-arg.el
@@ -0,0 +1,3 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+ (make-process :name "ls" :command "ls" :name "ls"))
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el
new file mode 100644
index 00000000000..4721035780b
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-make-process-unknown-keyword-arg.el
@@ -0,0 +1,4 @@
+;;; -*- lexical-binding: t -*-
+(defun foo ()
+ (make-process :name "ls" :command "ls"
+ :coding-system 'binary))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 593fd117685..6ed907ead7b 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1199,6 +1199,22 @@ bytecomp--tests-obsolete-var
"nowarn-inline-after-defvar.el"
"Lexical argument shadows" 'reverse)
+(bytecomp--define-warning-file-test
+ "warn-make-process-missing-keyword-arg.el"
+ "called without required keyword argument :command")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-unknown-keyword-arg.el"
+ "called with unknown keyword argument :coding-system")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-repeated-keyword-arg.el"
+ "called with repeated keyword argument :name")
+
+(bytecomp--define-warning-file-test
+ "warn-make-process-missing-keyword-value.el"
+ "missing value for keyword argument :command")
+
\f
;;;; Macro expansion.
--
2.39.2
next reply other threads:[~2023-08-03 6:47 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 6:47 Helmut Eller [this message]
2023-08-05 9:21 ` bug#65030: 30.0.50; Check keyword args of make-process Eli Zaretskii
2023-08-05 23:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-08-06 4:58 ` Eli Zaretskii
2023-08-06 8:49 ` Mattias Engdegård
2023-08-08 8:52 ` Robert Pluim
2023-08-08 9:16 ` Mattias Engdegård
2023-08-08 9:27 ` Robert Pluim
2023-08-08 9:42 ` Mattias Engdegård
2023-08-08 12:16 ` Eli Zaretskii
2023-08-08 13:05 ` Mattias Engdegård
2023-08-08 13:18 ` Robert Pluim
2023-08-08 16:38 ` Mattias Engdegård
2023-08-08 17:14 ` Helmut Eller
2023-08-08 20:15 ` Mattias Engdegård
2023-08-08 17:43 ` Eli Zaretskii
2023-08-08 17:51 ` Mattias Engdegård
2023-08-08 17:54 ` Stefan Kangas
2023-08-08 18:28 ` Eli Zaretskii
2023-08-08 13:37 ` Visuwesh
2023-08-08 13:52 ` Robert Pluim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2zg38r59r.fsf@gmail.com \
--to=eller.helmut@gmail.com \
--cc=65030@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).