branch: elpa/flycheck
commit a988009dc4e798b692e44e701f0d2ffb8c95fd20
Author: Bozhidar Batsov <[email protected]>
Commit: Bozhidar Batsov <[email protected]>
Fix remote-execution gaps found in review
Complete the TRAMP support with correctness fixes the initial pass
missed:
- protoc's --proto_path and pug's -p passed the raw TRAMP file name to
the remote binary; reduce them with file-local-name like the sibling
arguments already were.
- flycheck-temp-directory reported the local /tmp for source checkers
while the temp files are created on the remote host, so a checker
could be judged usable when the remote temp dir is not writable. Probe
the host-aware (temporary-file-directory) instead.
- The proselint version cache was a single global, so a version detected
on one host was reused on another; key it by host.
- flycheck-compile (via flycheck-checker-shell-command) resolved the
program and source paths locally, so running a checker as a compile
command in a remote buffer targeted the wrong host. Resolve the
executable on the remote host and localize the file arguments.
- Move the possibly-remote file-exists-p check in
flycheck-may-check-automatically after the trigger-set check, so a
disallowed trigger no longer blocks on a remote stat while typing.
---
flycheck.el | 66 +++++++++++++++++++++-------------
test/specs/languages/test-proselint.el | 26 ++++++++++++++
2 files changed, 67 insertions(+), 25 deletions(-)
diff --git a/flycheck.el b/flycheck.el
index 333419edfd..78b7397ec8 100644
--- a/flycheck.el
+++ b/flycheck.el
@@ -1684,7 +1684,10 @@ Return the path of the file."
Return nil if the CHECKER does not write temporary files."
(let ((args (flycheck-checker-arguments checker)))
(cond
- ((memq 'source args) temporary-file-directory)
+ ;; `flycheck-temp-file-system' creates the file with
+ ;; `make-nearby-temp-file', i.e. on the host of `default-directory',
+ ;; so probe that host's temporary directory, not the local one.
+ ((memq 'source args) (temporary-file-directory))
((memq 'source-inplace args)
(if buffer-file-name (file-name-directory buffer-file-name)
temporary-file-directory))
@@ -3816,7 +3819,6 @@ If CONDITIONS are given, determine whether syntax may be
checked
under at least one of them, according to
`flycheck-check-syntax-automatically'."
(and (not (or buffer-read-only (flycheck-ephemeral-buffer-p)))
- (file-exists-p default-directory)
(or (not conditions)
(let ((allowed
;; Remote buffers use a narrower set of trigger events by
@@ -3827,7 +3829,10 @@ under at least one of them, according to
flycheck-check-syntax-automatically-remote
flycheck-check-syntax-automatically)))
(seq-some (lambda (condition) (memq condition allowed))
- conditions)))))
+ conditions)))
+ ;; Checked last so a disallowed trigger short-circuits before this
+ ;; possibly-remote (and thus blocking) stat of `default-directory'.
+ (file-exists-p default-directory)))
(defvar-local flycheck--idle-trigger-timer nil
"Timer used to trigger a syntax check after an idle delay.")
@@ -7555,7 +7560,9 @@ use with `compilation-error-regexp-alist'."
Like `flycheck-substitute-argument', except for source,
source-inplace, and source-original."
(if (memq arg '(source source-inplace source-original))
- (list buffer-file-name)
+ ;; The command runs on the host of `default-directory', so strip any
+ ;; remote prefix from the file name.
+ (list (file-local-name buffer-file-name))
(flycheck-substitute-argument arg checker)))
(defun flycheck--checker-substituted-shell-command-arguments (checker)
@@ -7587,8 +7594,9 @@ shell execution."
(abs-prog
;; The executable path returned by
`flycheck-command-wrapper-function'
;; may not be absolute, so expand it here. See URL
- ;; `https://github.com/flycheck/flycheck/issues/1461'.
- (or (executable-find (car wrapped))
+ ;; `https://github.com/flycheck/flycheck/issues/1461'. Resolve it on
+ ;; the host the command runs on when `default-directory' is remote.
+ (or (executable-find (car wrapped) (file-remote-p default-directory))
(user-error "Cannot find `%s' using `executable-find'"
(car wrapped))))
(command (mapconcat #'shell-quote-argument
@@ -7596,7 +7604,8 @@ shell execution."
(if (flycheck-checker-get checker 'standard-input)
;; If the syntax checker expects the source from standard input add an
;; appropriate shell redirection
- (concat command " < " (shell-quote-argument (buffer-file-name)))
+ (concat command " < "
+ (shell-quote-argument (file-local-name (buffer-file-name))))
command)))
(defun flycheck-compile-name (_name)
@@ -11323,26 +11332,31 @@ See URL `https://proselint.com/' for more information
about proselint."
(let-alist (car response)
.result.<stdin>.diagnostics)))))
-(defvar flycheck--proselint-use-old-args 'unknown
- "Cache for proselint version detection.
-Value is t for old (<= 0.14.0), nil for new (>= 0.16.0),
-or `unknown' if not yet detected.")
+(defvar flycheck--proselint-use-old-args (make-hash-table :test 'equal)
+ "Cache for proselint version detection, keyed by host.
+The key is the remote identifier of `default-directory' (see
+`file-remote-p'), or nil for the local host, since the proselint
+on each host may have a different version. Each value is t for
+old (<= 0.14.0) proselint and nil for new (>= 0.16.0); a host
+absent from the table has not been probed yet.")
(defvar flycheck-proselint-executable)
(defun flycheck--proselint-args ()
- "Return command arguments for proselint, detecting the version once."
- (when (eq flycheck--proselint-use-old-args 'unknown)
- (setq flycheck--proselint-use-old-args
- ;; Probe on the host the check will run on (remote over TRAMP)
- (zerop (process-file
- (or flycheck-proselint-executable "proselint")
- nil nil nil "--version"))))
- (if flycheck--proselint-use-old-args
- ;; Proselint versions <= 0.14.0:
- (list "--json" "-")
- ;; Proselint versions >= 0.16.0
- (list "check" "--output-format=json")))
+ "Return command arguments for proselint, detecting the version once per
host."
+ (let ((host (file-remote-p default-directory)))
+ (when (eq (gethash host flycheck--proselint-use-old-args 'unknown)
'unknown)
+ (puthash host
+ ;; Probe on the host the check will run on (remote over TRAMP).
+ (zerop (process-file
+ (or flycheck-proselint-executable "proselint")
+ nil nil nil "--version"))
+ flycheck--proselint-use-old-args))
+ (if (gethash host flycheck--proselint-use-old-args)
+ ;; Proselint versions <= 0.14.0:
+ (list "--json" "-")
+ ;; Proselint versions >= 0.16.0
+ (list "check" "--output-format=json"))))
(flycheck-define-checker proselint
"Flycheck checker using Proselint.
@@ -11374,7 +11388,8 @@ See URL
`https://developers.google.com/protocol-buffers/'."
(flycheck-temp-dir-system))))
;; Add the current directory to resolve imports
(eval (concat "--proto_path="
- (file-name-directory (buffer-file-name))))
+ (file-local-name
+ (file-name-directory (buffer-file-name)))))
;; Add other import paths; this needs to be after the current
;; directory to produce the right output. See URL
;; `https://github.com/flycheck/flycheck/pull/1655'
@@ -11395,7 +11410,8 @@ See URL
`https://developers.google.com/protocol-buffers/'."
"A Pug syntax checker using the pug compiler.
See URL `https://pugjs.org/'."
- :command ("pug" "-p" (eval (expand-file-name (buffer-file-name))))
+ :command ("pug" "-p"
+ (eval (file-local-name (expand-file-name (buffer-file-name)))))
:standard-input t
:error-patterns
;; errors with includes/extends (e.g. missing files)
diff --git a/test/specs/languages/test-proselint.el
b/test/specs/languages/test-proselint.el
index 7e1c480aa9..73c949d8fc 100644
--- a/test/specs/languages/test-proselint.el
+++ b/test/specs/languages/test-proselint.el
@@ -4,6 +4,32 @@
(require 'test-helpers)
(describe "Language Proselint"
+
+ (describe "flycheck--proselint-args"
+ (before-each
+ (clrhash flycheck--proselint-use-old-args))
+
+ (it "probes a host once and caches the detected version"
+ ;; Exit 0 -> old proselint, which takes the "--json -" arguments.
+ (spy-on 'process-file :and-return-value 0)
+ (let ((default-directory "/tmp/"))
+ (expect (flycheck--proselint-args) :to-equal '("--json" "-"))
+ (expect (flycheck--proselint-args) :to-equal '("--json" "-")))
+ (expect 'process-file :to-have-been-called-times 1))
+
+ (it "detects the version independently on each host"
+ ;; `file-remote-p' parses the prefix without connecting, so the probe
+ ;; can be faked per host without a live remote.
+ (spy-on 'process-file :and-call-fake
+ (lambda (&rest _)
+ (if (file-remote-p default-directory) 1 0)))
+ (let ((default-directory "/tmp/"))
+ (expect (flycheck--proselint-args) :to-equal '("--json" "-")))
+ (let ((default-directory "/ssh:host:/tmp/"))
+ (expect (flycheck--proselint-args)
+ :to-equal '("check" "--output-format=json")))
+ (expect 'process-file :to-have-been-called-times 2)))
+
(flycheck-buttercup-def-checker-test proselint (text markdown) nil
(let ((flycheck-disabled-checkers '(markdown-markdownlint-cli
markdown-markdownlint-cli2 markdown-mdl markdown-pymarkdown)))
(flycheck-buttercup-with-env '(("LC_ALL" . nil))
