bug#57884: [PATCH] Flymake backend using the shellcheck program

[Philip Kaludercic]

Philip Kaludercic <philipk@posteo.net> writes:

>> Anyway, I rewrote the backend to use the JSON output of shellcheck,
>> which has the advantage that it provides the end position of each
>> diagnostic, so Flymake doesn't have to guess it (which is by nature
>> sometimes inaccurate).  Let me know what you think.
>
> LGTM, but I haven't tested it yet.

I just tried it out and it behaves the way you advertised it.

BTW, this diff describes the changes required if you were to pull out
the sentinel definition into a named function:

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 558b62b20a..d52e385b36 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -3129,50 +3129,49 @@ 'sh--json-read
     (require 'json)
     'json-read))
 
+(defun sh-shellcheck-sentinel (proc _event)
+  (when (memq (process-status proc) '(exit signal))
+    (unwind-protect
+        (if (with-current-buffer (process-get proc 'source)
+              (not (eq proc sh--shellcheck-process)))
+            (flymake-log :warning "Canceling obsolete check %s" proc)
+          (with-current-buffer (process-buffer proc)
+            (goto-char (point-min))
+            (thread-last
+              (sh--json-read)
+              (alist-get 'comments)
+              (seq-filter
+               (lambda (item)
+                 (let-alist item (string= .file "-"))))
+              (mapcar
+               (lambda (item)
+                 (let-alist item
+                   (flymake-make-diagnostic
+                    (process-get proc 'source)
+                    (cons .line .column)
+                    (unless (and (eq .line .endLine)
+                                 (eq .column .endColumn))
+                      (cons .endLine .endColumn))
+                    (pcase .level
+                      ("error" :error)
+                      ("warning" :warning)
+                      (_ :note))
+                    (format "SC%s: %s" .code .message)))))
+              (funcall (process-get proc 'report-fn)))))
+      (kill-buffer (process-buffer proc)))))
+
 (defun sh-shellcheck-flymake (report-fn &rest _args)
   "Flymake backend using the shellcheck program.
 Takes a Flymake callback REPORT-FN as argument, as expected of a
 member of `flymake-diagnostic-functions'."
   (when (process-live-p sh--shellcheck-process)
     (kill-process sh--shellcheck-process))
-  (let* ((source (current-buffer))
-         (dialect (named-let recur ((s sh-shell))
+  (let* ((dialect (named-let recur ((s sh-shell))
                     (pcase s
                       ((or 'bash 'dash 'sh) (symbol-name s))
                       ('ksh88 "ksh")
                       ((guard s)
-                       (recur (alist-get s sh-ancestor-alist))))))
-         (sentinel
-          (lambda (proc _event)
-            (when (memq (process-status proc) '(exit signal))
-              (unwind-protect
-                  (if (with-current-buffer source
-                        (not (eq proc sh--shellcheck-process)))
-                      (flymake-log :warning "Canceling obsolete check %s" proc)
-                    (with-current-buffer (process-buffer proc)
-                      (goto-char (point-min))
-                      (thread-last
-                        (sh--json-read)
-                        (alist-get 'comments)
-                        (seq-filter
-                         (lambda (item)
-                           (let-alist item (string= .file "-"))))
-                        (mapcar
-                         (lambda (item)
-                           (let-alist item
-                             (flymake-make-diagnostic
-                              source
-                              (cons .line .column)
-                              (unless (and (eq .line .endLine)
-                                           (eq .column .endColumn))
-                                (cons .endLine .endColumn))
-                              (pcase .level
-                                ("error" :error)
-                                ("warning" :warning)
-                                (_ :note))
-                              (format "SC%s: %s" .code .message)))))
-                        (funcall report-fn))))
-                (kill-buffer (process-buffer proc)))))))
+                       (recur (alist-get s sh-ancestor-alist)))))))
     (unless dialect
       (error "`sh-shellcheck-flymake' is not suitable for shell type `%s'"
              sh-shell))
@@ -3185,7 +3184,9 @@ sh-shellcheck-flymake
                       "-s" ,dialect
                       ,@sh-shellcheck-arguments
                       "-")
-           :sentinel sentinel))
+           :sentinel #'sh-shellcheck-sentinel))
+    (process-put sh--shellcheck-process 'source (current-buffer))
+    (process-put sh--shellcheck-process 'report-fn report-fn)
     (save-restriction
       (widen)
       (process-send-region sh--shellcheck-process (point-min) (point-max))

I still don't think it looks that bad, but I don't insist on it.