Updated with latest version. Changes include:

Added checks for basics in messages using `error'.
Added check for symbols that are both functions and symbols.
    These references are ambiguous and should be prefixed with
    "function", or "variable".  Added auto-fix for this also.
Added auto fix for args that do not occur in the doc string.
Fixed question about putting a symbol in `quotes'.
Added spaces to the end of all y/n questions.
Added checks for y/n question endings to require "? "

1 files changed, 226 insertions, 34 deletions

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index fcc589cc929..88b3d0d231f 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -3,7 +3,7 @@
 ;;;  Copyright (C) 1997, 1998  Free Software Foundation
 
 ;; Author: Eric M. Ludlam <zappo@gnu.org>
-;; Version: 0.4.3
+;; Version: 0.5.1
 ;; Keywords: docs, maint, lisp
 
 ;; This file is part of GNU Emacs.
@@ -87,6 +87,14 @@
 ;; skip looking for it by putting the following comment just in front
 ;; of the documentation string: "; checkdoc-params: (args go here)"
 ;;
+;; Checking message strings
+;;
+;;   The text that follows the `error', and `y-or-n-p' commands is
+;; also checked.  The documentation for `error' clearly states some
+;; simple style rules to follow which checkdoc will auto-fix for you.
+;; `y-or-n-p' also states that it should end in a space.  I added that
+;; it should end in "? " since that is almost always used.
+;;
 ;; Adding your own checks:
 ;;
 ;;   You can experiment with adding your own checks by setting the
@@ -173,6 +181,14 @@
 ;;         have comments before the doc-string.
 ;;       Fixed bug where keystrokes were identified from a variable name
 ;;         like ASSOC-P.
+;; 0.5   Added checks for basics in messages using `error'.
+;;       Added check for symbols that are both functions and symbols.
+;;         These references are ambiguous and should be prefixed with
+;;         "function", or "variable".  Added auto-fix for this also.
+;;       Added auto fix for args that do not occur in the doc string.
+;; 0.5.1 Fixed question about putting a symbol in `quotes'.
+;;       Added spaces to the end of all y/n questions.
+;;       Added checks for y/n question endings to require "? "
 
 ;;; TO DO:
 ;;   Hook into the byte compiler on a defun/defver level to generate
@@ -186,7 +202,7 @@
 ;;     not specifically docstring related.  Would this even be useful?
 
 ;;; Code:
-(defvar checkdoc-version "0.4.3"
+(defvar checkdoc-version "0.5.1"
   "Release version of checkdoc you are currently running.")
 
 ;; From custom web page for compatibility between versions of custom:
@@ -463,7 +479,7 @@ be re-created.")
 (defun checkdoc-eval-current-buffer ()
   "Evaluate and check documentation for the current buffer.
 Evaluation is done first because good documentation for something that
-doesn't work is just not useful.  Comments, Doc-strings, and rogue
+doesn't work is just not useful.  Comments, doc-strings, and rogue
 spacing are all verified."
   (interactive)
   (checkdoc-call-eval-buffer nil)
@@ -471,7 +487,7 @@ spacing are all verified."
 
 ;;;###autoload
 (defun checkdoc-current-buffer (&optional take-notes)
-  "Check the current buffer for document style, comment style, and rogue spaces.
+  "Check current buffer for document, comment, error style, and rogue spaces.
 Optional argument TAKE-NOTES non-nil will store all found errors in a
 warnings buffer, otherwise it stops after the first error."
   (interactive "P")
@@ -483,6 +499,7 @@ warnings buffer, otherwise it stops after the first error."
     (or (and buffer-file-name ;; only check comments in a file
 	     (checkdoc-comments take-notes))
 	(checkdoc take-notes)
+	(checkdoc-message-text take-notes)
 	(checkdoc-rogue-spaces take-notes)
 	(not (interactive-p))
 	(message "Checking buffer for style...Done."))))
@@ -651,7 +668,7 @@ if there is one."
   (interactive "P")
   (if take-notes (checkdoc-start-section "checkdoc-comments"))
   (if (not buffer-file-name)
-     (error "Can only check comments for a file buffer."))
+     (error "Can only check comments for a file buffer"))
   (let* ((checkdoc-spellcheck-documentation-flag
 	  (member checkdoc-spellcheck-documentation-flag
 		  '(buffer t)))
@@ -717,13 +734,15 @@ space at the end of each line."
       (let* ((checkdoc-spellcheck-documentation-flag
 	      (member checkdoc-spellcheck-documentation-flag
 		      '(defun t)))
+	     (beg (save-excursion (beginning-of-defun) (point)))
+	     (end (save-excursion (end-of-defun) (point)))
 	     (msg (checkdoc-this-string-valid)))
 	(if msg (if no-error (message msg) (error msg))
-	  (setq msg (checkdoc-rogue-space-check-engine
-		     (save-excursion (beginning-of-defun) (point))
-		     (save-excursion (end-of-defun) (point))))
+	  (setq msg (checkdoc-message-text-search beg end))
 	  (if msg (if no-error (message msg) (error msg))
-	    (if (interactive-p) (message "Checkdoc: done."))))))))
+	    (setq msg (checkdoc-rogue-space-check-engine beg end))
+	    (if msg (if no-error (message msg) (error msg)))))
+	(if (interactive-p) (message "Checkdoc: done."))))))
 
 ;;; Ispell interface for forcing a spell check
 ;;
