branch: elpa/flycheck
commit 6a6901f350f76a4763e7e98a54675967bcb185c8
Author: Bozhidar Batsov <[email protected]>
Commit: Bozhidar Batsov <[email protected]>
[Fix #1129] Drop eslint's blocking config probe
The :enabled test of javascript-eslint ran eslint --print-config
synchronously before the first check in every buffer, freezing Emacs
for its duration - the single biggest complaint in #1129. Diagnose a
missing configuration from the check's own asynchronous output instead:
:handle-suspicious learns a new return value, disable, which disables
the checker in the buffer quietly, like a failing :enabled test. On
projects with a config the probe cost disappears entirely.
---
CHANGES.rst | 10 +++
flycheck.el | 133 +++++++++++++++++++++++++-------
test/specs/languages/test-javascript.el | 19 +++++
test/specs/test-command-checker.el | 88 +++++++++++++++++++++
4 files changed, 224 insertions(+), 26 deletions(-)
diff --git a/CHANGES.rst b/CHANGES.rst
index ae2df861e2..b53880ff66 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -2,6 +2,16 @@
=======================
- Drop support for Emacs 27; Flycheck now requires Emacs 28.1 or newer.
+- [#1129]: ``javascript-eslint`` no longer probes for a configuration
+ file with a blocking ``--print-config`` call before the first check in
+ every buffer, which used to freeze Emacs for its duration (the
+ interactive ``flycheck-verify-setup`` still probes). A fatal eslint
+ failure (a missing or broken configuration) is now diagnosed from the
+ check's own exit status: the checker disables itself in the buffer
+ with an echo-area notice naming the reason, and a fallback checker
+ takes over on the next automatic check. Checker authors can use the
+ same mechanism by returning ``disable`` or ``(disable . reason)`` from
+ ``:handle-suspicious``.
- A new syntax check now interrupts a still-running one and starts
immediately, instead of waiting for it to finish and displaying its
stale results in the meantime. This makes slow checkers (cargo, mypy
diff --git a/flycheck.el b/flycheck.el
index 9cfed98509..67943f401f 100644
--- a/flycheck.el
+++ b/flycheck.el
@@ -3520,6 +3520,16 @@ optional DATA. STATUS may be one of the following
symbols:
user needs to be informed about. DATA is an optional
message.
+`self-disabled'
+ The syntax checker diagnosed itself as inapplicable to the
+ buffer, e.g. a linter without a configuration file. The
+ checker is disabled in the buffer like a failing `:enabled'
+ test, and checker selection is re-run so that another
+ checker can take over. DATA is an optional reason string
+ for the echo-area notice.
+
+ This report finishes the current syntax check.
+
A syntax checker _must_ report a status at least once with any
symbol that finishes the current syntax checker. Otherwise
Flycheck gets stuck with the current syntax check.
@@ -3550,6 +3560,26 @@ discarded."
(message "Suspicious state from syntax checker %s: %s"
checker (or data "UNKNOWN!")))
(flycheck-report-status 'suspicious))
+ (`self-disabled
+ (when flycheck-mode
+ ;; Disable the checker like a failing `:enabled' test. A
+ ;; fallback checker is selected on the next automatic
+ ;; check, uniformly for all buffers. We deliberately don't
+ ;; force a fallback in this very cycle: doing so has to
+ ;; route through the automatic-check gates (which refuse
+ ;; e.g. read-only buffers) and re-runs earlier chain
+ ;; members, for a marginal gain over the next idle tick.
+ (cl-pushnew checker flycheck--automatically-disabled-checkers)
+ (message
+ (substitute-command-keys
+ "Flycheck: %s disabled itself in this buffer%s; \
+\\[universal-argument] \\[flycheck-disable-checker] re-enables it")
+ checker (if data (format " (%s)" data) ""))
+ ;; Complete the check without running the disabled
+ ;; checker's own `:next-checkers'
+ (flycheck-finish-current-syntax-check
+ nil (flycheck-syntax-check-working-directory syntax-check)
+ 'no-next)))
(`finished
(when flycheck-mode
;; Only report errors from the checker if Flycheck Mode is
@@ -3561,13 +3591,14 @@ discarded."
(error "Unknown status %s from syntax checker %s"
status checker))))))))
-(defun flycheck-finish-current-syntax-check (errors working-dir)
+(defun flycheck-finish-current-syntax-check (errors working-dir &optional
no-next)
"Finish the current syntax-check in the current buffer with ERRORS.
ERRORS is a list of `flycheck-error' objects reported by the
current syntax check in `flycheck-current-syntax-check'.
-Report all ERRORS and potentially start any next syntax checkers.
+Report all ERRORS and, unless NO-NEXT is non-nil, potentially
+start any next syntax checkers.
If the current syntax checker reported excessive errors, they are
truncated or discarded via `flycheck--handle-excessive-errors',
@@ -3584,7 +3615,8 @@ WORKING-DIR."
working-dir))))
(flycheck-report-current-errors
(flycheck--handle-excessive-errors checker errors))
- (let ((next-checker (flycheck-get-next-checker-for-buffer checker)))
+ (let ((next-checker (unless no-next
+ (flycheck-get-next-checker-for-buffer checker))))
(if next-checker
(flycheck-start-current-syntax-check next-checker)
(setq flycheck-current-syntax-check nil)
@@ -6496,9 +6528,22 @@ of command checkers is `flycheck-sanitize-errors'.
as current. It should process the output and return a list
of non-standard errors that best describe what exactly has
failed. The returned errors go through `:error-filter' just
- like regular parsed errors. If the function cannot make
- sense of the output, it should return symbol `suspicious' to
- indicate that what has happened is really not expected.
+ like regular parsed errors.
+
+ The function may also return symbol `disable', or a cons
+ cell `(disable . REASON)' with a reason string, when the
+ output shows that the checker doesn't apply to this buffer
+ at all, e.g. a linter reporting that it has no configuration
+ file: the checker is then disabled in the buffer like a
+ failing `:enabled' test, with an echo-area notice including
+ REASON, and checker selection re-runs so a fallback checker
+ can take over. This avoids probing for applicability with a
+ blocking process call in `:enabled'; the asynchronous check
+ itself serves as the probe.
+
+ If the function cannot make sense of the output, it should
+ return symbol `suspicious' to indicate that what has
+ happened is really not expected.
This property is optional. If omitted, such state is always
treated as suspicious.
@@ -7090,28 +7135,42 @@ Parse the OUTPUT and report an appropriate error status.
Resolve all errors in OUTPUT using CWD as working directory."
(let ((errors (flycheck-parse-output output checker (current-buffer))))
- (when (and (not (equal exit-status 0)) (null errors))
- ;; Give the checker a chance to recover from suspicious state:
- ;; exit status is nonzero, but there are no errors.
- (let ((recovered (flycheck-handle-suspicious-state checker exit-status
- output)))
- (if (listp recovered)
- (setf errors recovered)
- ;; Warn about a suspicious result from the syntax checker. We do
- ;; right after parsing the errors, before filtering, because a syntax
- ;; checker might report errors from other files (e.g. includes) even
- ;; if there are no errors in the file being checked.
- (funcall callback 'suspicious
- (format "Flycheck checker %S returned %S, but \
+ (let ((self-disabled nil))
+ (when (and (not (equal exit-status 0)) (null errors))
+ ;; Give the checker a chance to recover from suspicious state:
+ ;; exit status is nonzero, but there are no errors.
+ (let ((recovered (flycheck-handle-suspicious-state checker exit-status
+ output)))
+ (cond
+ ((or (eq recovered 'disable)
+ (and (consp recovered) (eq (car recovered) 'disable)))
+ ;; The checker diagnosed itself as inapplicable to this
+ ;; buffer, e.g. a linter without a configuration file. The
+ ;; status report performs the disabling, subject to the usual
+ ;; staleness checks, and finishes the syntax check.
+ (setq self-disabled t)
+ (funcall callback 'self-disabled
+ (and (consp recovered) (cdr recovered))))
+ ((listp recovered)
+ (setf errors recovered))
+ (t
+ ;; Warn about a suspicious result from the syntax checker. We do
+ ;; right after parsing the errors, before filtering, because a
+ ;; syntax checker might report errors from other files
+ ;; (e.g. includes) even if there are no errors in the file being
+ ;; checked.
+ (funcall callback 'suspicious
+ (format "Flycheck checker %S returned %S, but \
its output contained no errors: %s\nTry installing a more \
recent version of %S, and please open a bug report if the issue \
persists in the latest release. Thanks!" checker exit-status
-output checker)))))
- (funcall callback 'finished
- ;; Fix error file names, by substituting them backwards from the
- ;; temporaries.
- (mapcar (lambda (e) (flycheck-fix-error-filename e files cwd))
- errors))))
+output checker))))))
+ (unless self-disabled
+ (funcall callback 'finished
+ ;; Fix error file names, by substituting them backwards
+ ;; from the temporaries.
+ (mapcar (lambda (e) (flycheck-fix-error-filename e files cwd))
+ errors))))))
;;; Executables of command checkers.
@@ -10417,6 +10476,24 @@ for more information about the custom directories."
'javascript-eslint nil nil nil
"--print-config" (or buffer-file-name "index.js"))))
+(defun flycheck--eslint-handle-suspicious (_checker exit-status output)
+ "Disable the checker when eslint cannot lint at all.
+
+Eslint exits with status 2 on any fatal failure -- a missing or
+broken configuration, a crashing plugin -- rather than lint
+findings, so the checker cannot be used in this buffer. This
+matches the semantics of the blocking `--print-config' probe that
+previous versions ran in `:enabled' (see URL
+`https://github.com/flycheck/flycheck/issues/1129'), and doesn't
+depend on the wording of any particular eslint version. The
+first line of OUTPUT is included in the disable notice.
+
+Any other exit status without parsable errors is suspicious: it
+suggests an output format Flycheck fails to parse."
+ (if (eq exit-status 2)
+ (cons 'disable (car (split-string output "\n" t)))
+ 'suspicious))
+
(defun flycheck-parse-eslint (output checker buffer)
"Parse ESLint errors/warnings from JSON OUTPUT.
@@ -10470,7 +10547,11 @@ See URL `https://eslint.org/'."
"--stdin" "--stdin-filename" source-original)
:standard-input t
:error-parser flycheck-parse-eslint
- :enabled (lambda () (flycheck-eslint-config-exists-p))
+ ;; A missing eslint config is diagnosed from the check's own output
+ ;; (see `flycheck--eslint-handle-suspicious') instead of a blocking
+ ;; `--print-config' probe in `:enabled', which used to freeze Emacs on
+ ;; the first check in every buffer
+ :handle-suspicious flycheck--eslint-handle-suspicious
:modes (js-mode js-jsx-mode js2-mode js2-jsx-mode js3-mode rjsx-mode
typescript-mode js-ts-mode typescript-ts-mode tsx-ts-mode)
:working-directory flycheck-eslint--find-working-directory
diff --git a/test/specs/languages/test-javascript.el
b/test/specs/languages/test-javascript.el
index 72d5c27e7f..0bc52ad04f 100644
--- a/test/specs/languages/test-javascript.el
+++ b/test/specs/languages/test-javascript.el
@@ -73,6 +73,25 @@
:end-line 5
:end-column 2))))))
+ (describe "Fatal-failure handling"
+ (it "disables the checker on a fatal eslint exit"
+ ;; Exit status 2 is eslint's fatal-error status, used for missing
+ ;; and broken configurations alike, in every eslint version
+ (expect (flycheck--eslint-handle-suspicious
+ 'javascript-eslint 2
+ "ESLint couldn't find an eslint.config.(js|mjs|cjs) file.
+Some more explanation here.")
+ :to-equal
+ '(disable
+ . "ESLint couldn't find an eslint.config.(js|mjs|cjs) file.")))
+
+ (it "stays suspicious on unparsable lint results"
+ ;; Exit status 1 means eslint found lint problems; reaching the
+ ;; suspicious handler then means we failed to parse them
+ (expect (flycheck--eslint-handle-suspicious
+ 'javascript-eslint 1 "[{\"unexpected\": \"format\"}]")
+ :to-be 'suspicious)))
+
(describe "Checker tests"
(flycheck-buttercup-def-checker-test javascript-eslint javascript error
(let ((inhibit-message t))
diff --git a/test/specs/test-command-checker.el
b/test/specs/test-command-checker.el
index 3ace0e88ba..e8a8cdd504 100644
--- a/test/specs/test-command-checker.el
+++ b/test/specs/test-command-checker.el
@@ -184,6 +184,94 @@ afterwards."
(expect (shut-up (flycheck-buttercup-should-syntax-check-in-buffer))
:to-throw 'flycheck-buttercup-suspicious-checker))))
+ (it "disables the checker when the :handle-suspicious function says so"
+ (assume (or (executable-find "python3") (executable-find "python")))
+ (cl-letf* ((flycheck-checker 'suspicious-exit)
+ ((symbol-plist 'suspicious-exit)
+ flycheck-test--suspicious-exit)
+ ((flycheck-checker-get 'suspicious-exit 'handle-suspicious)
+ (lambda (_checker _exit-status _output) 'disable)))
+ (flycheck-buttercup-with-temp-buffer
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ ;; The check finishes cleanly with no errors and no suspicious
+ ;; warning, and the checker is disabled like a failed :enabled
+ (flycheck-buttercup-should-syntax-check-in-buffer)
+ (expect (flycheck-automatically-disabled-checker-p 'suspicious-exit)
+ :to-be-truthy))))
+
+ (it "selects the fallback checker after a self-disable"
+ (assume (or (executable-find "python3") (executable-find "python")))
+ (flycheck-buttercup-with-temp-buffer
+ (rename-buffer "flycheck-self-disable-test")
+ (cl-letf* (((symbol-plist 'suspicious-exit)
+ flycheck-test--suspicious-exit)
+ ((flycheck-checker-get 'suspicious-exit
+ 'handle-suspicious)
+ (lambda (_checker _exit-status _output) 'disable))
+ ((symbol-function 'get-buffer-window)
+ (lambda (&rest _) (selected-window))))
+ (flycheck-define-generic-checker 'fallback-checker
+ "Report a fixed warning synchronously."
+ :start (lambda (checker callback)
+ (funcall callback 'finished
+ (list (flycheck-error-new-at
+ 1 1 'warning "fallback ran"
+ :checker checker))))
+ :modes '(suspicious-exit-mode))
+ (unwind-protect
+ (let ((flycheck-checkers '(suspicious-exit fallback-checker)))
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ (flycheck-mode)
+ (shut-up (flycheck-buttercup-buffer-sync))
+ ;; The self-disabled checker is out of the running...
+ (expect (flycheck-automatically-disabled-checker-p
+ 'suspicious-exit)
+ :to-be-truthy)
+ ;; ...so the next check picks the fallback checker
+ (expect (flycheck-get-checker-for-buffer)
+ :to-be 'fallback-checker)
+ (shut-up (flycheck-buttercup-buffer-sync))
+ (expect (mapcar #'flycheck-error-message
+ flycheck-current-errors)
+ :to-equal '("fallback ran")))
+ (setf (symbol-plist 'fallback-checker) nil)))))
+
+ (it "does not run the chain of a self-disabled checker"
+ (assume (or (executable-find "python3") (executable-find "python")))
+ (flycheck-buttercup-with-temp-buffer
+ (rename-buffer "flycheck-chain-skip-test")
+ (cl-letf* (((symbol-plist 'suspicious-exit)
+ flycheck-test--suspicious-exit)
+ ((flycheck-checker-get 'suspicious-exit
+ 'handle-suspicious)
+ (lambda (_checker _exit-status _output) 'disable))
+ ((flycheck-checker-get 'suspicious-exit 'next-checkers)
+ '(chain-target))
+ ((symbol-function 'get-buffer-window)
+ (lambda (&rest _) (selected-window))))
+ (flycheck-define-generic-checker 'chain-target
+ "Report a fixed error synchronously."
+ :start (lambda (checker callback)
+ (funcall callback 'finished
+ (list (flycheck-error-new-at
+ 1 1 'error "chain ran"
+ :checker checker))))
+ :modes '(suspicious-exit-mode))
+ (unwind-protect
+ (let ((flycheck-checkers '(suspicious-exit)))
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ (flycheck-mode)
+ (shut-up (flycheck-buttercup-buffer-sync))
+ ;; The disabled checker's own chain must not run
+ (expect (flycheck-automatically-disabled-checker-p
+ 'suspicious-exit)
+ :to-be-truthy)
+ (expect flycheck-current-errors :not :to-be-truthy))
+ (setf (symbol-plist 'chain-target) nil)))))
+
(it "treats a non-zero exit without errors as suspicious by default"
(assume (or (executable-find "python3") (executable-find "python")))
(cl-letf* ((flycheck-checker 'suspicious-exit)
