]> git.eshelyaron.com Git - emacs.git/commitdiff
Add Flymake fix suggestions for Checkdoc diagnostics
authorEshel Yaron <me@eshelyaron.com>
Mon, 3 Jun 2024 08:55:26 +0000 (10:55 +0200)
committerEshel Yaron <me@eshelyaron.com>
Mon, 3 Jun 2024 08:55:26 +0000 (10:55 +0200)
lisp/emacs-lisp/checkdoc.el
lisp/progmodes/elisp-mode.el

index c22dfb2eb26d1dbbb52ef5ff09ca959928191554..0cd0139045e9cd8c30128a6e4c1b20470b053d5b 100644 (file)
@@ -667,8 +667,8 @@ style."
                (recenter))
              (message "%s (C-h,%se,n,p,q)" (checkdoc-error-text
                                              (car (car err-list)))
-                      (if (checkdoc-error-unfixable (car (car err-list)))
-                          "" "f,"))
+                      (if (checkdoc-error-fix (car (car err-list)))
+                          "f," ""))
              (save-excursion
                (goto-char (checkdoc-error-start (car (car err-list))))
                (if (not (pos-visible-in-window-p))
@@ -759,12 +759,12 @@ style."
                     (with-current-buffer standard-output
                       (insert
                        "Checkdoc Keyboard Summary:\n"
-                       (if (checkdoc-error-unfixable (car (car err-list)))
-                           ""
-                         (concat
-                          "f, y    - auto Fix this warning without asking"
-                          " (if available.)\n"
-                          "         Very complex operations will still query.\n"))
+                       (if (checkdoc-error-fix (car (car err-list)))
+                           (concat
+                            "f, y    - auto Fix this warning without asking"
+                            " (if available.)\n"
+                            "         Very complex operations will still query.\n")
+                         "")
                        "e      - Enter recursive Edit.  Press C-M-c to exit.\n"
                        "SPC, n - skip to the Next error.\n"
                        "DEL, p - skip to the Previous error.\n"
@@ -1250,37 +1250,36 @@ Prefix argument is the same as for `checkdoc-defun'."
 
 (cl-defstruct (checkdoc-error
                (:constructor nil)
-               (:constructor checkdoc--create-error (text start end &optional unfixable)))
+               (:constructor checkdoc--create-error (text start end &optional fix)))
   (text nil :read-only t)
   (start nil :read-only t)
   (end nil :read-only t)
-  (unfixable nil :read-only t))
+  (fix nil :read-only t))
 
 (defvar checkdoc-create-error-function #'checkdoc--create-error-for-checkdoc
   "Function called when Checkdoc encounters an error.
-Should accept as arguments (TEXT START END &optional UNFIXABLE).
+Should accept as arguments (TEXT START END &optional FIX).
 
 TEXT is the descriptive text of the error.  START and END define the region
-it is sensible to highlight when describing the problem.
-Optional argument UNFIXABLE means that the error has no auto-fix available.
+it is sensible to highlight when describing the problem.  FIX is...
 
 An object of type `checkdoc-error' is returned if we are not
 generating a buffered list of errors.")
 
-(defun checkdoc-create-error (text start end &optional unfixable)
+(defun checkdoc-create-error (text start end &optional fix)
   "Used to create the return error text returned from all engines.
-TEXT, START, END and UNFIXABLE conform to
-`checkdoc-create-error-function', which see."
-  (funcall checkdoc-create-error-function text start end unfixable))
+TEXT, START, END and FIX conform to `checkdoc-create-error-function',
+which see."
+  (funcall checkdoc-create-error-function text start end fix))
 
-(defun checkdoc--create-error-for-checkdoc (text start end &optional unfixable)
+(defun checkdoc--create-error-for-checkdoc (text start end &optional fix)
   "Create an error for Checkdoc.
-TEXT, START, END and UNFIXABLE conform to
-`checkdoc-create-error-function', which see."
+TEXT, START, END and FIX conform to `checkdoc-create-error-function',
+which see."
   (if checkdoc-generate-compile-warnings-flag
       (progn (checkdoc-error start text)
             nil)
-    (checkdoc--create-error text start end unfixable)))
+    (checkdoc--create-error text start end fix)))
 
 ;;; Minor Mode specification
 ;;
@@ -1392,10 +1391,10 @@ buffer, otherwise stop after the first error."
      ;;   have a documentation string.  In earlier Emacs versions, you could
      ;;   save space by using a comment instead of a documentation string,
      ;;   but that is no longer the case.
-     (if (and (not (nth 1 fp))         ; not a variable
-             (or (nth 2 fp)            ; is interactive
+     (if (and (not (nth 1 fp))                    ; not a variable
+             (or (nth 2 fp)                      ; is interactive
                  checkdoc-force-docstrings-flag) ;or we always complain
-             (not (eq (following-char) ?\"))) ; no doc string
+             (not (eq (following-char) ?\")))    ; no doc string
         ;; Sometimes old code has comments where the documentation should
         ;; be.  Let's see if we can find the comment, and offer to turn it
         ;; into documentation for them.
@@ -1448,7 +1447,10 @@ buffer, otherwise stop after the first error."
                     "All interactive functions should have documentation"
                   "All variables and subroutines might as well have a \
 documentation string")
-                (point) (+ (point) 1) t))))))
+                (point) (+ (point) 1)
+                `("Add docstring"
+                  ((,(current-buffer)
+                    (,(point) ,(point) "\"\"\n  "))))))))))
     (if (and (not err) (= (following-char) ?\"))
         (with-syntax-table checkdoc-syntax-table
           (checkdoc-this-string-valid-engine fp take-notes))
@@ -1488,19 +1490,25 @@ buffer, otherwise stop after the first error."
             (checkdoc-create-error
              "Second line should not have indentation"
              (match-beginning 1)
-             (match-end 1)))))
+             (match-end 1)
+              `("Remove indentation"
+                ((,(current-buffer)
+                  (,(match-beginning 1) ,(match-end 1) ""))))))))
      ;; * Check for '(' in column 0.
      (when checkdoc-column-zero-backslash-before-paren
        (save-excursion
          (when (re-search-forward "^(" e t)
            (if (checkdoc-autofix-ask-replace (match-beginning 0)
-                                     (match-end 0)
-                                     (format-message "Escape this `('?")
-                                     "\\(")
+                                             (match-end 0)
+                                             (format-message "Escape this `('?")
+                                             "\\(")
                nil
              (checkdoc-create-error
               "Open parenthesis in column 0 should be escaped"
-              (match-beginning 0) (match-end 0))))))
+              (match-beginning 0) (match-end 0)
+              `("Escape parenthesis in column 0"
+                ((,(current-buffer)
+                  (,(match-beginning 0) ,(match-beginning 0) "\\")))))))))
      ;; * Do not start or end a documentation string with whitespace.
      (let (start end)
        (if (or (if (looking-at "\"\\([ \t\n]+\\)")
@@ -1517,7 +1525,8 @@ buffer, otherwise stop after the first error."
               nil
             (checkdoc-create-error
              "Documentation strings should not start or end with whitespace"
-             start end))))
+             start end
+              `("Remove whitespace" ((,(current-buffer) (,start ,end ""))))))))
      ;; * The first line of the documentation string should consist of one
      ;;   or two complete sentences that stand on their own as a summary.
      ;;   `M-x apropos' displays just the first line, and if it doesn't
@@ -1543,7 +1552,8 @@ buffer, otherwise stop after the first error."
               nil
             (checkdoc-create-error
              "First sentence should end with punctuation"
-             (point) (1+ (point))))))
+             (point) (1+ (point))
+              `("Add fullstop" ((,(current-buffer) (,(point) ,(point) "."))))))))
        ((looking-at "[\\!?;:.)]")
         ;; These are ok
         nil)