@@ -809,6 +828,7 @@ Prefix argument TAKE-NOTES is the same as for `checkdoc-defun'"
     (define-key pmap "b" 'checkdoc-current-buffer)
     (define-key pmap "B" 'checkdoc-ispell-current-buffer)
     (define-key pmap "e" 'checkdoc-eval-current-buffer)
+    (define-key pmap "m" 'checkdoc-message-text)
     (define-key pmap "c" 'checkdoc-comments)
     (define-key pmap "C" 'checkdoc-ispell-comments)
     (define-key pmap " " 'checkdoc-rogue-spaces)
@@ -839,6 +859,7 @@ Prefix argument TAKE-NOTES is the same as for `checkdoc-defun'"
        ["Check Comment Style" checkdoc-comments buffer-file-name]
        ["Check Comment Style and Spelling" checkdoc-ispell-comments
 	buffer-file-name]
+       ["Check message text" checkdoc-message-text t]
        ["Check for Rogue Spaces" checkdoc-rogue-spaces t]
        )))
 ;; XEmacs requires some weird stuff to add this menu in a minor mode.
@@ -950,7 +971,7 @@ regexp short cuts work."
 		(looking-at "\\([ \t]+\\)[^ \t\n]"))
 	   (if (checkdoc-autofix-ask-replace (match-beginning 1)
 					     (match-end 1)
-					     "Remove this whitespace?"
+					     "Remove this whitespace? "
 					     "")
 	       nil
 	     "Second line should not have indentation")))
@@ -966,7 +987,7 @@ regexp short cuts work."
 		     (setq start (point)
 			   end (1- e)))))
 	   (if (checkdoc-autofix-ask-replace
-		start end "Remove this whitespace?" "")
+		start end "Remove this whitespace? " "")
 	       nil
 	     "Documentation strings should not start or end with whitespace")))
      ;; * Every command, function, or variable intended for users to know
@@ -1004,7 +1025,7 @@ documentation string"))
 	     nil
 	   (forward-char 1)
 	   (if (checkdoc-autofix-ask-replace
-		(point) (1+ (point)) "Add period to sentence?"
+		(point) (1+ (point)) "Add period to sentence? "
 		".\"" t)
 	       nil
 	     "First sentence should end with punctuation.")))
