bug#31676: 27.0.50; More helpful error message for unescaped character literals

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 522 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> schrieb am Fr., 1. Juni 2018 um
12:19 Uhr:

>
> A reddit user noted these messages popping up when starting Emacs
>
> Loading ‘~/.emacs.d/init’: unescaped character literals `?[', `?]'
> detected!
>
> and wondered what to do about it.  Perhaps it would be a good idea to
> just say something like
>
> unescaped character literals `?[', `?]' detected, `?\[', `?\]' expected
>
> or something along those lines?  For Emacs 26.2.
>
>
Sounds reasonable, here's a patch.

[-- Attachment #1.2: Type: text/html, Size: 842 bytes --]

[-- Attachment #2: 0001-Make-warning-about-unescaped-character-literals-more-h.txt --]
[-- Type: text/plain, Size: 7055 bytes --]

From 4bd8348753980be95dc4bcba47e52f7f79255fb6 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 2 Jun 2018 11:59:02 +0200
Subject: [PATCH] Make warning about unescaped character literals more helpful.

See Bug#31676.

* src/lread.c (Flread_unescaped_character_literals_warning): New
defun.
(load_warn_unescaped_character_literals): Use it.
(syms_of_lread): Define new defun.  Unintern internal
variable, which is not used any more outside of lread.c

* lisp/emacs-lisp/bytecomp.el (byte-compile-from-buffer): Use new
defun.

* test/src/lread-tests.el (lread-tests--unescaped-char-literals):
test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-tests--unescaped-char-literals): Adapt unit tests.
---
 lisp/emacs-lisp/bytecomp.el            | 11 ++------
 src/lread.c                            | 38 ++++++++++++++++++++------
 test/lisp/emacs-lisp/bytecomp-tests.el |  6 ++--
 test/src/lread-tests.el                |  4 ++-
 4 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index ad6b5b7ce2..15f2e75786 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2062,14 +2062,9 @@ byte-compile-from-buffer
 		 (not (eobp)))
 	  (setq byte-compile-read-position (point)
 		byte-compile-last-position byte-compile-read-position)
-	  (let* ((lread--unescaped-character-literals nil)
-                 (form (read inbuffer)))
-            (when lread--unescaped-character-literals
-              (byte-compile-warn
-               "unescaped character literals %s detected!"
-               (mapconcat (lambda (char) (format "`?%c'" char))
-                          (sort lread--unescaped-character-literals #'<)
-                          ", ")))
+	  (let ((form (read inbuffer))
+                (warning (lread--unescaped-character-literals-warning)))
+            (when warning (byte-compile-warn "%s" warning))
 	    (byte-compile-toplevel-file-form form)))
 	;; Compile pending forms at end of file.
 	(byte-compile-flush-pending)
