branch: elpa/flycheck
commit 66ce4095a66be3f872f9677058449837272622c8
Author: Bozhidar Batsov <[email protected]>
Commit: Bozhidar Batsov <[email protected]>
Migrate dockerfile-hadolint to the SARIF parser
hadolint's --format sarif output carries proper severities, ids and
messages, so the checker no longer needs error patterns that break when
hadolint changes its human-readable format. This is the first consumer
of flycheck-parse-sarif.
Also handle zero-width SARIF regions (start == end) as point locations,
which hadolint emits for line-level findings; otherwise they would
produce empty highlights.
---
CHANGES.rst | 8 ++++
flycheck.el | 66 +++++++++++++++++++--------------
test/specs/languages/test-dockerfile.el | 9 ++---
test/specs/test-error-parsers.el | 39 +++++++++++++++++++
4 files changed, 90 insertions(+), 32 deletions(-)
diff --git a/CHANGES.rst b/CHANGES.rst
index dd2cac81eb..004184804f 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -2,6 +2,14 @@
=======================
- Drop support for Emacs 27; Flycheck now requires Emacs 28.1 or newer.
+- The ``dockerfile-hadolint`` checker now parses hadolint's SARIF output
+ (``--format sarif``) via ``flycheck-parse-sarif`` instead of matching
+ its text output with error patterns, so it no longer breaks when
+ hadolint tweaks its human-readable format.
+- ``flycheck-parse-sarif`` now treats a zero-width SARIF region (where
+ the start and end positions coincide, as some tools emit for
+ line-level findings) as spanning the whole line, instead of producing
+ an empty highlight.
- Add ``flycheck-parse-sarif``, a ready-made ``:error-parser`` for the
SARIF output format that many analyzers can emit. Like the existing
``flycheck-parse-checkstyle``, checker definitions can point
diff --git a/flycheck.el b/flycheck.el
index 4c8505df23..bb18828a4a 100644
--- a/flycheck.el
+++ b/flycheck.el
@@ -8028,18 +8028,38 @@ information about SARIF."
(seq-map
(lambda (location)
(let-alist location
- (flycheck-error-new-at
- .physicalLocation.region.startLine
- .physicalLocation.region.startColumn
- level message
- :id id
- :checker checker
- :buffer buffer
- :filename
- (flycheck-parse-sarif--uri
- .physicalLocation.artifactLocation.uri)
- :end-line .physicalLocation.region.endLine
- :end-column .physicalLocation.region.endColumn)))
+ (let* ((start-line
+ .physicalLocation.region.startLine)
+ (start-col
+ .physicalLocation.region.startColumn)
+ (end-line
+ .physicalLocation.region.endLine)
+ (end-col
+ .physicalLocation.region.endColumn)
+ ;; A zero-width region carries no span:
+ ;; endColumn equals startColumn on the same
+ ;; line (endLine defaults to startLine per
+ ;; the SARIF spec). Some tools emit these
+ ;; for line-level findings, so treat them as
+ ;; the whole line -- drop the column and end
+ ;; and let the highlighting mode take over,
+ ;; rather than highlight an empty range.
+ (zero-width
+ (and end-col
+ (equal end-col start-col)
+ (or (null end-line)
+ (equal end-line start-line)))))
+ (flycheck-error-new-at
+ start-line (unless zero-width start-col)
+ level message
+ :id id
+ :checker checker
+ :buffer buffer
+ :filename
+ (flycheck-parse-sarif--uri
+ .physicalLocation.artifactLocation.uri)
+ :end-line (unless zero-width end-line)
+ :end-column (unless zero-width end-col)))))
.locations)
;; A result without a location applies to the whole run
(list (flycheck-error-new-at
@@ -9275,25 +9295,17 @@ Requires DMD 2.066 or newer. See URL
`https://dlang.org/'."
"A Dockerfile syntax checker using hadolint.
See URL `https://github.com/hadolint/hadolint/'."
- :command ("hadolint" "--no-color" "-")
+ :command ("hadolint" "--format" "sarif" "-")
:standard-input t
- :error-patterns
- ((error line-start
- (file-name) ":" line " " (id (one-or-more alnum)) " error: "
(message)
- line-end)
- (warning line-start
- (file-name) ":" line " " (id (one-or-more alnum))
- " warning: " (message) line-end)
- (info line-start
- (file-name) ":" line " " (id (one-or-more alnum)) " info: " (message)
- line-end)
- (error line-start
- (file-name) ":" line ":" column " " (message)
- line-end))
+ :error-parser flycheck-parse-sarif
:error-filter
(lambda (errors)
+ ;; hadolint reports stdin as "-" for lint findings but as
+ ;; "/dev/stdin" for parse errors; strip both so the errors attach to
+ ;; the current buffer
(flycheck-sanitize-errors
- (flycheck-remove-error-file-names "-" errors)))
+ (flycheck-remove-error-file-names
+ "/dev/stdin" (flycheck-remove-error-file-names "-" errors))))
:modes (dockerfile-mode dockerfile-ts-mode))
(defun flycheck-credo--working-directory (&rest _ignored)
diff --git a/test/specs/languages/test-dockerfile.el
b/test/specs/languages/test-dockerfile.el
index 8ae2e176a1..211a3fb05a 100644
--- a/test/specs/languages/test-dockerfile.el
+++ b/test/specs/languages/test-dockerfile.el
@@ -7,17 +7,16 @@
(flycheck-buttercup-def-checker-test dockerfile-hadolint dockerfile error
(flycheck-buttercup-should-syntax-check
"language/dockerfile/Dockerfile.error" 'dockerfile-mode
- '(2 1 error "unexpected 'I' expecting '#', '\\', ADD, ARG, CMD, COPY,
ENTRYPOINT, ENV, EXPOSE, FROM, HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN,
SHELL, STOPSIGNAL, USER, VOLUME, WORKDIR, at least one space, or end of input"
- :checker dockerfile-hadolint)))
+ '(2 nil error "unexpected 'I'
+expecting '#', '\\', ADD, ARG, CMD, COPY, ENTRYPOINT, ENV, EXPOSE, FROM,
HEALTHCHECK, LABEL, MAINTAINER, ONBUILD, RUN, SHELL, STOPSIGNAL, USER, VOLUME,
WORKDIR, a pragma, at least one space, or end of input"
+ :id "DL1000" :checker dockerfile-hadolint)))
(flycheck-buttercup-def-checker-test dockerfile-hadolint dockerfile warnings
(flycheck-buttercup-should-syntax-check
"language/dockerfile/Dockerfile.warning" 'dockerfile-mode
'(1 nil warning "Always tag the version of an image explicitly"
:id "DL3006" :checker dockerfile-hadolint)
- '(2 nil error "Do not use apt-get upgrade or dist-upgrade"
- :id "DL3005" :checker dockerfile-hadolint)
- '(2 nil info "Delete the apt-get lists after installing something"
+ '(2 nil info "Delete the apt lists (/var/lib/apt/lists) after installing
something"
:id "DL3009" :checker dockerfile-hadolint)
'(3 nil error "Use absolute WORKDIR"
:id "DL3000" :checker dockerfile-hadolint))))
diff --git a/test/specs/test-error-parsers.el b/test/specs/test-error-parsers.el
index 37b036c3f9..347a9ebb72 100644
--- a/test/specs/test-error-parsers.el
+++ b/test/specs/test-error-parsers.el
@@ -111,6 +111,45 @@
(car (flycheck-parse-sarif sarif 'checker 'buffer)))
:to-equal "/abs/path.py")))
+ (it "treats a zero-width region as the whole line"
+ ;; A start == end region carries no span; drop the column and end so
+ ;; the whole line is highlighted rather than an empty range. Also
+ ;; covers endLine omitted, which defaults to startLine.
+ (dolist (region
'("\"startLine\":2,\"startColumn\":1,\"endLine\":2,\"endColumn\":1"
+ "\"startLine\":2,\"startColumn\":4,\"endColumn\":4"))
+ (let* ((sarif (concat
"{\"runs\":[{\"tool\":{\"driver\":{\"name\":\"d\"}},\
+\"results\":[{\"message\":{\"text\":\"m\"},\"locations\":[{\"physicalLocation\":\
+{\"artifactLocation\":{\"uri\":\"a.txt\"},\"region\":{" region "}}}]}]}]}"))
+ (err (car (flycheck-parse-sarif sarif 'checker 'buffer))))
+ (expect (flycheck-error-line err) :to-equal 2)
+ (expect (flycheck-error-column err) :to-be nil)
+ (expect (flycheck-error-end-line err) :to-be nil)
+ (expect (flycheck-error-end-column err) :to-be nil))))
+
+ (it "keeps a non-empty region's end position"
+ (let ((sarif "{\"runs\":[{\"tool\":{\"driver\":{\"name\":\"d\"}},\
+\"results\":[{\"message\":{\"text\":\"m\"},\"locations\":[{\"physicalLocation\":\
+{\"artifactLocation\":{\"uri\":\"a.txt\"},\"region\":\
+{\"startLine\":2,\"startColumn\":3,\"endLine\":2,\"endColumn\":7}}}]}]}]}"))
+ (let ((err (car (flycheck-parse-sarif sarif 'checker 'buffer))))
+ (expect (flycheck-error-column err) :to-equal 3)
+ (expect (flycheck-error-end-line err) :to-equal 2)
+ (expect (flycheck-error-end-column err) :to-equal 7))))
+
+ (it "keeps multiple results at the same location distinct"
+ (let ((sarif "{\"runs\":[{\"tool\":{\"driver\":{\"name\":\"d\"}},\
+\"results\":[\
+{\"ruleId\":\"A\",\"message\":{\"text\":\"first\"},\"locations\":[\
+{\"physicalLocation\":{\"artifactLocation\":{\"uri\":\"a.txt\"},\
+\"region\":{\"startLine\":3}}}]},\
+{\"ruleId\":\"B\",\"message\":{\"text\":\"second\"},\"locations\":[\
+{\"physicalLocation\":{\"artifactLocation\":{\"uri\":\"a.txt\"},\
+\"region\":{\"startLine\":3}}}]}]}]}"))
+ (expect (mapcar (lambda (e) (list (flycheck-error-id e)
+ (flycheck-error-message e)))
+ (flycheck-parse-sarif sarif 'checker 'buffer))
+ :to-equal '(("A" "first") ("B" "second")))))
+
(it "utf-8-decodes percent escapes in file URIs"
(let ((sarif "{\"runs\":[{\"tool\":{\"driver\":{\"name\":\"d\"}},\
\"results\":[{\"message\":{\"text\":\"m\"},\"locations\":[{\"physicalLocation\":\