@@ -1021,7 +1042,7 @@ documentation string"))
 	       ;; Here we have found a complete sentence, but no break.
 	       (if (checkdoc-autofix-ask-replace
 		    (1+ (match-beginning 0)) (match-end 0)
-		    "First line not a complete sentence.  Add CR here?"
+		    "First line not a complete sentence.  Add RET here? "
 		    "\n" t)
 		   (let (l1 l2)
 		     (forward-line 1)
@@ -1033,7 +1054,7 @@ documentation string"))
 				(current-column)))
 		     (if (> (+ l1 l2 1) 80)
 			 (setq msg "Incomplete auto-fix.  Doc-string \
-may require more formatting.")
+may require more formatting")
 		       ;; We can merge these lines!  Replace this CR
 		       ;; with a space.
 		       (delete-char 1) (insert " ")
@@ -1052,7 +1073,7 @@ may require more formatting.")
 			(< (current-column) numc))
 		   (if (checkdoc-autofix-ask-replace
 			p (1+ p)
-			"1st line not a complete sentence. Join these lines?"
+			"1st line not a complete sentence. Join these lines? "
 			" " t)
 		       (progn
 			 ;; They said yes.  We have more fill work to do...
@@ -1066,10 +1087,10 @@ may require more formatting.")
        (if (looking-at "[a-z]")
 	   (if (checkdoc-autofix-ask-replace
 		(match-beginning 0) (match-end 0)
-		"Capitalize your sentence?" (upcase (match-string 0))
+		"Capitalize your sentence? " (upcase (match-string 0))
 		t)
 	       nil
-	     "First line should be capitalized.")
+	     "First line should be capitalized")
 	 nil))
      ;; * For consistency, phrase the verb in the first sentence of a
      ;;   documentation string as an infinitive with "to" omitted.  For
@@ -1100,7 +1121,7 @@ may require more formatting.")
 				(match-beginning 1) (match-end 1))
 		      rs (assoc (downcase original)
 				checkdoc-common-verbs-wrong-voice))
-		(if (not rs) (error "Verb voice alist corrupted."))
+		(if (not rs) (error "Verb voice alist corrupted"))
 		(setq replace (let ((case-fold-search nil))
 				(save-match-data
 				  (if (string-match "^[A-Z]" original)
@@ -1108,14 +1129,14 @@ may require more formatting.")
 				    (cdr rs)))))
 		(if (checkdoc-autofix-ask-replace
 		     (match-beginning 1) (match-end 1)
-		     (format "Wrong voice for verb `%s'.  Replace with `%s'?"
+		     (format "Wrong voice for verb `%s'.  Replace with `%s'? "
 			     original replace)
 		     replace t)
 		    (setq rs nil)))
 	      (if rs
 		  ;; there was a match, but no replace
 		  (format
-		   "Incorrect voice in sentence.  Use `%s' instead of `%s'."
+		   "Incorrect voice in sentence.  Use `%s' instead of `%s'"
 		   replace original)))))
      ;;   * Don't write key sequences directly in documentation strings.
      ;;     Instead, use the `\\[...]' construct to stand for them.
@@ -1139,6 +1160,40 @@ mouse-[0-3]\\)\\)\\>"))
        (if (re-search-forward "\\\\\\\\\\[\\w+" e t
 			      (1+ checkdoc-max-keyref-before-warn))
 	   "Too many occurrences of \\[function].  Use \\{keymap} instead"))
+     ;; Ambiguous quoted symbol. When a symbol is both bound and fbound,
+     ;; and is referred to in documentation, it should be prefixed with
+     ;; something to disambiguate it.  This check must be before the
+     ;; 80 column check because it will probably break that.
+     (save-excursion
+       (let ((case-fold-search t)
+	     (ret nil))
+	 (while (and
+		 (re-search-forward
+		  "\\(\\<\\(variable\\|option\\|function\\|command\\|symbol\\)\
+\\s-+\\)?`\\(\\sw\\(\\sw\\|\\s_\\)+\\)'" e t)
+		 (not ret))
+	   (let ((sym (intern-soft (match-string 3)))
+		 (mb (match-beginning 3)))
+	     (if (and sym (boundp sym) (fboundp sym) (not (match-string 1)))
+		 (if (checkdoc-autofix-ask-replace
+		      mb (match-end 3) "Prefix this ambiguous symbol? "
+		      (match-string 3) t)
+		     ;; We didn't actuall replace anything.  Here we find
+		     ;; out what special word form they wish to use as
+		     ;; a prefix.
+		     (let ((disambiguate
+			    (completing-read
+			     "Disambiguating Keyword (default: variable): "
+			     '(("function") ("command") ("variable")
+			       ("option") ("symbol"))
+			     nil t nil nil "variable")))
+		       (goto-char (1- mb))
+		       (insert disambiguate " ")
+		       (forward-word 1))
+		   (setq ret
+			 (format "Disambiguate %s by preceeding w/ \
+function,command,variable,option or symbol." (match-string 3)))))))
+	 ret))
      ;; * Format the documentation string so that it fits in an
      ;;   Emacs window on an 80-column screen.  It is a good idea
      ;;   for most lines to be no wider than 60 characters.  The
