From 6535fd1fa9ac21238a168916249ac59677a6118e Mon Sep 17 00:00:00 2001 From: akater Date: Tue, 20 Jul 2021 01:25:01 +0000 Subject: Evaluate eql specializers * lisp/emacs-lisp/cl-generic.el (cl-generic-generalizers): Evaluate forms that are eql specializers. Provide backward compatibility with a warning. * test/lisp/emacs-lisp/cl-generic-tests.el: Add a test. * lisp/emacs-lisp/bindat.el (bindat--type): Adhere to the new rule. * lisp/emacs-lisp/edebug.el (edebug--match-&-spec-op): Adhere to the new rule. * lisp/emacs-lisp/map.el (map-into): Adhere to the new rule. * lisp/emacs-lisp/radix-tree.el (map-into): Adhere to the new rule. * lisp/frame.el (cl-generic-define-context-rewriter): Adhere to the new rule. * lisp/gnus/gnus-search.el (gnus-search-transform-expression): Adhere to the new rule. * lisp/image/image-converter.el (image-converter--probe image-converter--convert): Adhere to the new rule. * lisp/mail/smtpmail.el (smtpmail-try-auth-method): Adhere to the new rule. * lisp/progmodes/elisp-mode.el (xref-backend-definitions) (xref-backend-apropos): Adhere to the new rule. * lisp/progmodes/etags.el (xref-backend-identifier-at-point) (xref-backend-identifier-completion-table) (xref-backend-identifier-completion-ignore-case) (xref-backend-definitions)(xref-backend-apropos): Adhere to the new rule. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-cl-defmethod-with-types-ok) (checkdoc-cl-defmethod-qualified-ok) (checkdoc-cl-defmethod-with-extra-qualifier-ok): Adhere to the new rule. * etc/NEWS: Describe the change. --- test/lisp/emacs-lisp/checkdoc-tests.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index 7a7aa9fb3cd..2a1d8b27636 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -49,27 +49,27 @@ (with-temp-buffer (emacs-lisp-mode) ;; this method matches if A is the symbol `smthg' and if b is a list: - (insert "(cl-defmethod foo ((a (eql smthg)) (b list)) \"Return A+B.\")") + (insert "(cl-defmethod foo ((a (eql 'smthg)) (b list)) \"Return A+B.\")") (checkdoc-defun))) (ert-deftest checkdoc-cl-defmethod-qualified-ok () "Checkdoc should be happy with a `cl-defmethod' using qualifiers." (with-temp-buffer (emacs-lisp-mode) - (insert "(cl-defmethod test :around ((a (eql smthg))) \"Return A.\")") + (insert "(cl-defmethod test :around ((a (eql 'smthg))) \"Return A.\")") (checkdoc-defun))) (ert-deftest checkdoc-cl-defmethod-with-extra-qualifier-ok () "Checkdoc should be happy with a :extra qualified `cl-defmethod'." (with-temp-buffer (emacs-lisp-mode) - (insert "(cl-defmethod foo :extra \"foo\" ((a (eql smthg))) \"Return A.\")") + (insert "(cl-defmethod foo :extra \"foo\" ((a (eql 'smthg))) \"Return A.\")") (checkdoc-defun)) (with-temp-buffer (emacs-lisp-mode) (insert - "(cl-defmethod foo :extra \"foo\" :after ((a (eql smthg))) \"Return A.\")") + "(cl-defmethod foo :extra \"foo\" :after ((a (eql 'smthg))) \"Return A.\")") (checkdoc-defun))) (ert-deftest checkdoc-cl-defmethod-with-extra-qualifier-and-nil-args-ok () -- cgit v1.2.3 From 2110973351f01fb5cdf90b705acb58354b608050 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Mon, 13 Sep 2021 06:03:44 +0200 Subject: Improve checkdoc abbreviation handling * lisp/emacs-lisp/checkdoc.el (checkdoc-in-abbreviation-p): New helper function. (checkdoc-sentencespace-region-engine): Fix handling abbreviations after escaped parenthesis. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-tests-in-abbrevation-p) (checkdoc-tests-in-abbrevation-p/with-parens) (checkdoc-tests-in-abbrevation-p/with-escaped-parens): New tests. --- lisp/emacs-lisp/checkdoc.el | 36 ++++++++++++++++++++++++---------- test/lisp/emacs-lisp/checkdoc-tests.el | 24 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index 00cc7777e1a..1ab44e0f665 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -1998,6 +1998,31 @@ The text checked is between START and LIMIT." (setq c (1+ c))) (and (< 0 c) (= (% c 2) 0)))))) +(defun checkdoc-in-abbreviation-p (begin) + "Return non-nil if point is at an abbreviation. +Examples of abbreviations handled: \"e.g.\", \"i.e.\", \"cf.\"." + (save-excursion + (goto-char begin) + (condition-case nil + (progn + (forward-sexp -1) + ;; Piece of an abbreviation. + (looking-at + (rx (or letter ; single letter, as in "a." + (seq + ;; There might exist an escaped parenthesis, as + ;; this is often used in docstrings. In this + ;; case, `forward-sexp' will have skipped over it, + ;; so we need to skip it here too. + (? "\\(") + ;; The abbreviations: + (or (seq (any "iI") "." (any "eE")) ; i.e. + (seq (any "eE") ".g") ; e.g. + (seq (any "cC") "f"))) ; c.f. + "vs") ; vs. + "."))) + (error t)))) + (defun checkdoc-proper-noun-region-engine (begin end) "Check all text between BEGIN and END for lower case proper nouns. These are Emacs centric proper nouns which should be capitalized for @@ -2060,16 +2085,7 @@ If the offending word is in a piece of quoted text, then it is skipped." (e (match-end 1))) (unless (or (checkdoc-in-sample-code-p begin end) (checkdoc-in-example-string-p begin end) - (save-excursion - (goto-char b) - (condition-case nil - (progn - (forward-sexp -1) - ;; piece of an abbreviation - ;; FIXME etc - (looking-at - "\\([a-zA-Z]\\|[iI]\\.?e\\|[eE]\\.?g\\|[cC]f\\)\\.")) - (error t)))) + (checkdoc-in-abbreviation-p b)) (if (checkdoc-autofix-ask-replace b e "There should be two spaces after a period. Fix? " diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index 2a1d8b27636..a4b252031fe 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -122,4 +122,28 @@ See the comments in Bug#24998." (should (looking-at-p "\"baz\")")) (should-not (checkdoc-next-docstring)))) +(ert-deftest checkdoc-tests-in-abbrevation-p () + (with-temp-buffer + (emacs-lisp-mode) + (insert "foo bar e.g. baz") + (goto-char (point-min)) + (re-search-forward "e.g") + (should (checkdoc-in-abbreviation-p (point))))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/with-parens () + (with-temp-buffer + (emacs-lisp-mode) + (insert "foo bar (e.g. baz)") + (goto-char (point-min)) + (re-search-forward "e.g") + (should (checkdoc-in-abbreviation-p (point))))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/with-escaped-parens () + (with-temp-buffer + (emacs-lisp-mode) + (insert "foo\n\\(e.g. baz)") + (goto-char (point-min)) + (re-search-forward "e.g") + (should (checkdoc-in-abbreviation-p (point))))) + ;;; checkdoc-tests.el ends here -- cgit v1.2.3 From b6bff3ba7971daad737690d88a3921f1dd190476 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Thu, 16 Sep 2021 19:13:56 +0200 Subject: checkdoc: 'y-or-n-p' no longer needs space * lisp/emacs-lisp/checkdoc.el (checkdoc-message-text-engine): Change 'y-or-n-p' check to accept prompt ending with both "? " or "?", that is, it no longer needs the space. (Bug#50621) (checkdoc--fix-y-or-n-p): New helper function. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-tests-fix-y-or-n-p) (checkdoc-tests-fix-y-or-n-p/no-change) (checkdoc-tests-fix-y-or-n-p/with-space): New tests. --- lisp/emacs-lisp/checkdoc.el | 82 +++++++++++++--------------------- test/lisp/emacs-lisp/checkdoc-tests.el | 30 +++++++++++++ 2 files changed, 62 insertions(+), 50 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index 01f2c0d95f7..f8df223ce96 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -2475,6 +2475,31 @@ Argument END is the maximum bounds to search in." (setq return type)))) return)) +(defun checkdoc--fix-y-or-n-p () + "Fix `y-or-n-p' prompt to end with \"?\" or \"? \". +The space is technically redundant, but also more compatible with +Emacs versions before Emacs 24.1. In the future, we might treat +a space as a style error." + (when (and (save-excursion (forward-sexp 1) + (forward-char -3) + (not (looking-at "\\? "))) + (save-excursion (forward-sexp 1) + (forward-char -2) + (not (looking-at "\\?")))) + (if (and + (save-excursion (forward-sexp 1) + (forward-char -1) + (looking-at "\"")) + (checkdoc-autofix-ask-replace + (match-beginning 0) (match-end 0) + (format-message + "`y-or-n-p' argument should end with \"? \". Fix?") + "?\"" t)) + nil + (checkdoc-create-error + "`y-or-n-p' argument should end with \"?\"" + (match-beginning 0) (match-end 0))))) + (defun checkdoc-message-text-engine (&optional type) "Return or fix errors found in strings passed to a message display function. According to the documentation for the function `error', the error list @@ -2530,63 +2555,20 @@ Argument TYPE specifies the type of question, such as `error' or `y-or-n-p'." "Error messages should *not* end with a period" (match-beginning 0) (match-end 0)) 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 (eq type 'y-or-n-p) - (if (not (save-excursion (forward-sexp 1) - (forward-char -3) - (not (looking-at "\\? ")))) - nil - (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) - (format-message - "`y-or-n-p' argument should end with \"? \". Fix? ") - "? " t) - nil - (checkdoc-create-error - "`y-or-n-p' argument should end with \"? \"" - (match-beginning 0) (match-end 0))) - (if (save-excursion (forward-sexp 1) - (forward-char -2) - (looking-at " ")) - (if (checkdoc-autofix-ask-replace - (match-beginning 0) (match-end 0) - (format-message - "`y-or-n-p' argument should end with \"? \". Fix? ") - "? " t) - nil - (checkdoc-create-error - "`y-or-n-p' argument should end with \"? \"" - (match-beginning 0) (match-end 0))) - (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) - (format-message - "`y-or-n-p' argument should end with \"? \". Fix? ") - "? \"" t)) - nil - (checkdoc-create-error - "`y-or-n-p' argument should end with \"? \"" - (match-beginning 0) (match-end 0))))))) + ;; From `(elisp) Programming Tips': "A question asked in the + ;; minibuffer with `yes-or-no-p' or `y-or-n-p' should start with + ;; a capital letter and end with '?'." + (when (eq type 'y-or-n-p) + (checkdoc--fix-y-or-n-p)) ;; Now, let's just run the spell checker on this guy. (checkdoc-ispell-docstring-engine (save-excursion (forward-sexp 1) - (point))) - ))) + (point)))))) ;;; Auto-fix helper functions ;; (defun checkdoc-y-or-n-p (question) "Like `y-or-n-p', but pays attention to `checkdoc-autofix-flag'. -Argument QUESTION is the prompt passed to `y-or-n-p'." + Argument QUESTION is the prompt passed to `y-or-n-p'." (prog1 (if (or (not checkdoc-autofix-flag) (eq checkdoc-autofix-flag 'never)) diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index a4b252031fe..3eb7da3d4a7 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -146,4 +146,34 @@ See the comments in Bug#24998." (re-search-forward "e.g") (should (checkdoc-in-abbreviation-p (point))))) +(ert-deftest checkdoc-tests-fix-y-or-n-p () + (with-temp-buffer + (emacs-lisp-mode) + (let ((standard-output (current-buffer)) + (checkdoc-autofix-flag 'automatic)) + (prin1 '(y-or-n-p "foo")) ; "foo" + (goto-char (length "(y-or-n-p ")) + (checkdoc--fix-y-or-n-p) + (should (equal (buffer-string) "(y-or-n-p \"foo?\")"))))) + +(ert-deftest checkdoc-tests-fix-y-or-n-p/no-change () + (with-temp-buffer + (emacs-lisp-mode) + (let ((standard-output (current-buffer)) + (checkdoc-autofix-flag 'automatic)) + (prin1 '(y-or-n-p "foo?")) ; "foo?" + (goto-char (length "(y-or-n-p ")) + (checkdoc--fix-y-or-n-p) + (should (equal (buffer-string) "(y-or-n-p \"foo?\")"))))) + +(ert-deftest checkdoc-tests-fix-y-or-n-p/with-space () + (with-temp-buffer + (emacs-lisp-mode) + (let ((standard-output (current-buffer)) + (checkdoc-autofix-flag 'automatic)) + (prin1 '(y-or-n-p "foo? ")) ; "foo? " + (goto-char (length "(y-or-n-p ")) + (checkdoc--fix-y-or-n-p) + (should (equal (buffer-string) "(y-or-n-p \"foo? \")"))))) + ;;; checkdoc-tests.el ends here -- cgit v1.2.3 From 55083d90a30628d9eaa5b94196291ca15098aed0 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Thu, 23 Sep 2021 21:10:08 +0200 Subject: Avoid jumping too far in checkdoc-in-abbreviation-p * lisp/emacs-lisp/checkdoc.el (checkdoc-in-abbreviation-p): Use 'forward-ward' instead of 'forward-sexp' to avoid jumping too far in some situations. (Bug#50731) * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-tests--abbrev-test): New helper function. (checkdoc-tests-in-abbrevation-p/basic-case): Rename from 'checkdoc-tests-in-abbrevation-p'. (checkdoc-tests-in-abbrevation-p/with-parens) (checkdoc-tests-in-abbrevation-p/with-escaped-parens): Use above new helper function. (checkdoc-tests-in-abbrevation-p/single-char) (checkdoc-tests-in-abbrevation-p/with-em-dash) (checkdoc-tests-in-abbrevation-p/incorrect-abbreviation): New tests. --- lisp/emacs-lisp/checkdoc.el | 42 +++++++++++++++++++--------------- test/lisp/emacs-lisp/checkdoc-tests.el | 34 ++++++++++++++------------- 2 files changed, 41 insertions(+), 35 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index 5224a943ac0..4243e828001 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -2097,27 +2097,31 @@ Examples of abbreviations handled: \"e.g.\", \"i.e.\", \"cf.\"." (save-excursion (goto-char begin) (condition-case nil - (progn - (forward-sexp -1) + (let ((single-letter t)) + (forward-word -1) + ;; Skip over all dots backwards, as `forward-word' will only + ;; go one dot at a time in a string like "e.g.". + (while (save-excursion (forward-char -1) + (looking-at (rx "."))) + (setq single-letter nil) + (forward-word -1)) ;; Piece of an abbreviation. (looking-at - (rx (or letter ; single letter, as in "a." - (seq - ;; There might exist an escaped parenthesis, as - ;; this is often used in docstrings. In this - ;; case, `forward-sexp' will have skipped over it, - ;; so we need to skip it here too. - (? "\\(") - ;; The abbreviations: - (or (seq (any "cC") "f") ; cf. - (seq (any "eE") ".g") ; e.g. - (seq (any "iI") "." (any "eE")))) ; i.e. - "etc" ; etc. - "vs" ; vs. - ;; Some non-standard or less common ones that we - ;; might as well ignore. - "Inc" "Univ" "misc" "resp") - "."))) + (if single-letter + ;; Handle a single letter, as in "a.", as this might be + ;; a part of a list. + (rx letter ".") + (rx (or + ;; The abbreviations: + (seq (or (seq (any "cC") "f") ; cf. + (seq (any "eE") ".g") ; e.g. + (seq (any "iI") "." (any "eE")))) ; i.e. + "etc" ; etc. + "vs" ; vs. + ;; Some non-standard or less common ones that we + ;; might as well ignore. + "Inc" "Univ" "misc" "resp") + ".")))) (error t)))) (defun checkdoc-proper-noun-region-engine (begin end) diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index 3eb7da3d4a7..13b6d134e5c 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -122,29 +122,31 @@ See the comments in Bug#24998." (should (looking-at-p "\"baz\")")) (should-not (checkdoc-next-docstring)))) -(ert-deftest checkdoc-tests-in-abbrevation-p () +(defun checkdoc-tests--abbrev-test (buffer-contents goto-string) (with-temp-buffer (emacs-lisp-mode) - (insert "foo bar e.g. baz") + (insert buffer-contents) (goto-char (point-min)) - (re-search-forward "e.g") - (should (checkdoc-in-abbreviation-p (point))))) + (re-search-forward goto-string) + (checkdoc-in-abbreviation-p (point)))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/basic-case () + (should (checkdoc-tests--abbrev-test "foo bar e.g. baz" "e.g"))) (ert-deftest checkdoc-tests-in-abbrevation-p/with-parens () - (with-temp-buffer - (emacs-lisp-mode) - (insert "foo bar (e.g. baz)") - (goto-char (point-min)) - (re-search-forward "e.g") - (should (checkdoc-in-abbreviation-p (point))))) + (should (checkdoc-tests--abbrev-test "foo bar (e.g. baz)" "e.g"))) (ert-deftest checkdoc-tests-in-abbrevation-p/with-escaped-parens () - (with-temp-buffer - (emacs-lisp-mode) - (insert "foo\n\\(e.g. baz)") - (goto-char (point-min)) - (re-search-forward "e.g") - (should (checkdoc-in-abbreviation-p (point))))) + (should (checkdoc-tests--abbrev-test "foo\n\\(e.g. baz)" "e.g"))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/single-char () + (should (checkdoc-tests--abbrev-test "a. foo bar" "a"))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/with-em-dash () + (should (checkdoc-tests--abbrev-test "foo bar baz---e.g." "e.g"))) + +(ert-deftest checkdoc-tests-in-abbrevation-p/incorrect-abbreviation () + (should-not (checkdoc-tests--abbrev-test "foo bar a.b.c." "a.b.c"))) (ert-deftest checkdoc-tests-fix-y-or-n-p () (with-temp-buffer -- cgit v1.2.3 From f17fb37c517573652de538e2843043db7603f9e9 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Fri, 24 Sep 2021 23:00:57 +0200 Subject: Fix recently introduced bug in checkdoc * lisp/emacs-lisp/checkdoc.el (checkdoc-in-abbreviation-p): Fix recently introduced bug where some abbreviations weren't recognized. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-tests-in-abbrevation-p/basic-case): Extend test. --- lisp/emacs-lisp/checkdoc.el | 5 +++-- test/lisp/emacs-lisp/checkdoc-tests.el | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index df93d392417..0862e66ac5d 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -2105,14 +2105,15 @@ Examples of recognized abbreviations: \"e.g.\", \"i.e.\", \"cf.\"." (save-excursion (goto-char begin) (condition-case nil - (let ((single-letter t)) + (let (single-letter) (forward-word -1) ;; Skip over all dots backwards, as `forward-word' will only ;; go one dot at a time in a string like "e.g.". (while (save-excursion (forward-char -1) (looking-at (rx "."))) - (setq single-letter nil) (forward-word -1)) + (when (= (point) (1- begin)) + (setq single-letter t)) ;; Piece of an abbreviation. (looking-at (if single-letter diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index 13b6d134e5c..d452024b8ff 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -131,7 +131,10 @@ See the comments in Bug#24998." (checkdoc-in-abbreviation-p (point)))) (ert-deftest checkdoc-tests-in-abbrevation-p/basic-case () - (should (checkdoc-tests--abbrev-test "foo bar e.g. baz" "e.g"))) + (should (checkdoc-tests--abbrev-test "foo bar e.g. baz" "e.g")) + (should (checkdoc-tests--abbrev-test "behavior/errors etc. that" "etc")) + (should (checkdoc-tests--abbrev-test "foo vs. bar" "vs")) + (should (checkdoc-tests--abbrev-test "spy a.k.a. spy" "a.k.a"))) (ert-deftest checkdoc-tests-in-abbrevation-p/with-parens () (should (checkdoc-tests--abbrev-test "foo bar (e.g. baz)" "e.g"))) -- cgit v1.2.3 From 3cabf64131b93e1b0510a01bcce8efde38b20bc4 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Sun, 26 Sep 2021 01:20:55 +0200 Subject: checkdoc: Allow Lisp symbols to start a message * lisp/emacs-lisp/checkdoc.el (checkdoc-message-text-engine): Allow Lisp symbols to start a message. (checkdoc--error-bad-format-p): New helper function. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-test-error-format-is-good) (checkdoc-test-error-format-is-bad): New helper functions. (checkdoc-tests-error-message-bad-format-p) (checkdoc-tests-error-message-bad-format-p/defined-symbols) (checkdoc-tests-error-message-bad-format-p/not-capitalized): New tests. --- lisp/emacs-lisp/checkdoc.el | 37 +++++++++++++++++++++++++++------- test/lisp/emacs-lisp/checkdoc-tests.el | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) (limited to 'test/lisp/emacs-lisp/checkdoc-tests.el') diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el index b3707bb7863..5ea2f59ee60 100644 --- a/lisp/emacs-lisp/checkdoc.el +++ b/lisp/emacs-lisp/checkdoc.el @@ -164,6 +164,7 @@ (require 'cl-lib) (require 'help-mode) ;; for help-xref-info-regexp (require 'thingatpt) ;; for handy thing-at-point-looking-at +(require 'lisp-mode) ;; for lisp-mode-symbol-regexp (require 'dired) ;; for dired-get-filename and dired-map-over-marks (require 'lisp-mnt) @@ -2575,6 +2576,30 @@ Argument END is the maximum bounds to search in." (setq return type)))) return)) +(defun checkdoc--error-bad-format-p () + "Return non-nil if the start of error message at point has the wrong format. +The correct format is \"Foo\" or \"some-symbol: Foo\". See also +`error' and Info node `(elisp) Documentation Tips'." + (save-excursion + ;; Skip the first quote character in string. + (forward-char 1) + ;; A capital letter is always okay. + (unless (let ((case-fold-search nil)) + (looking-at (rx (or upper-case "%s")))) + ;; A defined Lisp symbol is always okay. + (unless (and (looking-at (rx (group (regexp lisp-mode-symbol-regexp)))) + (or (fboundp (intern (match-string 1))) + (boundp (intern (match-string 1))))) + ;; Other Lisp symbols are sometimes okay. + (rx-let ((c (? "\\\n"))) ; `c' is for a continued line + (let ((case-fold-search nil) + (some-symbol (rx (regexp lisp-mode-symbol-regexp) + c ":" c (+ (any " \t\n")))) + (lowercase-str (rx c (group (any "a-z") (+ wordchar))))) + (if (looking-at some-symbol) + (looking-at (concat some-symbol lowercase-str)) + (looking-at lowercase-str)))))))) + (defun checkdoc--fix-y-or-n-p () "Fix `y-or-n-p' prompt to end with \"?\" or \"? \". The space is technically redundant, but also more compatible with @@ -2622,16 +2647,14 @@ Argument TYPE specifies the type of question, such as `error' or `y-or-n-p'." ;; 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+")) + (if (and (checkdoc--error-bad-format-p) (not (checkdoc-autofix-ask-replace - (match-beginning 0) (match-end 0) + (match-beginning 1) (match-end 1) "Capitalize your message text?" - (capitalize (match-string 0)) + (capitalize (match-string 1)) t))) - (checkdoc-create-error - "Messages should start with a capital letter" - (match-beginning 0) (match-end 0)) + (checkdoc-create-error "Messages should start with a capital letter" + (match-beginning 1) (match-end 1)) nil) ;; In general, sentences should have two spaces after the period. (checkdoc-sentencespace-region-engine (point) diff --git a/test/lisp/emacs-lisp/checkdoc-tests.el b/test/lisp/emacs-lisp/checkdoc-tests.el index d452024b8ff..ef49e71599a 100644 --- a/test/lisp/emacs-lisp/checkdoc-tests.el +++ b/test/lisp/emacs-lisp/checkdoc-tests.el @@ -151,6 +151,43 @@ See the comments in Bug#24998." (ert-deftest checkdoc-tests-in-abbrevation-p/incorrect-abbreviation () (should-not (checkdoc-tests--abbrev-test "foo bar a.b.c." "a.b.c"))) +(defun checkdoc-test-error-format-is-good (msg &optional reverse literal) + (with-temp-buffer + (erase-buffer) + (emacs-lisp-mode) + (let ((standard-output (current-buffer))) + (if literal + (print (format "(error \"%s\")" msg)) + (prin1 `(error ,msg)))) + (goto-char (length "(error \"")) + (if reverse + (should (checkdoc--error-bad-format-p)) + (should-not (checkdoc--error-bad-format-p))))) + +(defun checkdoc-test-error-format-is-bad (msg &optional literal) + (checkdoc-test-error-format-is-good msg t literal)) + +(ert-deftest checkdoc-tests-error-message-bad-format-p () + (checkdoc-test-error-format-is-good "Foo") + (checkdoc-test-error-format-is-good "Foo: bar baz") + (checkdoc-test-error-format-is-good "some-symbol: Foo") + (checkdoc-test-error-format-is-good "`some-symbol' foo bar") + (checkdoc-test-error-format-is-good "%sfoo") + (checkdoc-test-error-format-is-good "avl-tree-enter:\\ + Updated data does not match existing data" nil 'literal)) + +(ert-deftest checkdoc-tests-error-message-bad-format-p/defined-symbols () + (defvar checkdoc-tests--var-symbol nil) + (checkdoc-test-error-format-is-good "checkdoc-tests--var-symbol foo bar baz") + (defun checkdoc-tests--fun-symbol ()) + (checkdoc-test-error-format-is-good "checkdoc-tests--fun-symbol foo bar baz")) + +(ert-deftest checkdoc-tests-error-message-bad-format-p/not-capitalized () + (checkdoc-test-error-format-is-bad "foo") + (checkdoc-test-error-format-is-bad "some-symbol: foo") + (checkdoc-test-error-format-is-bad "avl-tree-enter:\ + updated data does not match existing data")) + (ert-deftest checkdoc-tests-fix-y-or-n-p () (with-temp-buffer (emacs-lisp-mode) -- cgit v1.2.3