diff --git a/src/lread.c b/src/lread.c
index 239c66ccb8..435e58ed8d 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1022,18 +1022,36 @@ load_error_old_style_backquotes (void)
 static void
 load_warn_unescaped_character_literals (Lisp_Object file)
 {
-  if (NILP (Vlread_unescaped_character_literals)) return;
+  Lisp_Object warning = Flread_unescaped_character_literals_warning ();
+  if (NILP (warning)) return;
+  Lisp_Object format = build_string ("Loading `%s': %s");
+  CALLN (Fmessage, format, file, warning);
+}
+
+DEFUN ("lread--unescaped-character-literals-warning",
+       Flread_unescaped_character_literals_warning,
+       Slread_unescaped_character_literals_warning, 0, 0, 0,
+       doc: /* Return a warning about unescaped character literals.
+If there were any unescaped character literals in the last form read,
+return an appropriate warning message as a string.  Otherwise, return
+nil.  For internal use only.  */)
+     (void)
+{
+  if (NILP (Vlread_unescaped_character_literals)) return Qnil;
   CHECK_CONS (Vlread_unescaped_character_literals);
   Lisp_Object format =
-    build_string ("Loading `%s': unescaped character literals %s detected!");
+    build_string ("unescaped character literals %s detected, %s expected!");
   Lisp_Object separator = build_string (", ");
-  Lisp_Object inner_format = build_string ("`?%c'");
-  CALLN (Fmessage,
-         format, file,
-         Fmapconcat (list3 (Qlambda, list1 (Qchar),
-                            list3 (Qformat, inner_format, Qchar)),
-                     Fsort (Vlread_unescaped_character_literals, Qlss),
-                     separator));
+  Lisp_Object format_unescaped = build_string ("`?%c'");
+  Lisp_Object format_escaped = build_string ("`?\\%c'");
+  Lisp_Object sorted = Fsort (Vlread_unescaped_character_literals, Qlss);
+  return CALLN (Fformat_message, format,
+                Fmapconcat (list3 (Qlambda, list1 (Qchar),
+                                   list3 (Qformat, format_unescaped, Qchar)),
+                            sorted, separator),
+                Fmapconcat (list3 (Qlambda, list1 (Qchar),
+                                   list3 (Qformat, format_escaped, Qchar)),
+                            sorted, separator));
 }
 
 DEFUN ("get-load-suffixes", Fget_load_suffixes, Sget_load_suffixes, 0, 0, 0,
@@ -4784,6 +4802,7 @@ syms_of_lread (void)
   defsubr (&Sread);
   defsubr (&Sread_from_string);
   defsubr (&Slread__substitute_object_in_subtree);
+  defsubr (&Slread_unescaped_character_literals_warning);
   defsubr (&Sintern);
   defsubr (&Sintern_soft);
   defsubr (&Sunintern);
@@ -5048,6 +5067,7 @@ For internal use only.  */);
   Vlread_unescaped_character_literals = Qnil;
   DEFSYM (Qlread_unescaped_character_literals,
           "lread--unescaped-character-literals");
+  Funintern (Qlread_unescaped_character_literals, Qnil);
 
   DEFSYM (Qlss, "<");
   DEFSYM (Qchar, "char");
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 7c5aa9abed..e7e029ec5f 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -522,7 +522,7 @@ bytecomp-tests--with-temp-file
 (ert-deftest bytecomp-tests--unescaped-char-literals ()
   "Check that byte compiling warns about unescaped character
 literals (Bug#20852)."
-  (should (boundp 'lread--unescaped-character-literals))
+  (should-not (boundp 'lread--unescaped-character-literals))
   (bytecomp-tests--with-temp-file source
     (write-region "(list ?) ?( ?; ?\" ?[ ?])" nil source)
     (bytecomp-tests--with-temp-file destination
@@ -533,7 +533,9 @@ bytecomp-tests--with-temp-file
         (should (equal (cdr err)
                        (list (concat "unescaped character literals "
                                      "`?\"', `?(', `?)', `?;', `?[', `?]' "
-                                     "detected!"))))))))
+                                     "detected, "
+                                     "`?\\\"', `?\\(', `?\\)', `?\\;', `?\\[', "
+                                     "`?\\]' expected!"))))))))
 
 (ert-deftest bytecomp-tests--old-style-backquotes ()
   "Check that byte compiling warns about old-style backquotes."
diff --git a/test/src/lread-tests.el b/test/src/lread-tests.el
index 639a6da93a..50ab084dbb 100644
--- a/test/src/lread-tests.el
+++ b/test/src/lread-tests.el
@@ -140,7 +140,9 @@ lread-tests--last-message
     (should (equal (lread-tests--last-message)
                    (concat (format-message "Loading `%s': " file-name)
                            "unescaped character literals "
-                           "`?\"', `?(', `?)', `?;', `?[', `?]' detected!")))))
+                           "`?\"', `?(', `?)', `?;', `?[', `?]' detected, "
+                           "`?\\\"', `?\\(', `?\\)', `?\\;', `?\\[', `?\\]' "
+                           "expected!")))))
 
 (ert-deftest lread-tests--funny-quote-symbols ()
   "Check that 'smart quotes' or similar trigger errors in symbol names."
-- 
2.17.1