From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Augusto Stoffel Newsgroups: gmane.emacs.bugs Subject: bug#57884: [PATCH] Flymake backend using the shellcheck program Date: Sun, 18 Sep 2022 21:38:17 +0200 Message-ID: <878rmgcw2e.fsf@gmail.com> References: <87a66yaqwc.fsf@gmail.com> <83bkre0w4m.fsf@gnu.org> <871qs9c3er.fsf@gmail.com> <87zgewdhhd.fsf@posteo.net> <87fsgoyi0l.fsf@gmail.com> <87sfko4zjq.fsf@posteo.net> <87edw8n71p.fsf@gmail.com> <8735co4wxx.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="15964"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: Eli Zaretskii , 57884@debbugs.gnu.org, Stefan Kangas To: Philip Kaludercic Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Sep 18 21:39:15 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oa08J-0003wu-Fj for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 18 Sep 2022 21:39:15 +0200 Original-Received: from localhost ([::1]:47188 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oa08I-00019L-H5 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 18 Sep 2022 15:39:14 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58810) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oa086-00016F-6c for bug-gnu-emacs@gnu.org; Sun, 18 Sep 2022 15:39:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:52287) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oa085-0006Sb-Ug for bug-gnu-emacs@gnu.org; Sun, 18 Sep 2022 15:39:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oa085-0001sm-Pm for bug-gnu-emacs@gnu.org; Sun, 18 Sep 2022 15:39:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Augusto Stoffel Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 18 Sep 2022 19:39:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57884 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 57884-submit@debbugs.gnu.org id=B57884.16635299087192 (code B ref 57884); Sun, 18 Sep 2022 19:39:01 +0000 Original-Received: (at 57884) by debbugs.gnu.org; 18 Sep 2022 19:38:28 +0000 Original-Received: from localhost ([127.0.0.1]:51365 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oa07Y-0001ru-CZ for submit@debbugs.gnu.org; Sun, 18 Sep 2022 15:38:28 -0400 Original-Received: from mail-ed1-f43.google.com ([209.85.208.43]:39748) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oa07V-0001rg-P2 for 57884@debbugs.gnu.org; Sun, 18 Sep 2022 15:38:26 -0400 Original-Received: by mail-ed1-f43.google.com with SMTP id f20so33977869edf.6 for <57884@debbugs.gnu.org>; Sun, 18 Sep 2022 12:38:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date; bh=MhQujCn2hyQrn4rG6Yivj01/JjXu7NvPBvIUwPD2qbk=; b=Q8ApxpbYJki9lFMTZKskE+UFon2dRT0q+WTpA0s5hd8IbtiAi+Nkxuxr2mGHIgFj8D KIkSIPt/zPGqhov24ZMe54abhv73CI0TKKDzxSTYOQMAId4tvsMx9O7DwkjUySqAuOsG RaqtgBfS3YjxKjqRcO6g1eWi8Bw43i6iE99Drwqc8xrv+hNbuE3mRa43NxY9O2/hs0sW GVlaciiv/Nnzf+CwsYo5JfnSBTiDPLfWUvGVzo4fR+RVtvmFLxBTzy1oUPIAaYuZJ54t EUlo9E+lhDPqxE5/i8lRQWx1pKgIW5KsAU/Kbg/iyfwmarn1gB5K5pKB9lzN9IwLXOIk 048g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date; bh=MhQujCn2hyQrn4rG6Yivj01/JjXu7NvPBvIUwPD2qbk=; b=sq11Bw2pnN5MdmnIlPCUIfpsbvodu9rQ8D0368e+ksCCCIWgiuozO3Y6N7dBI7vbku BQK1jnT9f/1anmYXkYPsYGXSP4Em/a6JzsSeloGsPaSWNhBmvVQHOm92ahwwmydpaHRD DgP3lY0BXWZE3A7j8SGC1R0PDkJPzbeqdx0ZdELriqsGqF73pBDbEnuNTFetPM0pbcGV jps43Vx9t3i6nDgoMwtxAqNJ1J4VM89oM6juCgyxR2ppOidoDQ01lpXJqcI/Lij/qDPz vj+chLBZf3lnTTCCPrRjRMvo2Q130TkIV+XUNXZwD1XCD4xxUMo7DmalL+/4VtWZ3AMV V8uA== X-Gm-Message-State: ACrzQf0gf8Q2Cc/pDbbxJnqGZn5blKggW5liZixxj0V7WQzxb7ZYxQLi FuAWyHqtK6lFJu0YfYZK5+o= X-Google-Smtp-Source: AMsMyM7ym4q5acpnGrGdF9Jec7IUs0Qnn2SRalnKcvLcRPGKpvGQ6AZ5TOWleMx/Tz4ZyXgNO9GyrA== X-Received: by 2002:a05:6402:2c6:b0:44e:7d0a:c231 with SMTP id b6-20020a05640202c600b0044e7d0ac231mr12480865edx.283.1663529899884; Sun, 18 Sep 2022 12:38:19 -0700 (PDT) Original-Received: from ars3 ([2a02:8109:8ac0:56d0::8510]) by smtp.gmail.com with ESMTPSA id d9-20020a50fb09000000b0045393e56488sm5021899edq.58.2022.09.18.12.38.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Sep 2022 12:38:18 -0700 (PDT) In-Reply-To: <8735co4wxx.fsf@posteo.net> (Philip Kaludercic's message of "Sun, 18 Sep 2022 13:46:34 +0000") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:243015 Archived-At: --=-=-= Content-Type: text/plain On Sun, 18 Sep 2022 at 13:46, Philip Kaludercic wrote: >> I like in your backend that you read a JSON output, which presumably >> provides the start _and end_ of the diagnostic region. How did you >> convert from line/column to buffer position? > > I didn't do that, Flymake takes care of that if you give it the buffer > position (unless I misunderstood something, but it appears to be working > for me). Okay, perhaps the docstring of `flymake-make-diagnostic' is easy to misinterpret, but apparently it works fine if one passes a (LINE . COL) cons for BEG and END, even if LOCUS is a buffer. This would also mean that the call to `flymake-diag-region` in the example backend of the Flymake manual is unnecessary, so I'm a bit suspicious here. 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. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-New-Flymake-backend-using-the-shellcheck-program.patch >From 49fbfaf98cb6d11b877eaa4a72b116aa61646d75 Mon Sep 17 00:00:00 2001 From: Augusto Stoffel Date: Sat, 17 Sep 2022 18:30:04 +0200 Subject: [PATCH] New Flymake backend using the shellcheck program See bug#57884. * lisp/progmodes/sh-script.el: Require let-alist and subr-x when compiling. (sh--json-read): Helper function to deal with possible absence of json-parse-buffer. (sh-shellcheck-program, sh--shellcheck-process, sh-shellcheck-flymake): Variables and function defining a Flymake backend. (sh-mode): Add it to 'flymake-diagnostic-functions'. --- etc/NEWS | 4 ++ lisp/progmodes/sh-script.el | 90 ++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/etc/NEWS b/etc/NEWS index a6a8883593..5b2ed16063 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1320,6 +1320,10 @@ This controls how statements like the following are indented: foo && bar +*** New Flymake backend using the ShellCheck program +It is enabled by default, but requires that the external "shellcheck" +command is installed. + ** Cperl Mode --- diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el index 517fbbd8e7..558b62b20a 100644 --- a/lisp/progmodes/sh-script.el +++ b/lisp/progmodes/sh-script.el @@ -31,6 +31,9 @@ ;; available for filenames, variables known from the script, the shell and ;; the environment as well as commands. +;; A Flymake backend using the "shellcheck" program is provided. See +;; https://www.shellcheck.net/ for installation instructions. + ;;; Known Bugs: ;; - In Bourne the keyword `in' is not anchored to case, for, select ... @@ -141,7 +144,9 @@ (eval-when-compile (require 'skeleton) (require 'cl-lib) - (require 'comint)) + (require 'comint) + (require 'let-alist) + (require 'subr-x)) (require 'executable) (autoload 'comint-completion-at-point "comint") @@ -1580,6 +1585,7 @@ sh-mode ((equal (file-name-nondirectory buffer-file-name) ".profile") "sh") (t sh-shell-file)) nil nil) + (add-hook 'flymake-diagnostic-functions #'sh-shellcheck-flymake nil t) (add-hook 'hack-local-variables-hook #'sh-after-hack-local-variables nil t)) @@ -3103,6 +3109,88 @@ sh-delete-backslash (delete-region (1+ (point)) (progn (skip-chars-backward " \t") (point))))))) +;;; Flymake backend + +(defcustom sh-shellcheck-program "shellcheck" + "Name of the shellcheck executable." + :type 'string + :version "29.1") + +(defcustom sh-shellcheck-arguments nil + "Additional arguments to the shellcheck program." + :type '(repeat string) + :version "29.1") + +(defvar-local sh--shellcheck-process nil) + +(defalias 'sh--json-read + (if (fboundp 'json-parse-buffer) + (lambda () (json-parse-buffer :object-type 'alist)) + (require 'json) + 'json-read)) + +(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)) + (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))))))) + (unless dialect + (error "`sh-shellcheck-flymake' is not suitable for shell type `%s'" + sh-shell)) + (setq sh--shellcheck-process + (make-process + :name "shellcheck" :noquery t :connection-type 'pipe + :buffer (generate-new-buffer " *flymake-shellcheck*") + :command `(,sh-shellcheck-program + "--format=json1" + "-s" ,dialect + ,@sh-shellcheck-arguments + "-") + :sentinel sentinel)) + (save-restriction + (widen) + (process-send-region sh--shellcheck-process (point-min) (point-max)) + (process-send-eof sh--shellcheck-process)))) + (provide 'sh-script) ;;; sh-script.el ends here -- 2.37.3 --=-=-= Content-Type: text/plain PS: What is the best practice w.r.t. the fact that json-serialize might not be available? It seems that every library that needs to parse JSON has to define a new alias similar to sh--json-read here. This is rather suboptimal... --=-=-=--