From: Eshel Yaron Date: Mon, 3 Jun 2024 08:55:26 +0000 (+0200) Subject: Add Flymake fix suggestions for Checkdoc diagnostics X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=650c2a056af8df85065b2851d3513c1e3d62c60c;p=emacs.git Add Flymake fix suggestions for Checkdoc diagnostics --- diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index c22dfb2eb26..0cd0139045e 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -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 \\\\ & \\\\[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 diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el index 9e727bd4543..a8bb45a4dce 100644 --- a/lisp/progmodes/elisp-mode.el +++ b/lisp/progmodes/elisp-mode.el @@ -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