@@ -1604,7 +1614,11 @@ may require more formatting")
               nil
             (checkdoc-create-error
              "First line should be capitalized"
-             (match-beginning 0) (match-end 0)))
+             (match-beginning 0) (match-end 0)
+              `("Capitalize"
+                ((,(current-buffer)
+                  (,(match-beginning 0) ,(match-end 0)
+                   ,(upcase (match-string 0))))))))
         nil))
      ;;   * Don't write key sequences directly in documentation strings.
      ;;     Instead, use the `\\[...]' construct to stand for them.
@@ -1626,7 +1640,7 @@ may require more formatting")
               "Keycode " (match-string 1)
               " embedded in doc string.  Use \\\\<mapvar> & \\\\[command] "
               "instead")
-             (match-beginning 1) (match-end 1) t))))
+             (match-beginning 1) (match-end 1)))))
      ;; Optionally warn about too many command substitutions.
      (when checkdoc-max-keyref-before-warn
        (save-excursion
@@ -1679,6 +1693,7 @@ may require more formatting")
                         (format "Disambiguate %s by preceding w/ \
 function,command,variable,option or symbol." ms1))))))
         (if ret
+             ;; TODO - add fix.
             (checkdoc-create-error ret mb me)
           nil)))
      ;; * Format the documentation string so that it fits in an
