Commit b2790db0 authored by Noam Postavsky's avatar Noam Postavsky

Improve errors & warnings due to fancy quoted vars (Bug#32939)

Add some hints to the message for byte compiler free & unused variable
warnings, and 'void-variable' errors where the variable has confusable
quote characters in it.
* lisp/help.el (uni-confusables), uni-confusables-regexp): New
constants.
(help-command-error-confusable-suggestions): New function, added to
`command-error-function'.
(help-uni-confusable-suggestions): New function.
* lisp/emacs-lisp/bytecomp.el (byte-compile-variable-ref):
* lisp/emacs-lisp/cconv.el (cconv--analyze-use): Use it.

* lisp/emacs-lisp/lisp-mode.el
(lisp--match-confusable-symbol-character): New function.
(lisp-fdefs): Use it to fontify confusable characters with
font-lock-warning-face when they occur in symbol names.
* doc/lispref/modes.texi (Faces for Font Lock):
* doc/lispref/objects.texi (Basic Char Syntax): Recommend backslash
escaping of confusable characters, and mention new fontification.
* etc/NEWS: Announce the new fontification behavior.
* test/lisp/emacs-lisp/lisp-mode-tests.el (lisp-fontify-confusables):
New test.
parent 85f586f3
Pipeline #4192 failed with stage
in 56 minutes and 28 seconds
...@@ -3287,7 +3287,8 @@ assigned using the ordering as a guide. ...@@ -3287,7 +3287,8 @@ assigned using the ordering as a guide.
@table @code @table @code
@item font-lock-warning-face @item font-lock-warning-face
@vindex font-lock-warning-face @vindex font-lock-warning-face
for a construct that is peculiar, or that greatly changes the meaning of for a construct that is peculiar (e.g., an unescaped confusable quote
in an Emacs Lisp symbol like @samp{‘foo}), or that greatly changes the meaning of
other text, like @samp{;;;###autoload} in Emacs Lisp and @samp{#error} other text, like @samp{;;;###autoload} in Emacs Lisp and @samp{#error}
in C. in C.
......
...@@ -424,7 +424,11 @@ without a special escape meaning; thus, @samp{?\+} is equivalent to ...@@ -424,7 +424,11 @@ without a special escape meaning; thus, @samp{?\+} is equivalent to
characters. However, you must add a backslash before any of the characters. However, you must add a backslash before any of the
characters @samp{()[]\;"}, and you should add a backslash before any characters @samp{()[]\;"}, and you should add a backslash before any
of the characters @samp{|'`#.,} to avoid confusing the Emacs commands of the characters @samp{|'`#.,} to avoid confusing the Emacs commands
for editing Lisp code. You can also add a backslash before whitespace for editing Lisp code. You should also add a backslash before Unicode
characters which resemble the previously mentioned @acronym{ASCII}
ones, to avoid confusing people reading your code. Emacs will
highlight some non-escaped commonly confused characters such as
@samp{‘} to encourage this. You can also add a backslash before whitespace
characters such as space, tab, newline and formfeed. However, it is characters such as space, tab, newline and formfeed. However, it is
cleaner to use one of the easily readable escape sequences, such as cleaner to use one of the easily readable escape sequences, such as
@samp{\t} or @samp{\s}, instead of an actual whitespace character such @samp{\t} or @samp{\s}, instead of an actual whitespace character such
......
...@@ -2972,6 +2972,11 @@ and if the new behavior breaks your code please email ...@@ -2972,6 +2972,11 @@ and if the new behavior breaks your code please email
<32252@debbugs.gnu.org>. Because '%o' and '%x' can now format signed <32252@debbugs.gnu.org>. Because '%o' and '%x' can now format signed
integers, they now support the '+' and space flags. integers, they now support the '+' and space flags.
+++
** In Emacs Lisp mode, symbols with confusable quotes are highlighted.
For example, the first character in '‘foo' would be highlighted in
'font-lock-warning-face'.
+++ +++
** Omitting variables after '&optional' and '&rest' is now allowed. ** Omitting variables after '&optional' and '&rest' is now allowed.
For example '(defun foo (&optional))' is no longer an error. This is For example '(defun foo (&optional))' is no longer an error. This is
......
...@@ -3428,7 +3428,10 @@ for symbols generated by the byte compiler itself." ...@@ -3428,7 +3428,10 @@ for symbols generated by the byte compiler itself."
(boundp var) (boundp var)
(memq var byte-compile-bound-variables) (memq var byte-compile-bound-variables)
(memq var byte-compile-free-references)) (memq var byte-compile-free-references))
(byte-compile-warn "reference to free variable `%S'" var) (let* ((varname (prin1-to-string var))
(suggestions (help-uni-confusable-suggestions varname)))
(byte-compile-warn "reference to free variable `%s'%s" varname
(if suggestions (concat "\n " suggestions) "")))
(push var byte-compile-free-references)) (push var byte-compile-free-references))
(byte-compile-dynamic-variable-op 'byte-varref var)))) (byte-compile-dynamic-variable-op 'byte-varref var))))
...@@ -3444,7 +3447,10 @@ for symbols generated by the byte compiler itself." ...@@ -3444,7 +3447,10 @@ for symbols generated by the byte compiler itself."
(boundp var) (boundp var)
(memq var byte-compile-bound-variables) (memq var byte-compile-bound-variables)
(memq var byte-compile-free-assignments)) (memq var byte-compile-free-assignments))
(byte-compile-warn "assignment to free variable `%s'" var) (let* ((varname (prin1-to-string var))
(suggestions (help-uni-confusable-suggestions varname)))
(byte-compile-warn "assignment to free variable `%s'%s" varname
(if suggestions (concat "\n " suggestions) "")))
(push var byte-compile-free-assignments)) (push var byte-compile-free-assignments))
(byte-compile-dynamic-variable-op 'byte-varset var)))) (byte-compile-dynamic-variable-op 'byte-varset var))))
......
...@@ -591,8 +591,10 @@ FORM is the parent form that binds this var." ...@@ -591,8 +591,10 @@ FORM is the parent form that binds this var."
(eq ?_ (aref (symbol-name var) 0)) (eq ?_ (aref (symbol-name var) 0))
;; As a special exception, ignore "ignore". ;; As a special exception, ignore "ignore".
(eq var 'ignored)) (eq var 'ignored))
(byte-compile-warn "Unused lexical %s `%S'" (let ((suggestions (help-uni-confusable-suggestions (symbol-name var))))
varkind var))) (byte-compile-warn "Unused lexical %s `%S'%s"
varkind var
(if suggestions (concat "\n " suggestions) "")))))
;; If it's unused, there's no point converting it into a cons-cell, even if ;; If it's unused, there's no point converting it into a cons-cell, even if
;; it's captured and mutated. ;; it's captured and mutated.
(`(,binder ,_ t t ,_) (`(,binder ,_ t t ,_)
......
...@@ -280,6 +280,19 @@ This will generate compile-time constants from BINDINGS." ...@@ -280,6 +280,19 @@ This will generate compile-time constants from BINDINGS."
`(face ,font-lock-warning-face `(face ,font-lock-warning-face
help-echo "This \\ has no effect")))) help-echo "This \\ has no effect"))))
(defun lisp--match-confusable-symbol-character (limit)
;; Match a confusable character within a Lisp symbol.
(catch 'matched
(while t
(if (re-search-forward uni-confusables-regexp limit t)
;; Skip confusables which are backslash escaped, or inside
;; strings or comments.
(save-match-data
(unless (or (eq (char-before (match-beginning 0)) ?\\)
(nth 8 (syntax-ppss)))
(throw 'matched t)))
(throw 'matched nil)))))
(let-when-compile (let-when-compile
((lisp-fdefs '("defmacro" "defun")) ((lisp-fdefs '("defmacro" "defun"))
(lisp-vdefs '("defvar")) (lisp-vdefs '("defvar"))
...@@ -463,7 +476,10 @@ This will generate compile-time constants from BINDINGS." ...@@ -463,7 +476,10 @@ This will generate compile-time constants from BINDINGS."
(3 'font-lock-regexp-grouping-construct prepend)) (3 'font-lock-regexp-grouping-construct prepend))
(lisp--match-hidden-arg (lisp--match-hidden-arg
(0 '(face font-lock-warning-face (0 '(face font-lock-warning-face
help-echo "Hidden behind deeper element; move to another line?"))) help-echo "Hidden behind deeper element; move to another line?")))
(lisp--match-confusable-symbol-character
0 '(face font-lock-warning-face
help-echo "Confusable character"))
)) ))
"Gaudy level highlighting for Emacs Lisp mode.") "Gaudy level highlighting for Emacs Lisp mode.")
......
...@@ -1507,6 +1507,55 @@ the same names as used in the original source code, when possible." ...@@ -1507,6 +1507,55 @@ the same names as used in the original source code, when possible."
(let ((print-escape-newlines t)) (let ((print-escape-newlines t))
(help--docstring-quote (format "%S" (help--make-usage fn arglist))))) (help--docstring-quote (format "%S" (help--make-usage fn arglist)))))
;; Just some quote-like characters for now. TODO: generate this stuff
;; from official Unicode data.
(defconst uni-confusables
'((#x2018 . "'") ;; LEFT SINGLE QUOTATION MARK
(#x2019 . "'") ;; RIGHT SINGLE QUOTATION MARK
(#x201B . "'") ;; SINGLE HIGH-REVERSED-9 QUOTATION MARK
(#x201C . "\"") ;; LEFT DOUBLE QUOTATION MARK
(#x201D . "\"") ;; RIGHT DOUBLE QUOTATION MARK
(#x201F . "\"") ;; DOUBLE HIGH-REVERSED-9 QUOTATION MARK
(#x301E . "\"") ;; DOUBLE PRIME QUOTATION MARK
(#xFF02 . "'") ;; FULLWIDTH QUOTATION MARK
(#xFF07 . "'") ;; FULLWIDTH APOSTROPHE
))
(defconst uni-confusables-regexp
(concat "[" (mapcar #'car uni-confusables) "]"))
(defun help-uni-confusable-suggestions (string)
"Return a message describing confusables in STRING."
(let ((i 0)
(confusables nil))
(while (setq i (string-match uni-confusables-regexp string i))
(let ((replacement (alist-get (aref string i) uni-confusables)))
(push (aref string i) confusables)
(setq string (replace-match replacement t t string))
(setq i (+ i (length replacement)))))
(when confusables
(format-message
(if (> (length confusables) 1)
"Found confusable characters: %s; perhaps you meant: `%s'?"
"Found confusable character: %s, perhaps you meant: `%s'?")
(mapconcat (lambda (c) (format-message "`%c'" c))
confusables ", ")
string))))
(defun help-command-error-confusable-suggestions (data _context _signal)
(pcase data
(`(void-variable ,var)
(let ((suggestions (help-uni-confusable-suggestions
(symbol-name var))))
(when suggestions
(princ (concat "\n " suggestions) t))))
(_ nil)))
(add-function :after command-error-function
#'help-command-error-confusable-suggestions)
(provide 'help) (provide 'help)
......
...@@ -20,6 +20,10 @@ ...@@ -20,6 +20,10 @@
(require 'ert) (require 'ert)
(require 'cl-lib) (require 'cl-lib)
(require 'lisp-mode) (require 'lisp-mode)
(require 'faceup)
;;; Indentation
(defconst lisp-mode-tests--correctly-indented-sexp "\ (defconst lisp-mode-tests--correctly-indented-sexp "\
\(a \(a
...@@ -290,5 +294,27 @@ Expected initialization file: `%s'\" ...@@ -290,5 +294,27 @@ Expected initialization file: `%s'\"
(insert "\"\n") (insert "\"\n")
(lisp-indent-region (point-min) (point-max)))) (lisp-indent-region (point-min) (point-max))))
;;; Fontification
(ert-deftest lisp-fontify-confusables ()
"Unescaped 'smart quotes' should be fontified in `font-lock-warning-face'."
(with-temp-buffer
(dolist (ch
'(#x2018 ;; LEFT SINGLE QUOTATION MARK
#x2019 ;; RIGHT SINGLE QUOTATION MARK
#x201B ;; SINGLE HIGH-REVERSED-9 QUOTATION MARK
#x201C ;; LEFT DOUBLE QUOTATION MARK
#x201D ;; RIGHT DOUBLE QUOTATION MARK
#x201F ;; DOUBLE HIGH-REVERSED-9 QUOTATION MARK
#x301E ;; DOUBLE PRIME QUOTATION MARK
#xFF02 ;; FULLWIDTH QUOTATION MARK
#xFF07 ;; FULLWIDTH APOSTROPHE
))
(insert (format "«w:%c»foo \\%cfoo\n" ch ch)))
(let ((faceup (buffer-string)))
(faceup-clean-buffer)
(should (faceup-test-font-lock-buffer 'emacs-lisp-mode faceup)))))
(provide 'lisp-mode-tests) (provide 'lisp-mode-tests)
;;; lisp-mode-tests.el ends here ;;; lisp-mode-tests.el ends here
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment