From b2790db049da98b541d07bac21ca7d7c220d3be0 Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Sat, 10 Mar 2018 18:12:55 -0500 Subject: [PATCH] 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. --- doc/lispref/modes.texi | 3 +- doc/lispref/objects.texi | 6 ++- etc/NEWS | 5 +++ lisp/emacs-lisp/bytecomp.el | 10 ++++- lisp/emacs-lisp/cconv.el | 6 ++- lisp/emacs-lisp/lisp-mode.el | 18 ++++++++- lisp/help.el | 49 +++++++++++++++++++++++++ test/lisp/emacs-lisp/lisp-mode-tests.el | 26 +++++++++++++ 8 files changed, 116 insertions(+), 7 deletions(-) diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi index 7283930507f..c554ccdf4a0 100644 --- a/doc/lispref/modes.texi +++ b/doc/lispref/modes.texi @@ -3287,7 +3287,8 @@ assigned using the ordering as a guide. @table @code @item 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} in C. diff --git a/doc/lispref/objects.texi b/doc/lispref/objects.texi index d9971f6839e..e948814d1f7 100644 --- a/doc/lispref/objects.texi +++ b/doc/lispref/objects.texi @@ -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 @samp{()[]\;"}, and you should add a backslash before any 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 cleaner to use one of the easily readable escape sequences, such as @samp{\t} or @samp{\s}, instead of an actual whitespace character such diff --git a/etc/NEWS b/etc/NEWS index 3a2efcfc2e0..688b3f85cbb 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -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 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. For example '(defun foo (&optional))' is no longer an error. This is diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 905d99a5971..118356ec26a 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3428,7 +3428,10 @@ for symbols generated by the byte compiler itself." (boundp var) (memq var byte-compile-bound-variables) (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)) (byte-compile-dynamic-variable-op 'byte-varref var)))) @@ -3444,7 +3447,10 @@ for symbols generated by the byte compiler itself." (boundp var) (memq var byte-compile-bound-variables) (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)) (byte-compile-dynamic-variable-op 'byte-varset var)))) diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el index 58ca9d5f57e..09af7cb9104 100644 --- a/lisp/emacs-lisp/cconv.el +++ b/lisp/emacs-lisp/cconv.el @@ -591,8 +591,10 @@ FORM is the parent form that binds this var." (eq ?_ (aref (symbol-name var) 0)) ;; As a special exception, ignore "ignore". (eq var 'ignored)) - (byte-compile-warn "Unused lexical %s `%S'" - varkind var))) + (let ((suggestions (help-uni-confusable-suggestions (symbol-name 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 ;; it's captured and mutated. (`(,binder ,_ t t ,_) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index 5df52ebc98f..56f8ef63682 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -280,6 +280,19 @@ This will generate compile-time constants from BINDINGS." `(face ,font-lock-warning-face 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 ((lisp-fdefs '("defmacro" "defun")) (lisp-vdefs '("defvar")) @@ -463,7 +476,10 @@ This will generate compile-time constants from BINDINGS." (3 'font-lock-regexp-grouping-construct prepend)) (lisp--match-hidden-arg (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.") diff --git a/lisp/help.el b/lisp/help.el index 604a365957c..8bb942abf4f 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -1507,6 +1507,55 @@ the same names as used in the original source code, when possible." (let ((print-escape-newlines t)) (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) diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el index e4ba929ecbd..c0dd68c0a0b 100644 --- a/test/lisp/emacs-lisp/lisp-mode-tests.el +++ b/test/lisp/emacs-lisp/lisp-mode-tests.el @@ -20,6 +20,10 @@ (require 'ert) (require 'cl-lib) (require 'lisp-mode) +(require 'faceup) + + +;;; Indentation (defconst lisp-mode-tests--correctly-indented-sexp "\ \(a @@ -290,5 +294,27 @@ Expected initialization file: `%s'\" (insert "\"\n") (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) ;;; lisp-mode-tests.el ends here -- 2.39.2