bug#50470: 27.1; 'company-mode' 'eshell'

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 460 bytes --]

On 3/17/2023 11:36 PM, Jim Porter wrote:
> Hopefully I can get it finished up over the weekend.
Ok, here we are. This adds a new defvar called 
'eshell-parse-for-completion-p', which Eshell argument parsers can 
consult to adjust their behavior. In particular, when that variable is 
true, it means a) don't expand globs (let Pcomplete handle that), and b) 
return a stub for subcommands and Lisp function forms (we don't want to 
execute these inadvertently).

[-- Attachment #2: 0001-Avoid-parsing-some-Eshell-forms-when-performing-comp.patch --]
[-- Type: text/plain, Size: 14277 bytes --]

From 7f1bce1768f0072dd469f9047a59ffd1fde8b4f4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 18 Mar 2023 15:39:57 -0700
Subject: [PATCH] Avoid parsing some Eshell forms when performing completion

During completion, we want to evaluate most Eshell forms
(e.g. variable references), but skip others (e.g. globbing,
subcommands, Lisp forms).  For globbing, we want to pass the literal
glob to Pcomplete so it can use the glob for selecting completion
candidates.  For subcommands and Lisp forms in particular, we
especially want to avoid evaluation, since they can produce arbitary
side effects!  (Bug#50470)

* lisp/eshell/em-cmpl.el (eshell-parse-for-completion-p): New
variable...
(eshell-complete-parse-arguments): ... let-bind it to 't'.

* lisp/eshell/em-glob.el (eshell-parse-glob-chars):
* lisp/eshell/esh-var.el (eshell-parse-variable-ref):
* lisp/eshell/esh-cmd.el (eshell-parse-subcommand-argument)
(eshell-parse-lisp-argument): Check 'eshell-parse-for-completion-p'.
(eshell-do-eval): Use 'car-safe' when checking the body of a 'let'
form.

* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/parse-arguments/unevaluated-subcommand)
(em-cmpl-test/parse-arguments/unevaluated-lisp-form)
(em-cmpl-test/file-completion/glob, em-cmpl-test/command-completion)
(em-cmpl-test/subcommand-completion): New tests.
(em-cmpl-test/lisp-function-completion): Check "$(func)" syntax.
---
 lisp/eshell/em-cmpl.el            | 13 +++++-
 lisp/eshell/em-glob.el            |  3 +-
 lisp/eshell/esh-cmd.el            | 27 ++++++-----
 lisp/eshell/esh-var.el            | 78 +++++++++++++++++--------------
 test/lisp/eshell/em-cmpl-tests.el | 56 +++++++++++++++++++++-
 5 files changed, 126 insertions(+), 51 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index b65652019d4..5714aeaabfb 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -85,6 +85,16 @@ eshell-cmpl
   :tag "Argument completion"
   :group 'eshell-module))
 
+;;; Internal variables:
+
+(defvar eshell-parse-for-completion-p nil
+  "This is set to t before calling `eshell-parse-arguments' for completion.
+Hooks for `eshell-parse-argument-hook' should consult this to
+adjust their behavior when parsing a command for completion, if
+necessary.  For example, subcommands should return some stub
+value when this is set so that the completion code doesn't try to
+invoke the subcommand.")
+
 ;;; User Variables:
 
 (defcustom eshell-cmpl-load-hook nil
@@ -328,7 +338,8 @@ eshell-complete-parse-arguments
     (if (setq delim
 	      (catch 'eshell-incomplete
 		(ignore
-		 (setq args (eshell-parse-arguments begin end)))))
+                 (setq args (let ((eshell-parse-for-completion-p t))
+                              (eshell-parse-arguments begin end))))))
         (cond ((member (car delim) '("{" "${" "$<"))
 	       (setq begin (1+ (cadr delim))
 		     args (eshell-parse-arguments begin end)))
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index 8a2ba13b2ad..7517ee57833 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -162,7 +162,8 @@ eshell-parse-glob-chars
 The character is not advanced for ordinary globbing characters, so
 that other function may have a chance to override the globbing
 interpretation."
-  (when (memq (char-after) eshell-glob-chars-list)
+  (when (and (not (bound-and-true-p eshell-parse-for-completion-p))
+             (memq (char-after) eshell-glob-chars-list))
     (if (not (memq (char-after) '(?\( ?\[)))
 	(ignore (eshell-add-glob-modifier))
       (let ((here (point)))
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 93f2616020c..9ca3d06fc22 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -675,13 +675,15 @@ eshell-parse-subcommand-argument
 	   (or (= (point-max) (1+ (point)))
 	       (not (eq (char-after (1+ (point))) ?\}))))
       (let ((end (eshell-find-delimiter ?\{ ?\})))
-	(if (not end)
-            (throw 'eshell-incomplete "{")
-	  (when (eshell-arg-delimiter (1+ end))
-	    (prog1
+        (unless end
+          (throw 'eshell-incomplete "{"))
+        (when (eshell-arg-delimiter (1+ end))
+          (prog1
+              (if (bound-and-true-p eshell-parse-for-completion-p)
+                  "(unevaluated subcommand)"
 		`(eshell-as-subcommand
-                  ,(eshell-parse-command (cons (1+ (point)) end)))
-	      (goto-char (1+ end))))))))
+                  ,(eshell-parse-command (cons (1+ (point)) end))))
+            (goto-char (1+ end)))))))
 
 (defun eshell-parse-lisp-argument ()
   "Parse a Lisp expression which is specified as an argument."
@@ -689,14 +691,15 @@ eshell-parse-lisp-argument
 	   (not eshell-current-quoted)
 	   (looking-at eshell-lisp-regexp))
       (let* ((here (point))
-	     (obj
+             (lisp-form
 	      (condition-case nil
 		  (read (current-buffer))
-		(end-of-file
-                 (throw 'eshell-incomplete "(")))))
+                (end-of-file (throw 'eshell-incomplete "(")))))
 	(if (eshell-arg-delimiter)
-	    `(eshell-command-to-value
-              (eshell-lisp-command (quote ,obj)))
+            (if (bound-and-true-p eshell-parse-for-completion-p)
+                "(unevaluated lisp form)"
+              `(eshell-command-to-value
+                (eshell-lisp-command ',lisp-form)))
 	  (ignore (goto-char here))))))
 
 (defun eshell-separate-commands (terms separator &optional
@@ -1168,7 +1171,7 @@ eshell-do-eval
 	(setcar (cdr args) (eshell-do-eval (cadr args) synchronous-p))
 	(eval form))
        ((eq (car form) 'let)
-        (when (not (eq (car (cadr args)) 'eshell-do-eval))
+        (unless (eq (car-safe (cadr args)) 'eshell-do-eval)
           (eshell-manipulate "evaluating let args"
             (dolist (letarg (car args))
               (when (and (listp letarg)
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 5d6299af564..b7420f2437b 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -507,10 +507,12 @@ eshell-parse-variable-ref
   (cond
    ((eq (char-after) ?{)
     (let ((end (eshell-find-delimiter ?\{ ?\})))
-      (if (not end)
-          (throw 'eshell-incomplete "${")
-        (forward-char)
-        (prog1
+      (unless end
+        (throw 'eshell-incomplete "${"))
+      (forward-char)
+      (prog1
+          (if (bound-and-true-p eshell-parse-for-completion-p)
+              "(unevaluated subcommand)"
             `(eshell-apply-indices
               (eshell-convert
                (eshell-command-to-value
@@ -527,45 +529,49 @@ eshell-parse-variable-ref
                ;; just be joined back together afterwards.
                ,(when (and (not modifier-p) eshell-current-quoted)
                   '(not indices)))
-              indices ,eshell-current-quoted)
-          (goto-char (1+ end))))))
+              indices ,eshell-current-quoted))
+        (goto-char (1+ end)))))
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
-      (if (not end)
-          (throw 'eshell-incomplete "$<")
-        (let* ((temp (make-temp-file temporary-file-directory))
-               (cmd (concat (buffer-substring (1+ (point)) end)
-                            " > " temp)))
-          (prog1
+      (unless end
+        (throw 'eshell-incomplete "$<"))
+      (let* ((temp (make-temp-file temporary-file-directory))
+             (cmd (concat (buffer-substring (1+ (point)) end)
+                          " > " temp)))
+        (prog1
+            (if (bound-and-true-p eshell-parse-for-completion-p)
+                "(unevaluated subcommand)"
               `(let ((eshell-current-handles
                       (eshell-create-handles ,temp 'overwrite)))
-                 (progn
-                   (eshell-as-subcommand
-                    ,(let ((eshell-current-quoted nil))
-                       (eshell-parse-command cmd)))
-                   (ignore
-                    (nconc eshell-this-command-hook
-                           ;; Quote this lambda; it will be evaluated
-                           ;; by `eshell-do-eval', which requires very
-                           ;; particular forms in order to work
-                           ;; properly.  See bug#54190.
-                           (list (function
-                                  (lambda ()
-                                    (delete-file ,temp)
-                                    (when-let ((buffer (get-file-buffer ,temp)))
-                                      (kill-buffer buffer)))))))
-                   (eshell-apply-indices ,temp indices ,eshell-current-quoted)))
-            (goto-char (1+ end)))))))
+                 (eshell-as-subcommand
+                  ,(let ((eshell-current-quoted nil))
+                     (eshell-parse-command cmd)))
+                 (ignore
+                  (nconc eshell-this-command-hook
+                         ;; Quote this lambda; it will be evaluated by
+                         ;; `eshell-do-eval', which requires very
+                         ;; particular forms in order to work
+                         ;; properly.  See bug#54190.
+                         (list (function
+                                (lambda ()
+                                  (delete-file ,temp)
+                                  (when-let ((buffer (get-file-buffer ,temp)))
+                                    (kill-buffer buffer)))))))
+                 (eshell-apply-indices ,temp indices
+                                       ,eshell-current-quoted)))
+          (goto-char (1+ end))))))
    ((eq (char-after) ?\()
-    (condition-case nil
+    (let ((lisp-form
+           (condition-case nil
+               (read (or (eshell-unescape-inner-double-quote (point-max))
+                         (current-buffer)))
+             (end-of-file (throw 'eshell-incomplete "$(")))))
+      (if (bound-and-true-p eshell-parse-for-completion-p)
+          "(unevaluated lisp form)"
         `(eshell-apply-indices
           (eshell-command-to-value
-           (eshell-lisp-command
-            ',(read (or (eshell-unescape-inner-double-quote (point-max))
-                        (current-buffer)))))
-          indices ,eshell-current-quoted)
-      (end-of-file
-       (throw 'eshell-incomplete "$("))))
+           (eshell-lisp-command ',lisp-form))
+          indices ,eshell-current-quoted))))
    ((looking-at (rx-to-string
                  `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
     (eshell-with-temp-command
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index ea907f1945d..e0976c380cb 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -123,6 +123,33 @@ em-cmpl-test/parse-arguments/variable/splice
               (car (eshell-complete-parse-arguments))
               '("echo" "foo" "bar"))))))
 
+(ert-deftest em-cmpl-test/parse-arguments/unevaluated-subcommand ()
+  "Test that subcommands return a stub when parsing for completion."
+  (with-temp-eshell
+   (insert "echo {echo hi}")
+   (should (eshell-arguments-equal
+            (car (eshell-complete-parse-arguments))
+            '("echo" "(unevaluated subcommand)"))))
+  (with-temp-eshell
+   (insert "echo ${echo hi}")
+   (should (eshell-arguments-equal
+            (car (eshell-complete-parse-arguments))
+            `("echo" ,(propertize "(unevaluated subcommand)"
+                                  'escaped t))))))
+
+(ert-deftest em-cmpl-test/parse-arguments/unevaluated-lisp-form ()
+  "Test that Lisp forms return a stub when parsing for completion."
+  (with-temp-eshell
+   (insert "echo (concat \"hi\")")
+   (should (eshell-arguments-equal
+            (car (eshell-complete-parse-arguments))
+            '("echo" "(unevaluated lisp form)"))))
+  (with-temp-eshell
+   (insert "echo (concat \"hi\")")
+   (should (eshell-arguments-equal
+            (car (eshell-complete-parse-arguments))
+            '("echo" "(unevaluated lisp form)")))))
+
 (ert-deftest em-cmpl-test/file-completion/unique ()
   "Test completion of file names when there's a unique result."
   (with-temp-eshell
@@ -150,6 +177,15 @@ em-cmpl-test/file-completion/non-unique
          (forward-line -1)
          (should (looking-at "Complete, but not unique")))))))
 
+(ert-deftest em-cmpl-test/file-completion/glob ()
+  "Test completion of file names using a glob."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (write-region nil nil (expand-file-name "file.el"))
+     (should (equal (eshell-insert-and-complete "echo fi*.el")
+                    "echo file.el ")))))
+
 (ert-deftest em-cmpl-test/file-completion/after-list ()
   "Test completion of file names after previous list arguments.
 See bug#59956."
@@ -159,6 +195,21 @@ em-cmpl-test/file-completion/after-list
      (should (equal (eshell-insert-and-complete "echo (list 1 2) fi")
                     "echo (list 1 2) file.txt ")))))
 
+(ert-deftest em-cmpl-test/command-completion ()
+  "Test completion of command names like \"command\"."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "listif")
+                  "listify "))))
+
+(ert-deftest em-cmpl-test/subcommand-completion ()
+  "Test completion of command names like \"{command}\"."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "{ listif")
+                  "{ listify ")))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo ${ listif")
+                  "echo ${ listify "))))
+
 (ert-deftest em-cmpl-test/lisp-symbol-completion ()
   "Test completion of Lisp forms like \"#'symbol\" and \"`symbol\".
 See <lisp/eshell/esh-cmd.el>."
@@ -174,7 +225,10 @@ em-cmpl-test/lisp-function-completion
 See <lisp/eshell/esh-cmd.el>."
   (with-temp-eshell
    (should (equal (eshell-insert-and-complete "echo (eshell/ech")
-                  "echo (eshell/echo"))))
+                  "echo (eshell/echo")))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $(eshell/ech")
+                  "echo $(eshell/echo"))))
 
 (ert-deftest em-cmpl-test/special-ref-completion/type ()
   "Test completion of the start of special references like \"#<buffer\".
-- 
2.25.1