@@ -1733,14 +1748,17 @@ function,command,variable,option or symbol." ms1))))))
                    nil
                  (checkdoc-create-error
                   "\"True\" should usually be \"Non-nil\""
-                  (match-beginning 1) (match-end 1))))
+                  (match-beginning 1) (match-end 1)
+                  `("Say \"Non-nil\" instead of \"True\""
+                    ((,(current-buffer)
+                      (,(match-beginning 1) ,(match-end 1) "Non-nil")))))))
 
             ;; If the variable has -flag in the name, make sure
             (if (and (string-match "-flag$" (car fp))
                      (not (looking-at "\"\\*?Non-nil\\s-+means\\s-+")))
                 (checkdoc-create-error
                  "Flag variable doc strings should usually start: Non-nil means"
-                 s (marker-position e) t))
+                 s (marker-position e)))
              ;; Don't rename variable to "foo-flag".  This is unnecessary
              ;; and such names often end up inconvenient when the variable
              ;; is later expanded to non-boolean values. --Stef
@@ -1851,7 +1869,7 @@ function,command,variable,option or symbol." ms1))))))
                     (if (< found last-pos)
                         (checkdoc-create-error
                          "Arguments occur in the doc string out of order"
-                         s (marker-position e) t)))))
+                         s (marker-position e))))))
             ;; * For consistency, phrase the verb in the first sentence of a
             ;;   documentation string for functions as an imperative.
             ;;   For instance, use `Return the cons of A and
@@ -1898,7 +1916,11 @@ Replace with \"%s\"?" original replace)
                           (format
                            "Probably \"%s\" should be imperative \"%s\""
                            original replace)
-                          (match-beginning 1) (match-end 1))))))
+                          (match-beginning 1) (match-end 1)
+                           `("Use imperative verb"
+                             ((,(current-buffer)
+                               (,(match-beginning 1) ,(match-end 1)
+                                ,replace)))))))))
             ;; "Return true ..." should be "Return non-nil ..."
             (when (looking-at "\"Return \\(true\\)\\b")
                (if (checkdoc-autofix-ask-replace
@@ -1908,7 +1930,10 @@ Replace with \"%s\"?" original replace)
                    nil
                  (checkdoc-create-error
                   "\"true\" should usually be \"non-nil\""
-                  (match-beginning 1) (match-end 1))))
+                  (match-beginning 1) (match-end 1)
+                  `("Say \"non-nil\" instead of \"true\""
+                    ((,(current-buffer)
+                      (,(match-beginning 1) ,(match-end 1) "non-nil")))))))
             ;; Done with functions
             )))
      ;;* When a documentation string refers to a Lisp symbol, write it as