@@ -1179,7 +1234,7 @@ mouse-[0-3]\\)\\)\\>"))
 		    (setq found (intern-soft ms))
 		    (or (boundp found) (fboundp found)))
 	       (progn
-		 (setq msg (format "Lisp symbol %s should appear in `quotes'"
+		 (setq msg (format "Add quotes around lisp symbol `%s'? "
 				   ms))
 		 (if (checkdoc-autofix-ask-replace
 		      (match-beginning 1) (+ (match-beginning 1)
@@ -1192,7 +1247,7 @@ mouse-[0-3]\\)\\)\\>"))
        (if (re-search-forward "\\(`\\(t\\|nil\\)'\\)" e t)
 	   (if (checkdoc-autofix-ask-replace
 		(match-beginning 1) (match-end 1)
-		(format "%s should not appear in quotes. Remove?"
+		(format "%s should not appear in quotes. Remove? "
 			(match-string 2))
 		(match-string 2) t)
 	       nil
@@ -1235,10 +1290,12 @@ mouse-[0-3]\\)\\)\\>"))
 		   (last-pos 0)
 		   (found 1)
 		   (order (and (nth 3 fp) (car (nth 3 fp))))
-		   (nocheck (append '("&optional" "&rest") (nth 3 fp))))
+		   (nocheck (append '("&optional" "&rest") (nth 3 fp)))
+		   (inopts nil))
 	       (while (and args found (> found last-pos))
 		 (if (member (car args) nocheck)
-		     (setq args (cdr args))
+		     (setq args (cdr args)
+			   inopts t)
 		   (setq last-pos found
 			 found (save-excursion
 				 (re-search-forward
@@ -1264,15 +1321,32 @@ mouse-[0-3]\\)\\)\\>"))
 			     (if (checkdoc-autofix-ask-replace
 				  (match-beginning 1) (match-end 1)
 				  (format
-				   "Argument `%s' should appear as `%s'. Fix?"
+				   "Argument `%s' should appear as `%s'. Fix? "
 				   (car args) (upcase (car args)))
 				  (upcase (car args)) t)
 				 (setq found (match-beginning 1))))))
 		   (if found (setq args (cdr args)))))
 	       (if (not found)
-		   (format
-		    "Argument `%s' should appear as `%s' in the doc-string"
-		    (car args) (upcase (car args)))
+		   ;; It wasn't found at all!  Offer to attach this new symbol
+		   ;; to the end of the documentation string.
+		   (if (y-or-n-p
+			(format "Add %s documentation to end of doc-string?"
+				(upcase (car args))))
+		       ;; No do some majic an invent a doc string.
+		       (save-excursion
+			 (goto-char e) (forward-char -1)
+			 (insert "\n"
+				 (if inopts "Optional a" "A")
+				 "rgument " (upcase (car args))
+				 " ")
+			 (insert (read-string "Describe: "))
+			 (if (not (save-excursion (forward-char -1)
+						  (looking-at "[.?!]")))
+			     (insert "."))
+			 nil)
+		     (format
+		      "Argument `%s' should appear as `%s' in the doc-string"
+		      (car args) (upcase (car args))))
 		 (if (or (and order (eq order 'yes))
 			 (and (not order) checkdoc-arguments-in-order-flag))
 		     (if (< found last-pos)
@@ -1488,9 +1562,9 @@ Some editors & news agents may remove it")))
 	 ;; This is not a complex activity
 	 (if (checkdoc-autofix-ask-replace
 	      (match-beginning 1) (match-end 1)
-	      "White space at end of line. Remove?" "")
+	      "White space at end of line. Remove? " "")
 	     nil
-	   (setq msg "White space found at end of line.")))))
+	   (setq msg "White space found at end of line")))))
     ;; Return an error and leave the cursor at that spot, or restore
     ;; the cursor.
     (if msg
@@ -1530,7 +1604,7 @@ Code:, and others referenced in the style guide."
 	   ;; it's set to never
 	   (if (and checkdoc-autofix-flag
 		    (not (eq checkdoc-autofix-flag 'never))
-		    (y-or-n-p "There is no first line summary!  Add one?"))
+		    (y-or-n-p "There is no first line summary!  Add one? "))
 	       (progn
 		 (goto-char (point-min))
 		 (insert ";;; " fn fe " --- " (read-string "Summary: ") "\n"))
@@ -1573,7 +1647,7 @@ Code:, and others referenced in the style guide."
 		   nil t))
 	     (if (and checkdoc-autofix-flag
 		      (not (eq checkdoc-autofix-flag 'never))
-		      (y-or-n-p "No identifiable footer!  Add one?"))
+		      (y-or-n-p "No identifiable footer!  Add one? "))
 		 (progn
 		   (goto-char (point-max))
 		   (insert "\n(provide '" fn ")\n;;; " fn fe " ends here\n"))
@@ -1600,8 +1674,8 @@ Code:, and others referenced in the style guide."
 	       (if (and (checkdoc-outside-major-sexp) ;in code is ok.
 			(checkdoc-autofix-ask-replace
 			 (match-beginning 1) (match-end 1)
-			 "Multiple occurances of ;;; found. Use ;; instead?" ""
-			 complex-replace))
+			 "Multiple occurances of ;;; found. Use ;; instead? "
+			 "" complex-replace))
 		   ;; Learn that, yea, the user did want to do this a
 		   ;; whole bunch of times.
 		   (setq complex-replace nil))
@@ -1636,6 +1710,124 @@ Code:, and others referenced in the style guide."
 	(or (progn (beginning-of-defun) (bobp))
 	    (progn (end-of-defun) (< (point) p)))))))
 
