bug#71504: 30.0.50; FR: Fix suggestions ("quick fix") for Flymake diagnostics

[Eshel Yaron]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: sbaugh@janestreet.com,  71504@debbugs.gnu.org
>> Date: Thu, 11 Jul 2024 07:43:53 +0200
>> 
>> Hi Eli,
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > It sounds like we all agree, but then what is the problem?
>> 
>> I'm not sure I understand what you're asking, I don't think there's any
>> problem here per se, just a need for a decision on how to proceed:
>> 
>> If you and Spencer agree with my overall implementation, I'll polish it
>> a bit and post an updated patch for master.  If you want to implement
>> the requested feature in some other way, that's perfectly fine too.
>> Lastly, if you don't think Emacs should provide a command for applying
>> fixes to Flymake diagnostics, feel free to close this feature request.
>
> I'm asking what is the overall idea of the proposed implementation.  I
> think it's worthwhile to present it, so we could see if we all agree
> with that idea and the details of the proposed implementation.

Thanks.  To clarify, ideally Spencer will implement this feature request
however he sees fit.  I'm offering my implementation as a reference, but
I'm not advocating for it over other alternatives that may come up.

The idea of my implementation is to allow Flymake backends to associate
fixes with some of the diagnostics they create, and to add a command
that tries to apply a fix for the diagnostic at point.  For the details,
see below the same patch I attached to this message:
https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg01318.html

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 2e602658ea7..cf5349765db 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -368,7 +368,7 @@ flymake-error
   locus beg end type text backend data overlay-properties overlay
   ;; FIXME: See usage of these two in `flymake--highlight-line'.
   ;; Ideally they wouldn't be needed.
-  orig-beg orig-end)
+  orig-beg orig-end fix-function)
 
 ;;;###autoload
 (defun flymake-make-diagnostic (locus
@@ -377,7 +377,8 @@ flymake-make-diagnostic
                                 type
                                 text
                                 &optional data
-                                overlay-properties)
+                                overlay-properties
+                                fix-function)
   "Make a Flymake diagnostic for LOCUS's region from BEG to END.
 LOCUS is a buffer object or a string designating a file name.
 
@@ -396,14 +397,24 @@ flymake-make-diagnostic
 OVERLAY-PROPERTIES is an alist of properties attached to the
 created diagnostic, overriding the default properties and any
 properties listed in the `flymake-overlay-control' property of
-the diagnostic's type symbol."
+the diagnostic's type symbol.
+
+FIX-FUNCTION, if non-nil, is a function that takes DATA and returns a
+list of fix suggestions for this diagnostic.  Each fix suggestion is a
+list (TITLE EDITS), where TITLE is a string describing the fix and EDITS
+is a list of (FILE-OR-BUFFER . CHANGES) cons cells, where FILE-OR-BUFFER
+is the file name or buffer to edit, and CHANGES is a list of changes to
+perform in FILE-OR-BUFFER.  Each element of CHANGES is in turn a
+list (BEG END STR), where BEG and END are buffer positions to delete and
+STR is the string to insert at BEG afterwards."
   (when (stringp locus)
     (setq locus (expand-file-name locus)))
   (flymake--diag-make :locus locus :beg beg :end end
                       :type type :text text :data data
                       :overlay-properties overlay-properties
                       :orig-beg beg
-                      :orig-end end))
+                      :orig-end end
+                      :fix-function fix-function))
 
 ;;;###autoload
 (defun flymake-diagnostics (&optional beg end)
@@ -849,6 +860,42 @@ flymake--update-eol-overlays
             (overlay-put o 'before-string (flymake--eol-overlay-summary 
src-ovs))
           (delete-overlay o))))))
 
+(defun flymake-diagnostic-context-menu (menu click)
+  "Extend MENU with fix suggestions for diagnostic at CLICK."
+  (when-let ((diag (seq-find #'flymake--diag-fix-function
+                             (flymake-diagnostics
+                              (posn-point (event-start click)))))
+             (fixes (funcall (flymake--diag-fix-function diag)
+                             (flymake--diag-data diag)))
+             (i 1))
+    (dolist (fix fixes)
+      (define-key menu (vector (intern (format "flymake-fix-%d" i)))
+                  `(menu-item ,(format "Fix: %s" (car fix))
+                              ,(lambda ()
+                                 (interactive)
+                                 (refactor-apply-edits (cadr fix)))
+                              ,@(cddr fix)))
+      (cl-incf i)))
+  menu)
+
+(defun flymake-fix (pos)
+  "Fix Flymake diagnostic at POS."
+  (interactive "d")
+  ;; TODO - fix _all_ diagnostics at point.
+  (if-let ((diags (flymake-diagnostics pos)))
+      (if-let ((diag (seq-find #'flymake--diag-fix-function diags))
+               (fixes (funcall (flymake--diag-fix-function diag)
+                               (flymake--diag-data diag))))
+          (refactor-apply-edits
+           (car (if (cdr fixes)
+                    (alist-get
+                     (completing-read (format-prompt "Fix" (caar fixes))
+                                      fixes nil t nil nil (caar fixes))
+                     fixes nil nil #'string=)
+                  (cdar fixes))))
+        (message "No fix available for this diagnostic"))
+    (user-error "No diagnostic at this position")))
+
 (cl-defun flymake--highlight-line (diagnostic &optional foreign)
   "Attempt to overlay DIAGNOSTIC in current buffer.
 
@@ -956,6 +1003,7 @@ flymake--highlight-line
              (flymake-diagnostics pos)
              "\n"))))
       (default-maybe 'severity (warning-numeric-level :error))
+      (default-maybe 'context-menu-functions 
'(flymake-diagnostic-context-menu))
       ;; Use (PRIMARY . SECONDARY) priority, to avoid clashing with
       ;; `region' face, for example (bug#34022).
       (default-maybe 'priority (cons nil (+ 40 (overlay-get ov 'severity)))))
diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index a348e9ba6fd..fccee14af82 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -3195,6 +3195,21 @@ sh-shellcheck-arguments
 
 (defvar-local sh--shellcheck-process nil)
 
+(defun sh-shellcheck-fix (data)
+  "Format DATA, a cons cell (TITLE . FIX), as a Flymake fix suggestion."
+  `((,(car data)
+     ((,(current-buffer)
+       ,@(mapcar
+          (lambda (rep)
+            (let-alist rep
+              (without-restriction
+                (save-excursion
+                  (goto-char (point-min))
+                  (list (1- (+ (pos-bol .line) .column))
+                        (1- (+ (pos-bol .endLine) .endColumn))
+                        .replacement)))))
+          (alist-get 'replacements (cdr data))))))))
+
 (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
@@ -3236,7 +3251,9 @@ sh-shellcheck-flymake
                                 ("error" :error)
                                 ("warning" :warning)
                                 (_ :note))
-                              (format "SC%s: %s" .code .message)))))
+                              (format "SC%s: %s" .code .message)
+                              (cons .message .fix) nil
+                              (when (consp .fix) #'sh-shellcheck-fix)))))
                         (funcall report-fn))))
                 (kill-buffer (process-buffer proc)))))))
     (unless dialect