@@ -1949,7 +1974,12 @@ Replace with \"%s\"?" original replace)
         (if msg
             (checkdoc-create-error msg (match-beginning 1)
                                    (+ (match-beginning 1)
-                                      (length ms)))
+                                      (length ms))
+                                    `("Add qoutes around Lisp symbol"
+                                      ((,(current-buffer)
+                                        (,(match-beginning 1)
+                                         ,(+ (match-beginning 1) (length ms))
+                                         ,(format "`%s'" ms))))))
           nil)))
      ;; t and nil case
      (save-excursion
@@ -1962,7 +1992,11 @@ Replace with \"%s\"?" original replace)
               nil
             (checkdoc-create-error
              "Symbols t and nil should not appear in single quotes"
-             (match-beginning 1) (match-end 1)))))
+             (match-beginning 1) (match-end 1)
+              `("Remove quotes"
+                ((,(current-buffer)
+                  (,(match-beginning 1) ,(match-end 1)
+                   ,(match-string 2)))))))))
      ;; Here is some basic sentence formatting
      (checkdoc-sentencespace-region-engine (point) e)
      ;; Here are common proper nouns that should always appear capitalized.
@@ -2148,7 +2182,7 @@ internally skip over no answers.
 If the offending word is in a piece of quoted text, then it is skipped."
   (save-excursion
     (let ((case-fold-search nil)
-         (errtxt nil) bb be)
+         (errtxt nil) bb be tx)
       (with-syntax-table checkdoc-syntax-table
         (goto-char begin)
         (while (re-search-forward checkdoc-proper-noun-regexp end t)
@@ -2186,8 +2220,11 @@ If the offending word is in a piece of quoted text, then it is skipped."
                           (format
                            "Name %s should appear capitalized as %s"
                            text (capitalize text))
-                          bb b be e)))))))
-      (if errtxt (checkdoc-create-error errtxt bb be)))))
+                          bb b be e tx text)))))))
+      (if errtxt (checkdoc-create-error errtxt bb be
+                                        `("Capitalize"
+                                          ((,(current-buffer)
+                                            (,bb ,be ,(capitalize tx))))))))))
 
 (defun checkdoc-sentencespace-region-engine (begin end)
   "Make sure all sentences have double spaces between BEGIN and END."
@@ -2218,7 +2255,10 @@ If the offending word is in a piece of quoted text, then it is skipped."
                       (setq errtxt
                             "There should be two spaces after a period"
                             bb b be e)))))))
