Subject: bug#65030: 30.0.50; Check keyword args of make-process

From: Helmut Eller <[email-masked]>
Date: Thu, 03 Aug 2023 08:47:44 +0200

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

>From 758ba9b8b26333fc60ca4693616c5f2b355d4fcc Mon Sep 17 00:00:00 2001
From: Helmut Eller <[email-masked]>
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")
+
 
 ;;;; Macro expansion.
 
-- 
2.39.2