+;;; `error' and `message' text verifier.
+;;
+(defun checkdoc-message-text (&optional take-notes)
+  "Scan the buffer for occurrences of the error function, and verify text.
+Optional argument TAKE-NOTES causes all errors to be logged."
+  (interactive "P")
+  (if take-notes (checkdoc-start-section "checkdoc-message-text"))
+  (let ((p (point))
+	(e (checkdoc-message-text-search)))
+    (if e (if take-notes (checkdoc-error (point) e) (error e)))
+    (if (and take-notes e) (checkdoc-show-diagnostics))
+    (goto-char p))
+  (if (interactive-p) (message "Checking error message text...done.")))
+    
+(defun checkdoc-message-text-search (&optional beg end)
+  "Search between BEG and END for an error with `error'.
+Optional arguments BEG and END represent the boundary of the check.
+The default boundary is the entire buffer."
+  (let ((e nil))
+    (if (not (or beg end)) (setq beg (point-min) end (point-max)))
+    (goto-char beg)
+    (while (and (not e) (re-search-forward "(\\s-*error[ \t\n]" end t))
+      (if (looking-at "\"")
+	  (setq e (checkdoc-message-text-engine 'error))))
+    (goto-char beg)
+    (while (and (not e) (re-search-forward
+			 "\\<y-or-n-p\\(-with-timeout\\)?[ \t\n]" end t))
+      ;; Format is common as a first arg..
+      (if (looking-at "(format[ \t\n]") (goto-char (match-end 0)))
+      (if (looking-at "\"")
+	  (setq e (checkdoc-message-text-engine 'y-or-n-p))))
+    (goto-char beg)
+    ;; this is cheating for checkdoc only.
+    (while (and (not e) (re-search-forward
+			 "(checkdoc-autofix-ask-replace[ \t\n]"
+			 end t))
+      (forward-sexp 2)
+      (skip-chars-forward " \t\n")
+      (if (looking-at "(format[ \t\n]") (goto-char (match-end 0)))
+      (if (looking-at "\"")
+	  (setq e (checkdoc-message-text-engine 'y-or-n-p))))
+    ;; Is it worth adding checks for read commands too? That would
+    ;; require fixing up `interactive' which could be unpleasant.
+    ;; Most people get that right by accident anyway.
+    e))
+  
+(defun checkdoc-message-text-engine (type)
+  "Return or fix errors found in strings passed to a message display function.
+According to the documentation for the function `error', the error string
+should not end with a period, and should start with a capitol letter.
+The function `y-or-n-p' has similar constraints.
+Argument TYPE specifies the type of question, such as `error or `y-or-n-p."
+  (let ((case-fold-search nil))
+    (or
+     ;; From the documentation of the symbol `error':
+     ;; In Emacs, the convention is that error messages start with a capital
+     ;; letter but *do not* end with a period.  Please follow this convention
+     ;; for the sake of consistency.
+     (if (and (save-excursion (forward-char 1)
+			      (looking-at "[a-z]\\w+"))
+	      (not (checkdoc-autofix-ask-replace
+		    (match-beginning 0) (match-end 0)
+		    "Capitalize your message text? "
+		    (capitalize (match-string 0))
+		    t)))
+	 "Messages should start with a capitol letter"
+       nil)
+     (if (and (eq type 'error)
+	      (save-excursion (forward-sexp 1)
+			      (forward-char -2)
+			      (looking-at "\\."))
+	      (not (checkdoc-autofix-ask-replace (match-beginning 0)
+						 (match-end 0)
+						 "Remove period from error? "
+						 ""
+						 t)))
+	 "Error messages should *not* end with a period"
+       nil)
+     ;; `y-or-n-p' documentation explicitly says:
+     ;; It should end in a space; `y-or-n-p' adds `(y or n) ' to it.
+     ;; I added the ? requirement.  Without it, it is unclear that we
+     ;; ask a question and it appears to be an undocumented style.
+     (if (and (eq type 'y-or-n-p)
+	      (save-excursion (forward-sexp 1)
+			      (forward-char -3)
+			      (not (looking-at "\\? ")))
+	      (if (save-excursion (forward-sexp 1)
+				  (forward-char -2)
+				  (looking-at "\\?"))
+		  ;; If we see a ?, then replace with "? ".
+		  (if (checkdoc-autofix-ask-replace
+		       (match-beginning 0) (match-end 0)
+		       "y-or-n-p text should endwith \"? \".  Fix? "
+		       "? " t)
+		      nil
+		    "y-or-n-p text should endwith \"? \".")
+		(if (save-excursion (forward-sexp 1)
+				    (forward-char -2)
+				    (looking-at " "))
+		    (if (checkdoc-autofix-ask-replace
+			 (match-beginning 0) (match-end 0)
+			 "y-or-n-p text should endwith \"? \".  Fix? "
+			 "? " t)
+			nil
+		      "y-or-n-p text should endwith \"? \".")
+		  (if (and ;; if this isn't true, we have a problem.
+		       (save-excursion (forward-sexp 1)
+				       (forward-char -1)
+				       (looking-at "\""))
+		       (checkdoc-autofix-ask-replace
+			(match-beginning 0) (match-end 0)
+			"y-or-n-p text should endwith \"? \".  Fix? "
+			"? \"" t))
+		      nil
+		    "y-or-n-p text should endwith \"? \"."))))
+	 nil)
+     )))
+
 ;;; Auto-fix helper functions
 ;;
 (defun checkdoc-autofix-ask-replace (start end question replacewith