-         (if errtxt (checkdoc-create-error errtxt bb be))))))
+         (if errtxt (checkdoc-create-error errtxt bb be
+                                            `("Add missing space"
+                                              ((,(current-buffer)
+                                                (,be ,be " "))))))))))
 
 ;;; Ispell engine
 ;;
@@ -2306,8 +2346,7 @@ If `checkdoc-autofix-flag' permits, delete that whitespace instead.
 If optional arguments START and END are non-nil, bound the check to
 this region.
 Optional argument INTERACT may permit the user to fix problems on the fly."
-  (let ((p (point))
-       (msg nil) s e (f nil))
+  (let ((p (point)) (msg nil) s e)
     (if (not start) (setq start (point-min)))
     ;; If end is nil, it means end of buffer to search anyway
     (or
@@ -2319,7 +2358,7 @@ Optional argument INTERACT may permit the user to fix problems on the fly."
         (setq msg
               "Don't use `? ' at the end of a line. \
 News agents may remove it"
-              s (match-beginning 0) e (match-end 0) f t)
+              s (match-beginning 0) e (match-end 0))
         ;; If interactive is passed down, give them a chance to fix things.
          (if (and interact (y-or-n-p (concat msg ". Fix?")))
             (progn
@@ -2344,7 +2383,8 @@ News agents may remove it"
     ;; Return an error and leave the cursor at that spot, or restore
     ;; the cursor.
     (if msg
-       (checkdoc-create-error msg s e f)
+        ;; TODO - add fix.
+       (checkdoc-create-error msg s e)
       (goto-char p)
       nil)))
 
@@ -2406,13 +2446,13 @@ Code:, and others referenced in the style guide."
               ((not (re-search-forward ";;; .* --- .*\n" nil t))
                 (checkdoc-create-error
                  "You should have a summary line (\";;; .* --- .*\")"
-                 nil nil t)))
+                 nil nil)))
              (if (checkdoc-y-or-n-p
                    "You should have a \";;; Commentary:\", add one?")
                   (insert checkdoc-commentary-header-string)
                (checkdoc-create-error
                 "You should have a section marked \";;; Commentary:\""
-                nil nil t)))
+                nil nil)))
          nil)
        err))
       (setq
@@ -2620,7 +2660,10 @@ a space as a style error."
         nil
       (checkdoc-create-error
        "`y-or-n-p' argument should end with \"?\""
-       (match-beginning 0) (match-end 0)))))
+       (match-beginning 0) (match-end 0)
+       `("Add question mark"
+         ((,(current-buffer)
+           (,(match-beginning 0) ,(match-beginning 0) "?"))))))))
 
 (defun checkdoc-message-text-engine (&optional type)
   "Return or fix errors found in strings passed to a message display function.
@@ -2651,7 +2694,11 @@ Argument TYPE specifies the type of question, such as `error' or `y-or-n-p'."
                     (capitalize (match-string 1))
                    t)))
          (checkdoc-create-error "Messages should start with a capital letter"
-          (match-beginning 1) (match-end 1))
+                                (match-beginning 1) (match-end 1)
+                                `("Capitalize"
+                                  ((,(current-buffer)
+                                    (,(match-beginning 1) ,(match-end 1)
+                                     ,(capitalize (match-string 1)))))))
        nil)
      ;; In general, sentences should have two spaces after the period.
      (checkdoc-sentencespace-region-engine (point)
@@ -2673,7 +2720,10 @@ Argument TYPE specifies the type of question, such as `error' or `y-or-n-p'."
                                                 t)))
         (checkdoc-create-error
          "Error messages should *not* end with a period"
-         (match-beginning 0) (match-end 0))
+         (match-beginning 0) (match-end 0)
+          `("Remove fullstop"
+            ((,(current-buffer)
+              (,(match-beginning 0) ,(match-end 0) "")))))
        nil)
      ;; From `(elisp) Programming Tips': "A question asked in the
      ;; minibuffer with `yes-or-no-p' or `y-or-n-p' should start with
index 9e727bd4543d763bf475747b62a33233c9062749..a8bb45a4dce832500dfa555f9cc0f690cfda2c3e 100644 (file)
@@ -2102,8 +2102,8 @@ ARGLIST is either a string, or a list of strings or symbols."
 Calls REPORT-FN directly."
   (let (collected)
     (let* ((checkdoc-create-error-function
-            (lambda (text start end &optional unfixable)
-              (push (list text start end unfixable) collected)
+            (lambda (text start end &optional fix)
+              (push (list text start end fix) collected)
               nil))
            (checkdoc-autofix-flag nil)
            (checkdoc-generate-compile-warnings-flag nil)
@@ -2118,13 +2118,14 @@ Calls REPORT-FN directly."
               (checkdoc-current-buffer t)))
         (kill-buffer checkdoc-diagnostic-buffer)))
     (funcall report-fn
-             (cl-loop for (text start end _unfixable) in
+             (cl-loop for (text start end fix) in
                       collected
                       collect
                       (flymake-make-diagnostic
                        (current-buffer)
                        (or start 1) (or end (1+ (or start 1)))
-                       :note text)))
+                       :note text
+                       fix nil (when fix #'list))))
     collected))
 
 (defun elisp-flymake--byte-compile-done (report-fn