[nongnu] elpa/with-simulated-input 8f335ad321 108/134: Implement warnings for inputs that have no effect

[ELPA Syncer]

branch: elpa/with-simulated-input
commit 8f335ad321759b268d100ae490dd3c4d56e3f322
Author: Ryan C. Thompson <rct@thompsonclan.org>
Commit: Ryan C. Thompson <rct@thompsonclan.org>

    Implement warnings for inputs that have no effect
---
 tests/test-with-simulated-input.el | 33 +++++++++++-------
 with-simulated-input.el            | 71 +++++++++++++++++++++++++++++++-------
 2 files changed, 78 insertions(+), 26 deletions(-)

diff --git a/tests/test-with-simulated-input.el 
b/tests/test-with-simulated-input.el
index d7140867aa..2e689e0a42 100644
--- a/tests/test-with-simulated-input.el
+++ b/tests/test-with-simulated-input.el
@@ -28,7 +28,13 @@
   "Like `progn', but evaluate BODY entirely at runtime.
 
 This is useful if BODY involves macros and you want to defer the
-expansion of those macros until BODY is evaluated."
+expansion of those macros until BODY is evaluated.
+
+Note: I don't recommend this function for general use, because it
+doesn't seem to properly put BODY in the correct lexical scope,
+but it's good enough for use in this test suite. Lexical scopes
+established *inside* BODY work just fine, so just make sure to
+put this outside any relevant `let' forms."
   `(eval
     '(progn
        ,@(cl-loop for expr in body
@@ -288,18 +294,19 @@ during macro expansion will be caught as well."
   ;; non-string literal, or any expression known to involve only pure
   ;; functions.
   (it "should ignore the return value of non-literal expressions in KEYS"
-    (let ((desired-input "hello")
-          (undesired-input "goodbye"))
-      (expect
-       (with-simulated-input
-           '((prog1 undesired-input
-               ;; This is the only thing that should actually get
-               ;; inserted.
-               (insert desired-input))
-             undesired-input
-             "RET")
-         (read-string "Enter a string: "))
-       :to-equal desired-input)))
+    (expect-warning
+     (let ((desired-input "hello")
+           (undesired-input "goodbye"))
+       (expect
+        (with-simulated-input
+            '((prog1 undesired-input
+                ;; This is the only thing that should actually get
+                ;; inserted.
+                (insert desired-input))
+              undesired-input
+              "RET")
+          (read-string "Enter a string: "))
+        :to-equal desired-input))))
 
   (it "should throw an error if the input is incomplete"
     (expect
diff --git a/with-simulated-input.el b/with-simulated-input.el
index ea94a0565a..187c9ff369 100644
--- a/with-simulated-input.el
+++ b/with-simulated-input.el
@@ -191,21 +191,30 @@ in `progn'."
            body)))
   ;; TODO Support integers (i.e. single characters) in KEYS
   (cond
-   ((null body)
-    (display-warning 'with-simulated-input
-                     "BODY is empty; KEYS will not be used")
-    nil)
+   ;; This case applies when BODY consists of only constant
+   ;; expressions (or no expressions at all). Since all the
+   ;; expressions are constant, there's no point in evaluating any of
+   ;; them except the last one, and there's no possibility that any
+   ;; input will be read, so we can skip all the proprocessing and
+   ;; just return the last element of BODY.
    ((not (cl-find-if-not #'hack-one-local-variable-constantp body))
-    (display-warning 'with-simulated-input
-                     "BODY consists of only constant expressions; KEYS will 
not be used")
-    ;; Since all the expressions are constant, there's no point in
-    ;; evaluating any of them except the last one.
+    (display-warning
+     'with-simulated-input
+     (if body
+         "BODY consists of only constant expressions; KEYS will be ignored."
+       "BODY is empty; KEYS will be ignored."))
     (car (last body)))
+   ;; If KEYS is nil, we don't have to do any pre-processing on it. We
+   ;; still need to call `with-simulated-input-1', which will evaluate
+   ;; BODY and throw an error if it tries to read input.
    ((null keys)
     `(with-simulated-input-1
       (lambda ()
         ,@body)
       nil))
+   ;; If KEYS is a symbol, then it is a variable reference. This is
+   ;; kind of janky and should probably be deprecated, except possibly
+   ;; in the case where it evaluates to a string.
    ((and keys (symbolp keys))
     `(cond
       ((null ,keys)
@@ -227,10 +236,16 @@ in `progn'."
       (t
        (error "KEYS must be a string or list, not %s: %s = %S"
               (type-of ,keys) ',keys ,keys))))
+   ;; If KEYS is a list whose first element is not `quote', then it is
+   ;; a function call, whose return value will be used as the value of
+   ;; KEYS. This is *definitely* deprecated.
    ((and (listp keys)
          (not (eq (car keys) 'quote))
          (or (functionp (car keys))
              (macrop (car keys))))
+    (display-warning
+     'with-simulated-input
+     "Passing a function call as KEYS is deprecated and will not be supported 
in future releases.")
     (let ((evaluated-keys-sym (make-symbol "evaluated-keys")))
       `(let ((,evaluated-keys-sym (,@keys)))
          (pcase ,evaluated-keys-sym
@@ -238,26 +253,56 @@ in `progn'."
             (prog1 (setq ,evaluated-keys-sym x)
               (display-warning
                'with-simulated-input
-               "Passing KEYS as a quoted list is deprecated.")))
+               "Passing KEYS as a quoted list is deprecated and will not be 
supported in future releases.")))
            ((guard (not (listp ,evaluated-keys-sym))) (cl-callf list 
,evaluated-keys-sym)))
          (apply
           #'with-simulated-input-1
           (lambda ()
             ,@body)
-          (cl-loop for key in ,evaluated-keys-sym collect (if (stringp key) 
key `(lambda () ,key)))))))
+          (cl-loop
+           for key in ,evaluated-keys-sym
+           if (stringp key) collect key
+           ;; It is occasionally useful to include nil as an element of
+           ;; KEYS, so we don't produce a warning for it.
+           else if (null key) do (ignore)
+           else if (hack-one-local-variable-constantp key) do
+           (display-warning
+            'with-simulated-input-1
+            "Non-string forms in KEYS are evaluated for side effects only. 
Non-string constants in KEYS have no effect.")
+           else if (symbolp key) do
+           (display-warning
+            'with-simulated-input-1
+            "Non-string forms in KEYS are evaluated for side effects only. 
Variables in KEYS have no effect.")
+           else collect `(lambda () ,key))))))
+   ;; The primary supported KEYS syntax: either a string, or an
+   ;; un-quoted list of strings and list expressions to execute as
+   ;; input.
    (t
-    ;; (message "Keys is something else: %S" keys)
     (pcase keys
       (`(quote ,x)
        (prog1 (setq keys x)
          (display-warning
           'with-simulated-input
-          "Passing KEYS as a quoted list is deprecated.")))
+          "Passing KEYS as a quoted list is deprecated and will not be 
supported in future releases.")))
       ((guard (not (listp keys))) (cl-callf list keys)))
     `(with-simulated-input-1
       (lambda ()
         ,@body)
-      ,@(cl-loop for key in keys collect (if (stringp key) key `(lambda () 
,key)))))))
+      ,@(cl-loop
+         for key in keys
+         if (stringp key) collect key
+         ;; It is occasionally useful to include nil as an element of
+         ;; KEYS, so we don't produce a warning for it.
+         else if (null key) do (ignore)
+         else if (hack-one-local-variable-constantp key) do
+         (display-warning
+          'with-simulated-input-1
+          "Non-string forms in KEYS are evaluated for side effects only. 
Non-string constants in KEYS have no effect.")
+         else if (symbolp key) do
+         (display-warning
+          'with-simulated-input-1
+          "Non-string forms in KEYS are evaluated for side effects only. 
Variables in KEYS have no effect.")
+         else collect `(lambda () ,key))))))
 
 (defvar wsi-simulated-idle-time nil
   "The current simulated idle time.