Three Flymake backends Was Re: Two issues with the new Flymake

[João Távora]

Lele Gaifax <address@hidden> writes:

> Hi all,
>
> I'm happily using the new Flymake implementation coupled with the Python
> backend (that I'm still hoping it could be merged into Emacs 26, see bug
> #28808).

Hi,

I attach patches for Lele's Python backend, which would close 28808 and
I'm also throwing in backends for Ruby and Perl.

I think this type of Flymake backends was given a green flag by the
maintainers while ago, *for emacs 26*, but I will wait a couple of days
for objections.

A second point that I wish to raise pertains to what to do in master,
going forward.  As I had predicted, these backends look ridiculously
alike, minus very small differences. They all pipe buffer contents into
checker tools, regexp-search the output, and emit flymake diagnostics.

Obviously, this makes maintenance hard if small fixes are made to the
common structure. The tex-chktex backend recently contributed by Mark is
already an example of that in that it is missing a fix that this type of
backend requires (I pushed a fix in the meantime).

So I'm thinking that, for master (_not_ emacs-26) we could use a
declarative flymake-define-simple-backend macro.

The problem with these macros it that its easy to make them either too
flexible or not flexible enough (or both in the common case :-)).
Anyway, here's my idea for that macro preceded by a examples of how it
simplifies the backends that I propose.

   (flymake-define-simple-backend
    ruby-flymake
    ruby-flymake-command
    '("^\\(?:.*.rb\\|-\\):\\([0-9]+\\): \\(.*\\)$" 1 nil nil 2))
    
   (flymake-define-simple-backend
    perl-flymake
    perl-flymake-command
    '("^\\(.+\\) at - line \\([0-9]+\\)" 2 nil nil 1)
    (lambda (msg)
      (if (string-match
           "\\(Scalar value\\|Useless use\\|Unquoted string\\)"
           msg)
          :warning
        :error)))
    
   (flymake-define-simple-backend
    python-flymake
    python-flymake-command
    python-flymake-command-output-pattern
    (lambda (text)
      (assoc-default
       text
       python-flymake-msg-alist
       #'string-match)))

And now the macro.

  (defvar-local flymake--simple-backend-procs
    (make-hash-table))
   
  (defmacro flymake-define-simple-backend (name command pattern &optional 
type-predicate)
    "Define a simple Flymake backend under NAME.
  This backend runs the COMMAND syntax tool, passes the current
  buffer contents to its standard input, and uses PATTERN to
  examine the output and look for diagnostic messages.
   
  PATTERN must evaluate to a list of the form (REGEXP LINE COLUMN
  TYPE MESSAGE): if REGEXP matches, the LINE'th subexpression gives
  the line number, the COLUMN'th subexpression gives the column
  number on that line, the TYPE'th subexpression gives the type of
  the message and the MESSAGE'th gives the message text itself.
   
  If COLUMN or TYPE are nil or that index didn't match, that
  information is not present on the matched line and a default will
  be used."
    (let ((name-once name))
      `(defun ,name-once (report-fn &rest _args)
         "A Flymake backend defined with
  `flymake-define-simple-backend'."
         (let* ((command-once ,command)
                (pattern-once ,pattern)
                (pred-once (function ,type-predicate))
                (process (gethash ',name-once flymake--simple-backend-procs))
                (source (current-buffer)))
           (unless (executable-find (car command-once))
             (error "Cannot find a suitable checker"))
           (when (process-live-p process)
             (kill-process process))
           (save-restriction
             (widen)
             (setq
              process
              (puthash
               ',name-once
               (make-process
                :name (symbol-name ',name-once) :noquery t :connection-type 
'pipe
                :buffer (generate-new-buffer
                         (format " *simple backend %s*" ',name-once))
                :command command-once
                :sentinel
                (lambda (proc _event)
                  (when (eq 'exit (process-status proc))
                    (unwind-protect
                        (if (with-current-buffer source
                              (eq proc
                                  (gethash ',name-once 
flymake--simple-backend-procs)))
                            (with-current-buffer (process-buffer proc)
                              (goto-char (point-min))
                              (cl-loop
                               while (search-forward-regexp
                                      (nth 0 pattern-once)
                                      nil t)
                               for msg = (match-string
                                          (nth 4 pattern-once))
                               for (beg . end) = (flymake-diag-region
                                                  source
                                                  (string-to-number
                                                   (match-string
                                                    (nth 1 pattern-once)))
                                                  (if (nth 2 pattern-once)
                                                      (string-to-number
                                                       (match-string
                                                        (nth 3 pattern-once)))))
                               for match-target = (if (nth 3 pattern-once)
                                                      (match-string
                                                       (nth 3 pattern-once))
                                                    msg)
                               for type = (if pred-once
                                              (funcall pred-once match-target)
                                            (if (string-match "warning" 
match-target)
                                                :warning
                                              :error))
                               collect (flymake-make-diagnostic source
                                                                beg
                                                                end
                                                                type
                                                                msg)
                               into diags
                               finally (funcall report-fn diags)))
                          (flymake-log :debug "Canceling obsolete check %s"
                                       proc))
                      (kill-buffer (process-buffer proc))))))
               flymake--simple-backend-procs))
             (process-send-region process (point-min) (point-max))
             (process-send-eof process))))))

>From fd800a9e16493872ff3c8244a2e30e2d9e61fca4 Mon Sep 17 00:00:00 2001
From: Lele Gaifax <address@hidden>
Date: Fri, 3 Nov 2017 12:20:36 +0000
Subject: [PATCH 1/3] Add a Flymake backend for Python

Implement new Flymake backend with related customizable settings.

* lisp/progmodes/python.el (python-flymake-command)
(python-flymake-command-output-pattern)
(python-flymake-msg-alist): New defcustom.
(python--flymake-parse-output): New function, able to parse
python-flymake-command output accordingly to
python-flymake-command-output-pattern.
(python-flymake): New function implementing the backend
interface using python--flymake-parse-output for the real
work.
(python-mode): Add python-flymake to flymake-diagnostic-functions.
---
 lisp/progmodes/python.el | 136 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 135 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 895117b9ee..b7902fb978 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -5142,6 +5142,138 @@ python-util-valid-regexp-p
   (ignore-errors (string-match regexp "") t))
 
 
+;;; Flymake integration
+
+(defgroup python-flymake nil
+  "Integration between Python and Flymake."
+  :group 'python
+  :link '(custom-group-link :tag "Flymake" flymake)
+  :version "26.1")
+
+(defcustom python-flymake-command '("pyflakes")
+  "The external tool that will be used to perform the syntax check.
+This is a non empty list of strings, the checker tool possibly followed by
+required arguments.  Once launched it will receive the Python source to be
+checked as its standard input.
+To use `flake8' you would set this to (\"flake8\" \"-\")."
+  :group 'python-flymake
+  :type '(repeat string))
+
+;; The default regexp accomodates for older pyflakes, which did not
+;; report the column number, and at the same time it's compatible with
+;; flake8 output, although it may be redefined to explicitly match the
+;; TYPE
+(defcustom python-flymake-command-output-pattern
+  (list
+   "^\\(?:<?stdin>?\\):\\(?1:[0-9]+\\):\\(?:\\(?2:[0-9]+\\):\\)? \\(?3:.*\\)$"
+   1 2 nil 3)
+  "Specify how to parse the output of `python-flymake-command'.
+The value has the form (REGEXP LINE COLUMN TYPE MESSAGE): if
+REGEXP matches, the LINE'th subexpression gives the line number,
+the COLUMN'th subexpression gives the column number on that line,
+the TYPE'th subexpression gives the type of the message and the
+MESSAGE'th gives the message text itself.
+
+If COLUMN or TYPE are nil or that index didn't match, that
+information is not present on the matched line and a default will
+be used."
+  :group 'python-flymake
+  :type '(list regexp
+               (integer :tag "Line's index")
+               (choice
+                (const :tag "No column" nil)
+                (integer :tag "Column's index"))
+               (choice
+                (const :tag "No type" nil)
+                (integer :tag "Type's index"))
+               (integer :tag "Message's index")))
+
+(defcustom python-flymake-msg-alist
+  '(("\\(^redefinition\\|.*unused.*\\|used$\\)" . :warning))
+  "Alist used to associate messages to their types.
+Each element should be a cons-cell (REGEXP . TYPE), where TYPE must be
+one defined in the variable `flymake-diagnostic-types-alist'.
+For example, when using `flake8' a possible configuration could be:
+
+  ((\"\\(^redefinition\\|.*unused.*\\|used$\\)\" . :warning)
+   (\"^E999\" . :error)
+   (\"^[EW][0-9]+\" . :note))
+
+By default messages are considered errors."
+  :group 'python-flymake
+  :type `(alist :key-type (regexp)
+                :value-type (symbol)))
+
+(defvar-local python--flymake-proc nil)
+
+(defun python--flymake-parse-output (source proc report-fn)
+  "Collect diagnostics parsing checker tool's output line by line."
+  (let ((rx (nth 0 python-flymake-command-output-pattern))
+        (lineidx (nth 1 python-flymake-command-output-pattern))
+        (colidx (nth 2 python-flymake-command-output-pattern))
+        (typeidx (nth 3 python-flymake-command-output-pattern))
+        (msgidx (nth 4 python-flymake-command-output-pattern)))
+    (with-current-buffer (process-buffer proc)
+      (goto-char (point-min))
+      (cl-loop
+       while (search-forward-regexp rx nil t)
+       for msg = (match-string msgidx)
+       for (beg . end) = (flymake-diag-region
+                          source
+                          (string-to-number
+                           (match-string lineidx))
+                          (and colidx
+                               (match-string colidx)
+                               (string-to-number
+                                (match-string colidx))))
+       for type = (or (and typeidx
+                           (match-string typeidx)
+                           (assoc-default
+                            (match-string typeidx)
+                            python-flymake-msg-alist
+                            #'string-match))
+                      (assoc-default msg
+                                     python-flymake-msg-alist
+                                     #'string-match)
+                      :error)
+       collect (flymake-make-diagnostic
+                source beg end type msg)
+       into diags
+       finally (funcall report-fn diags)))))
+
+(defun python-flymake (report-fn &rest _args)
+  "Flymake backend for Python.
+This backend uses `python-flymake-command' (which see) to launch a process
+that is passed the current buffer's content via stdin.
+REPORT-FN is Flymake's callback function."
+  (unless (executable-find (car python-flymake-command))
+    (error "Cannot find a suitable checker"))
+
+  (when (process-live-p python--flymake-proc)
+    (kill-process python--flymake-proc))
+
+  (let ((source (current-buffer)))
+    (save-restriction
+      (widen)
+      (setq python--flymake-proc
+            (make-process
+             :name "python-flymake"
+             :noquery t
+             :connection-type 'pipe
+             :buffer (generate-new-buffer " *python-flymake*")
+             :command python-flymake-command
+             :sentinel
+             (lambda (proc _event)
+               (when (eq 'exit (process-status proc))
+                 (unwind-protect
+                     (when (with-current-buffer source
+                             (eq proc python--flymake-proc))
+                       (python--flymake-parse-output source proc report-fn))
+                   (kill-buffer (process-buffer proc)))))))
+      (process-send-region python--flymake-proc (point-min) (point-max))
+      (process-send-eof python--flymake-proc))))
+
+
 (defun python-electric-pair-string-delimiter ()
   (when (and electric-pair-mode
              (memq last-command-event '(?\" ?\'))
@@ -5255,7 +5387,9 @@ python-mode
   (make-local-variable 'python-shell-internal-buffer)
 
   (when python-indent-guess-indent-offset
-    (python-indent-guess-indent-offset)))
+    (python-indent-guess-indent-offset))
+
+  (add-hook 'flymake-diagnostic-functions #'python-flymake nil t))
 
 
 (provide 'python)
-- 
2.14.2

>From 88440e31f48cd2fb44fef4d6b4ecaaf017246002 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <address@hidden>
Date: Fri, 3 Nov 2017 16:05:39 +0000
Subject: [PATCH 2/3] Add a Flymake backend for Perl

Define a simple backend in perl-mode.el, which cperl-mode.el also
uses.

* lisp/progmodes/cperl-mode.el (cperl-mode): Add to
flymake-diagnostic-functions.

* lisp/progmodes/flymake-proc.el
(flymake-proc-allowed-file-name-masks): Disable legacy backend
for perl files.

* lisp/progmodes/perl-mode.el (perl-flymake-command): New
defcustom.
(perl--flymake-proc): New buffer-local variable.
(perl-flymake): New function.
(perl-mode): Add to flymake-diagnostic-functions.
---
 lisp/progmodes/cperl-mode.el   |  4 ++-
 lisp/progmodes/flymake-proc.el |  2 +-
 lisp/progmodes/perl-mode.el    | 71 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 853604d1d7..4c63ec2fb4 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -1896,7 +1896,9 @@ cperl-mode
   (if cperl-pod-here-scan
       (or cperl-syntaxify-by-font-lock
        (progn (or cperl-faces-init (cperl-init-faces-weak))
-             (cperl-find-pods-heres)))))
+             (cperl-find-pods-heres))))
+  ;; Setup Flymake
+  (add-hook 'flymake-diagnostic-functions 'perl-flymake nil t))
 
 ;; Fix for perldb - make default reasonable
 (defun cperl-db ()
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 5c4d451d63..2e98b2afd1 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -73,7 +73,7 @@ flymake-proc-allowed-file-name-masks
     ("\\.xml\\'" flymake-proc-xml-init)
     ("\\.html?\\'" flymake-proc-xml-init)
     ("\\.cs\\'" flymake-proc-simple-make-init)
-    ("\\.p[ml]\\'" flymake-proc-perl-init)
+    ;; ("\\.p[ml]\\'" flymake-proc-perl-init)
     ("\\.php[345]?\\'" flymake-proc-php-init)
     ("\\.h\\'" flymake-proc-master-make-header-init 
flymake-proc-master-cleanup)
     ("\\.java\\'" flymake-proc-simple-make-java-init 
flymake-proc-simple-java-cleanup)
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index 24b934ce6c..0f1088920a 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -581,6 +581,73 @@ perl-current-defun-name
        (match-string-no-properties 1))))
 
 
+;;; Flymake support
+(defcustom perl-flymake-command '("perl" "-w" "-c")
+  "External tool used to check Perl source code.
+This is a non empty list of strings, the checker tool possibly
+followed by required arguments.  Once launched it will receive
+the Perl source to be checked as its standard input."
+  :group 'perl
+  :type '(repeat string))
+
+(defvar-local perl--flymake-proc nil)
+
+;;;###autoload
+(defun perl-flymake (report-fn &rest _args)
+  "Perl backend for Flymake.  Launches
+`perl-flymake-command' (which see) and passes to its standard
+input the contents of the current buffer.  The output of this
+command is analysed for error and warning messages."
+  (unless (executable-find (car perl-flymake-command))
+    (error "Cannot find a suitable checker"))
+
+  (when (process-live-p perl--flymake-proc)
+    (kill-process perl--flymake-proc))
+
+  (let ((source (current-buffer)))
+    (save-restriction
+      (widen)
+      (setq
+       perl--flymake-proc
+       (make-process
+        :name "perl-flymake" :noquery t :connection-type 'pipe
+        :buffer (generate-new-buffer " *perl-flymake*")
+        :command perl-flymake-command
+        :sentinel
+        (lambda (proc _event)
+          (when (eq 'exit (process-status proc))
+            (unwind-protect
+                (if (with-current-buffer source (eq proc perl--flymake-proc))
+                    (with-current-buffer (process-buffer proc)
+                      (goto-char (point-min))
+                      (cl-loop
+                       while (search-forward-regexp
+                              "^\\(.+\\) at - line \\([0-9]+\\)"
+                              nil t)
+                       for msg = (match-string 1)
+                       for (beg . end) = (flymake-diag-region
+                                          source
+                                          (string-to-number (match-string 2)))
+                       for type =
+                       (if (string-match
+                            "\\(Scalar value\\|Useless use\\|Unquoted 
string\\)"
+                            msg)
+                           :warning
+                         :error)
+                       collect (flymake-make-diagnostic source
+                                                        beg
+                                                        end
+                                                        type
+                                                        msg)
+                       into diags
+                       finally (funcall report-fn diags)))
+                  (flymake-log :debug "Canceling obsolete check %s"
+                               proc))
+              (display-buffer (process-buffer proc)))))))
+      (process-send-region perl--flymake-proc (point-min) (point-max))
+      (process-send-eof perl--flymake-proc))))
+
+
 (defvar perl-mode-hook nil
   "Normal hook to run when entering Perl mode.")
 
@@ -665,7 +732,9 @@ perl-mode
   ;; Setup outline-minor-mode.
   (setq-local outline-regexp perl-outline-regexp)
   (setq-local outline-level 'perl-outline-level)
-  (setq-local add-log-current-defun-function #'perl-current-defun-name))
+  (setq-local add-log-current-defun-function #'perl-current-defun-name)
+  ;; Setup Flymake
+  (add-hook 'flymake-diagnostic-functions 'perl-flymake nil t))
 
 ;; This is used by indent-for-comment
 ;; to decide how much to indent a comment in Perl code
-- 
2.14.2

>From 88440e31f48cd2fb44fef4d6b4ecaaf017246002 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <address@hidden>
Date: Fri, 3 Nov 2017 16:05:39 +0000
Subject: [PATCH 3/3] Add a Flymake backend for Perl

Define a simple backend in perl-mode.el, which cperl-mode.el also
uses.

* lisp/progmodes/cperl-mode.el (cperl-mode): Add to
flymake-diagnostic-functions.

* lisp/progmodes/flymake-proc.el
(flymake-proc-allowed-file-name-masks): Disable legacy backend
for perl files.

* lisp/progmodes/perl-mode.el (perl-flymake-command): New
defcustom.
(perl--flymake-proc): New buffer-local variable.
(perl-flymake): New function.
(perl-mode): Add to flymake-diagnostic-functions.
---
 lisp/progmodes/cperl-mode.el   |  4 ++-
 lisp/progmodes/flymake-proc.el |  2 +-
 lisp/progmodes/perl-mode.el    | 71 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/cperl-mode.el b/lisp/progmodes/cperl-mode.el
index 853604d1d7..4c63ec2fb4 100644
--- a/lisp/progmodes/cperl-mode.el
+++ b/lisp/progmodes/cperl-mode.el
@@ -1896,7 +1896,9 @@ cperl-mode
   (if cperl-pod-here-scan
       (or cperl-syntaxify-by-font-lock
        (progn (or cperl-faces-init (cperl-init-faces-weak))
-             (cperl-find-pods-heres)))))
+             (cperl-find-pods-heres))))
+  ;; Setup Flymake
+  (add-hook 'flymake-diagnostic-functions 'perl-flymake nil t))
 
 ;; Fix for perldb - make default reasonable
 (defun cperl-db ()
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 5c4d451d63..2e98b2afd1 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -73,7 +73,7 @@ flymake-proc-allowed-file-name-masks
     ("\\.xml\\'" flymake-proc-xml-init)
     ("\\.html?\\'" flymake-proc-xml-init)
     ("\\.cs\\'" flymake-proc-simple-make-init)
-    ("\\.p[ml]\\'" flymake-proc-perl-init)
+    ;; ("\\.p[ml]\\'" flymake-proc-perl-init)
     ("\\.php[345]?\\'" flymake-proc-php-init)
     ("\\.h\\'" flymake-proc-master-make-header-init 
flymake-proc-master-cleanup)
     ("\\.java\\'" flymake-proc-simple-make-java-init 
flymake-proc-simple-java-cleanup)
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index 24b934ce6c..0f1088920a 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -581,6 +581,73 @@ perl-current-defun-name
        (match-string-no-properties 1))))
 
 
+;;; Flymake support
+(defcustom perl-flymake-command '("perl" "-w" "-c")
+  "External tool used to check Perl source code.
+This is a non empty list of strings, the checker tool possibly
+followed by required arguments.  Once launched it will receive
+the Perl source to be checked as its standard input."
+  :group 'perl
+  :type '(repeat string))
+
+(defvar-local perl--flymake-proc nil)
+
+;;;###autoload
+(defun perl-flymake (report-fn &rest _args)
+  "Perl backend for Flymake.  Launches
+`perl-flymake-command' (which see) and passes to its standard
+input the contents of the current buffer.  The output of this
+command is analysed for error and warning messages."
+  (unless (executable-find (car perl-flymake-command))
+    (error "Cannot find a suitable checker"))
+
+  (when (process-live-p perl--flymake-proc)
+    (kill-process perl--flymake-proc))
+
+  (let ((source (current-buffer)))
+    (save-restriction
+      (widen)
+      (setq
+       perl--flymake-proc
+       (make-process
+        :name "perl-flymake" :noquery t :connection-type 'pipe
+        :buffer (generate-new-buffer " *perl-flymake*")
+        :command perl-flymake-command
+        :sentinel
+        (lambda (proc _event)
+          (when (eq 'exit (process-status proc))
+            (unwind-protect
+                (if (with-current-buffer source (eq proc perl--flymake-proc))
+                    (with-current-buffer (process-buffer proc)
+                      (goto-char (point-min))
+                      (cl-loop
+                       while (search-forward-regexp
+                              "^\\(.+\\) at - line \\([0-9]+\\)"
+                              nil t)
+                       for msg = (match-string 1)
+                       for (beg . end) = (flymake-diag-region
+                                          source
+                                          (string-to-number (match-string 2)))
+                       for type =
+                       (if (string-match
+                            "\\(Scalar value\\|Useless use\\|Unquoted 
string\\)"
+                            msg)
+                           :warning
+                         :error)
+                       collect (flymake-make-diagnostic source
+                                                        beg
+                                                        end
+                                                        type
+                                                        msg)
+                       into diags
+                       finally (funcall report-fn diags)))
+                  (flymake-log :debug "Canceling obsolete check %s"
+                               proc))
+              (display-buffer (process-buffer proc)))))))
+      (process-send-region perl--flymake-proc (point-min) (point-max))
+      (process-send-eof perl--flymake-proc))))
+
+
 (defvar perl-mode-hook nil
   "Normal hook to run when entering Perl mode.")
 
@@ -665,7 +732,9 @@ perl-mode
   ;; Setup outline-minor-mode.
   (setq-local outline-regexp perl-outline-regexp)
   (setq-local outline-level 'perl-outline-level)
-  (setq-local add-log-current-defun-function #'perl-current-defun-name))
+  (setq-local add-log-current-defun-function #'perl-current-defun-name)
+  ;; Setup Flymake
+  (add-hook 'flymake-diagnostic-functions 'perl-flymake nil t))
 
 ;; This is used by indent-for-comment
 ;; to decide how much to indent a comment in Perl code
-- 
2.14.2