branch: elpa/flycheck
commit ed0f9b546130fa367dcef8b3be396dc9f62d8b94
Author: Bozhidar Batsov <[email protected]>
Commit: Bozhidar Batsov <[email protected]>
[Fix #1787] Add :handle-suspicious property for command checkers
When a checker exits with a non-zero code without producing any
parsable errors, Flycheck reports a cryptic "suspicious state"
warning and eventually disables the checker. Tool-wrapper checkers
like flycheck-eldev know how to make sense of such failures (missing
dependencies, broken -pkg.el files), so give them a hook to translate
the output into regular errors shown in the buffer.
Based on the patch from #1795.
Co-authored-by: Paul Pogonyshev <[email protected]>
---
CHANGES.rst | 5 ++++
flycheck.el | 59 +++++++++++++++++++++++++++++++-------
test/specs/test-command-checker.el | 43 +++++++++++++++++++++++++++
test/specs/test-helpers.el | 18 ++++++++++++
4 files changed, 114 insertions(+), 11 deletions(-)
diff --git a/CHANGES.rst b/CHANGES.rst
index a5a3d50744..ff83cca0bd 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -2,6 +2,11 @@
=======================
- Drop support for Emacs 27; Flycheck now requires Emacs 28.1 or newer.
+- [#1787]: Add the ``:handle-suspicious`` property to command checkers.
+ It lets a checker translate a "suspicious state" (a non-zero exit
+ status with no parsable errors, e.g. a missing dependency) into
+ regular errors shown in the buffer, instead of the generic
+ suspicious-state warning.
- [#2161]: Fix the ``org-lint`` checker erroring out on Emacs 31, where
``org-lint`` reports line numbers as strings.
- [#2174]: Fix the ``haskell-ghc`` and ``haskell-stack-ghc`` checkers
diff --git a/flycheck.el b/flycheck.el
index 7b2c55f45e..168b2d5f0f 100644
--- a/flycheck.el
+++ b/flycheck.el
@@ -5893,6 +5893,23 @@ of command checkers is `flycheck-sanitize-errors'.
`flycheck-parse-with-patterns'. In this case,
`:error-patterns' is mandatory.
+`:handle-suspicious FUNCTION'
+ A function to handle suspicious state: when the process
+ returns non-zero code, but no standard errors (i.e. using
+ `:error-patterns' or `:error-parser') are found.
+
+ The function is called with three arguments: CHECKER,
+ EXIT-STATUS and OUTPUT (as string) with the checked buffer
+ 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.
+
+ This property is optional. If omitted, such state is always
+ treated as suspicious.
+
`:standard-input t'
Whether to send the buffer contents on standard input.
@@ -5943,6 +5960,7 @@ default `:verify' function of command checkers."
(patterns (plist-get properties :error-patterns))
(parser (or (plist-get properties :error-parser)
#'flycheck-parse-with-patterns))
+ (handle-suspicious (plist-get properties :handle-suspicious))
(enabled (plist-get properties :enabled))
(standard-input (plist-get properties :standard-input)))
(unless command
@@ -5980,10 +5998,11 @@ default `:verify' function of command checkers."
(car p)))
patterns)))
(pcase-dolist (`(,prop . ,value)
- `((command . ,command)
- (error-parser . ,parser)
- (error-patterns . ,patterns)
- (standard-input . ,standard-input)))
+ `((command . ,command)
+ (error-parser . ,parser)
+ (error-patterns . ,patterns)
+ (handle-suspicious . ,handle-suspicious)
+ (standard-input . ,standard-input)))
(setf (flycheck-checker-get symbol prop) value)))))
(eval-and-compile
@@ -6479,16 +6498,22 @@ 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))
- ;; 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 \
+ ;; 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 \
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)))
+output checker)))))
(funcall callback 'finished
;; Fix error file names, by substituting them backwards from the
;; temporaries.
@@ -6877,6 +6902,15 @@ CHECKER. BUFFER is the buffer which was checked.
Return the errors parsed with the error patterns of CHECKER."
(funcall (flycheck-checker-get checker 'error-parser) output checker buffer))
+(defun flycheck-handle-suspicious-state (checker exit-status output)
+ "Handle suspicious state of given CHECKER.
+EXIT-STATUS and OUTPUT are passed to `:handle-suspicious'
+function of the CHECKER, if any."
+ (if-let* ((handle-suspicious
+ (flycheck-checker-get checker 'handle-suspicious)))
+ (funcall handle-suspicious checker exit-status output)
+ 'suspicious))
+
(defun flycheck-fix-error-filename (err buffer-files cwd)
"Fix the file name of ERR from BUFFER-FILES.
@@ -7448,6 +7482,7 @@ SYMBOL with `flycheck-def-executable-var'."
(let ((command (plist-get properties :command))
(parser (plist-get properties :error-parser))
(filter (plist-get properties :error-filter))
+ (handle-suspicious (plist-get properties :handle-suspicious))
(explainer (plist-get properties :error-explainer))
(predicate (plist-get properties :predicate))
(enabled-fn (plist-get properties :enabled))
@@ -7464,6 +7499,8 @@ SYMBOL with `flycheck-def-executable-var'."
:error-patterns ',(plist-get properties :error-patterns)
,@(when filter
`(:error-filter #',filter))
+ ,@(when handle-suspicious
+ `(:handle-suspicious #',handle-suspicious))
,@(when explainer
`(:error-explainer #',explainer))
:modes ',(plist-get properties :modes)
diff --git a/test/specs/test-command-checker.el
b/test/specs/test-command-checker.el
index da48c10c25..cba1fc89cd 100644
--- a/test/specs/test-command-checker.el
+++ b/test/specs/test-command-checker.el
@@ -128,6 +128,49 @@
(expect (shut-up (flycheck-buttercup-should-syntax-check-in-buffer))
:to-throw 'flycheck-buttercup-suspicious-checker))))
+ (it "reports errors returned by the :handle-suspicious function"
+ (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)
+ (list (flycheck-error-new-at
+ 1 1 'error
+ (format "exited with %d: %s" exit-status
+ (string-trim output))
+ :checker checker)))))
+ (flycheck-buttercup-with-temp-buffer
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ (flycheck-buttercup-should-syntax-check-in-buffer
+ '(1 1 error "exited with 1: tool exploded"
+ :checker suspicious-exit)))))
+
+ (it "stays suspicious when the :handle-suspicious function declines"
+ (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) 'suspicious)))
+ (flycheck-buttercup-with-temp-buffer
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ (expect (shut-up (flycheck-buttercup-should-syntax-check-in-buffer))
+ :to-throw 'flycheck-buttercup-suspicious-checker))))
+
+ (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)
+ ((symbol-plist 'suspicious-exit)
+ flycheck-test--suspicious-exit))
+ (flycheck-buttercup-with-temp-buffer
+ (suspicious-exit-mode)
+ (insert "hello\n")
+ (expect (shut-up (flycheck-buttercup-should-syntax-check-in-buffer))
+ :to-throw 'flycheck-buttercup-suspicious-checker))))
+
(it "forces English messages without overriding the character set"
;; LC_MESSAGES=C gives English output, while LC_ALL is left alone so
;; that the subprocess keeps the user's LC_CTYPE (see #2170).
diff --git a/test/specs/test-helpers.el b/test/specs/test-helpers.el
index 9ac1424247..a9ac73768d 100644
--- a/test/specs/test-helpers.el
+++ b/test/specs/test-helpers.el
@@ -181,6 +181,24 @@ The manifest path is relative to
(setf (symbol-plist 'print-locale) nil)
+;;; Suspicious-state test helpers (for command-checker tests)
+
+(define-derived-mode suspicious-exit-mode prog-mode "suspicious")
+
+(flycheck-define-command-checker 'suspicious-exit
+ "Exit with a non-zero code without producing parsable errors."
+ :command `(,(or (executable-find "python3") "python")
+ "-c" "import sys; print('tool exploded'); sys.exit(1)")
+ :error-patterns '((error bol "never-matches:" line ":" (message)))
+ :modes '(suspicious-exit-mode))
+
+(defconst flycheck-test--suspicious-exit
+ (symbol-plist 'suspicious-exit)
+ "Saved plist for the suspicious-exit checker.")
+
+(setf (symbol-plist 'suspicious-exit) nil)
+
+
;;; Custom error level for error-level tests
(flycheck-define-error-level 'test-level
