From 0a06c1e41bcc119da29f46e2f9e1e85da06dc5b1 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Mon, 20 Mar 2023 17:25:54 -0700 Subject: [PATCH 3/3] 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). For globbing, we want to pass the literal glob to Pcomplete so it can use the glob for selecting completion candidates. For subcommands (including Lisp forms), we especially want to avoid evaluation, since they can produce arbitary side effects! (Bug#50470) * lisp/eshell/esh-cmd.el (eshell-allow-commands): New variable... (eshell-commands-forbidden): New error... (eshell-named-command, eshell-lisp-command): ... use them. * lisp/eshell/em-cmpl.el (eshell-complete--eval-argument-form): Disallow command forms and handle errors ourselves. (eshell-complete-parse-arguments): Don't parse glob characters. * 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 | 60 ++++++++++++++++++++----------- lisp/eshell/esh-cmd.el | 15 ++++++++ test/lisp/eshell/em-cmpl-tests.el | 59 +++++++++++++++++++++++++++++- 3 files changed, 113 insertions(+), 21 deletions(-) diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el index b65652019d4..732bbb3f1fa 100644 --- a/lisp/eshell/em-cmpl.el +++ b/lisp/eshell/em-cmpl.el @@ -306,9 +306,24 @@ eshell--pcomplete-insert-tab (defun eshell-complete--eval-argument-form (arg) "Evaluate a single Eshell argument form ARG for the purposes of completion." - (let ((result (eshell-do-eval `(eshell-commands ,arg) t))) - (cl-assert (eq (car result) 'quote)) - (cadr result))) + (condition-case err + (let* (;; Don't allow running commands; they could have + ;; arbitrary side effects, which we don't want when we're + ;; just performing completions! + (eshell-allow-commands) + ;; Handle errors ourselves so that we can properly catch + ;; `eshell-commands-forbidden'. + (eshell-handle-errors) + (result (eshell-do-eval `(eshell-commands ,arg) t))) + (cl-assert (eq (car result) 'quote)) + (cadr result)) + (eshell-commands-forbidden + (propertize "\0" 'eshell-argument-stub + (intern (format "%s-command" (cadr err))))) + (error + (lwarn 'eshell :error + "Failed to evaluate argument form during completion: %S" arg) + (propertize "\0" 'eshell-argument-stub 'error)))) (defun eshell-complete-parse-arguments () "Parse the command line arguments for `pcomplete-argument'." @@ -325,23 +340,28 @@ eshell-complete-parse-arguments (if (= begin end) (end-of-line)) (setq end (point-marker))) - (if (setq delim - (catch 'eshell-incomplete - (ignore - (setq args (eshell-parse-arguments begin end))))) - (cond ((member (car delim) '("{" "${" "$<")) - (setq begin (1+ (cadr delim)) - args (eshell-parse-arguments begin end))) - ((member (car delim) '("$'" "$\"" "#<")) - ;; Add the (incomplete) argument to our arguments, and - ;; note its position. - (setq args (append (nth 2 delim) (list (car delim))) - incomplete-arg t) - (push (- (nth 1 delim) 2) posns)) - ((member (car delim) '("(" "$(")) - (throw 'pcompleted (elisp-completion-at-point))) - (t - (eshell--pcomplete-insert-tab)))) + ;; Don't expand globs when parsing arguments; we want to pass any + ;; globs to Pcomplete unaltered. + (declare-function eshell-parse-glob-chars "em-glob" ()) + (let ((eshell-parse-argument-hook (remq #'eshell-parse-glob-chars + eshell-parse-argument-hook))) + (if (setq delim + (catch 'eshell-incomplete + (ignore + (setq args (eshell-parse-arguments begin end))))) + (cond ((member (car delim) '("{" "${" "$<")) + (setq begin (1+ (cadr delim)) + args (eshell-parse-arguments begin end))) + ((member (car delim) '("$'" "$\"" "#<")) + ;; Add the (incomplete) argument to our arguments, and + ;; note its position. + (setq args (append (nth 2 delim) (list (car delim))) + incomplete-arg t) + (push (- (nth 1 delim) 2) posns)) + ((member (car delim) '("(" "$(")) + (throw 'pcompleted (elisp-completion-at-point))) + (t + (eshell--pcomplete-insert-tab))))) (when (get-text-property (1- end) 'comment) (eshell--pcomplete-insert-tab)) (let ((pos (1- end))) diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el index 1a458290dfe..d5237ee1f04 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -293,6 +293,17 @@ eshell-last-async-procs When the process in the CDR completes, resume command evaluation.") +(defvar eshell-allow-commands t + "If non-nil, allow evaluating command forms (including Lisp forms). +If you want to forbid command forms, you can let-bind this to a +non-nil value before calling `eshell-do-eval'. Then, any command +forms will signal `eshell-commands-forbidden'. This is useful +if, for example, you want to evaluate simple expressions like +variable expansions, but not fully-evaluate the command. See +also `eshell-complete-parse-arguments'.") + +(define-error 'eshell-commands-forbidden "Commands forbidden") + ;;; Functions: (defsubst eshell-interactive-process-p () @@ -1328,6 +1339,8 @@ eshell/which (defun eshell-named-command (command &optional args) "Insert output from a plain COMMAND, using ARGS. COMMAND may result in an alias being executed, or a plain command." + (unless eshell-allow-commands + (signal 'eshell-commands-forbidden '(named))) (setq eshell-last-arguments args eshell-last-command-name (eshell-stringify command)) (run-hook-with-args 'eshell-prepare-command-hook) @@ -1465,6 +1478,8 @@ eshell-last-output-end (defun eshell-lisp-command (object &optional args) "Insert Lisp OBJECT, using ARGS if a function." + (unless eshell-allow-commands + (signal 'eshell-commands-forbidden '(lisp))) (catch 'eshell-external ; deferred to an external command (setq eshell-last-command-status 0 eshell-last-arguments args) diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el index ea907f1945d..1f7712e23d1 100644 --- a/test/lisp/eshell/em-cmpl-tests.el +++ b/test/lisp/eshell/em-cmpl-tests.el @@ -123,6 +123,36 @@ 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" ,(propertize + "\0" 'eshell-argument-stub 'named-command))))) + (with-temp-eshell + (insert "echo ${echo hi}") + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize + "\0" 'eshell-argument-stub 'named-command)))))) + +(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" ,(propertize + "\0" 'eshell-argument-stub 'lisp-command))))) + (with-temp-eshell + (insert "echo $(concat \"hi\")") + (should (eshell-arguments-equal + (car (eshell-complete-parse-arguments)) + `("echo" ,(propertize + "\0" 'eshell-argument-stub 'lisp-command)))))) + (ert-deftest em-cmpl-test/file-completion/unique () "Test completion of file names when there's a unique result." (with-temp-eshell @@ -150,6 +180,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 +198,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 ." @@ -174,7 +228,10 @@ em-cmpl-test/lisp-function-completion See ." (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 \"#