Errors indicated by ineffective backslashes in string literals

Errors indicated by ineffective backslashes in string literals

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]

Sorry to bother you again, but while trying to get a better understanding of regexp usage and errors I found several ineffective backslashes in string literals (like "\.") and decided to do something about them.

Some were just superfluous and confusing, but others were apparent errors. Here is the resulting patch.


[-- Attachment #2: 0001-Fix-some-ineffective-backslashes-in-string-literals.patch --]
[-- Type: application/octet-stream, Size: 10943 bytes --]

From 3928725db206290ebc7df8a4eaabd56dbc42fee9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sat, 16 Mar 2019 18:19:23 +0100
Subject: [PATCH] Fix some ineffective backslashes in string literals

Deal with lone backslashes that have no effect in string literals,
but indicate that something is amiss.

* lisp/auth-source-pass.el (auth-source-pass-entries):
* lisp/textmodes/artist.el (artist-figlet-get-font-list-windows):
* lisp/org/ob-abc.el (org-babel-expand-body:abc, org-babel-execute:abc):
* lisp/org/ob-forth.el (org-babel-forth-session-execute):
* lisp/vc/vc-git.el (vc-git--program-version):
Add backslash in regexp for correctness.

* lisp/gnus/nnmail.el (nnmail-split-abbrev-alist):
Replace `\||' with `\\|' to follow the obvious regexp intent.

* lisp/org/org-list.el (org-plain-list-ordered-item-terminator):
Add backslash in doc comment so that it appears as intended.

* lisp/progmodes/cc-engine.el (c-forward-decl-or-cast-1, c-end-of-decl-1):
* lisp/progmodes/f90.el (f90-font-lock-keywords-2):
* lisp/progmodes/etags.el (etags-tags-completion-table):
* lisp/progmodes/ruby-mode.el (ruby-syntax-propertize):
* test/lisp/emacs-lisp/cl-print-tests.el (cl-print-tests-1):
Remove superfluous backslashes from regexp.

* test/lisp/emacs-lisp/rx-tests.el (rx-char-any):
Remove superfluous backslash from doc comment.
---
 lisp/auth-source-pass.el               | 2 +-
 lisp/gnus/nnmail.el                    | 2 +-
 lisp/org/ob-abc.el                     | 4 ++--
 lisp/org/ob-forth.el                   | 2 +-
 lisp/org/org-list.el                   | 2 +-
 lisp/progmodes/cc-engine.el            | 6 +++---
 lisp/progmodes/etags.el                | 2 +-
 lisp/progmodes/f90.el                  | 2 +-
 lisp/progmodes/ruby-mode.el            | 2 +-
 lisp/textmodes/artist.el               | 2 +-
 lisp/vc/vc-git.el                      | 2 +-
 test/lisp/emacs-lisp/cl-print-tests.el | 2 +-
 test/lisp/emacs-lisp/rx-tests.el       | 2 +-
 13 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lisp/auth-source-pass.el b/lisp/auth-source-pass.el
index e37cadb07c..deb805a6e1 100644
--- a/lisp/auth-source-pass.el
+++ b/lisp/auth-source-pass.el
@@ -189,7 +189,7 @@ often."
   (let ((store-dir (expand-file-name "~/.password-store/")))
     (mapcar
      (lambda (file) (file-name-sans-extension (file-relative-name file store-dir)))
-     (directory-files-recursively store-dir "\.gpg$"))))
+     (directory-files-recursively store-dir "\\.gpg$"))))
 
 (defun auth-source-pass--find-all-by-entry-name (entryname user)
   "Search the store for all entries either matching ENTRYNAME/USER or ENTRYNAME.
diff --git a/lisp/gnus/nnmail.el b/lisp/gnus/nnmail.el
index f6d7525293..a95cdb4a4f 100644
--- a/lisp/gnus/nnmail.el
+++ b/lisp/gnus/nnmail.el
@@ -489,7 +489,7 @@ Example:
     (from . "from\\|sender\\|resent-from")
     (nato . "to\\|cc\\|resent-to\\|resent-cc")
     (naany . "from\\|to\\|cc\\|sender\\|resent-from\\|resent-to\\|resent-cc")
-    (list . "list-id\\|list-post\\|x-mailing-list\||x-beenthere\\|x-loop"))
+    (list . "list-id\\|list-post\\|x-mailing-list\\|x-beenthere\\|x-loop"))
   "Alist of abbreviations allowed in `nnmail-split-fancy'."
   :group 'nnmail-split
   :type '(repeat (cons :format "%v" symbol regexp)))
diff --git a/lisp/org/ob-abc.el b/lisp/org/ob-abc.el
index cefbe716e1..43ee1d9921 100644
--- a/lisp/org/ob-abc.el
+++ b/lisp/org/ob-abc.el
@@ -47,7 +47,7 @@
 	     (value (cdr pair)))
 	 (setq body
 	       (replace-regexp-in-string
-		(concat "\$" (regexp-quote name))
+		(concat "\\$" (regexp-quote name))
 		(if (stringp value) value (format "%S" value))
 		body))))
      vars)
@@ -59,7 +59,7 @@
   (message "executing Abc source code block")
   (let* ((cmdline (cdr (assq :cmdline params)))
 	 (out-file (let ((file (cdr (assq :file params))))
-		     (if file (replace-regexp-in-string "\.pdf$" ".ps" file)
+		     (if file (replace-regexp-in-string "\\.pdf$" ".ps" file)
 		       (error "abc code block requires :file header argument"))))
 	 (in-file (org-babel-temp-file "abc-"))
 	 (render (concat "abcm2ps" " " cmdline
diff --git a/lisp/org/ob-forth.el b/lisp/org/ob-forth.el
index 8ca292656a..88ed964fd7 100644
--- a/lisp/org/ob-forth.el
+++ b/lisp/org/ob-forth.el
@@ -53,7 +53,7 @@ This function is called by `org-babel-execute-src-block'"
 (defun org-babel-forth-session-execute (body params)
   (require 'forth-mode)
   (let ((proc (forth-proc))
-	(rx " \\(\n:\\|compiled\n\\\|ok\n\\)")
+	(rx " \\(\n:\\|compiled\n\\|ok\n\\)")
 	(result-start))
     (with-current-buffer (process-buffer (forth-proc))
       (mapcar (lambda (line)
diff --git a/lisp/org/org-list.el b/lisp/org/org-list.el
index ef85b402f0..22692d224a 100644
--- a/lisp/org/org-list.el
+++ b/lisp/org/org-list.el
@@ -236,7 +236,7 @@ into
 
 (defcustom org-plain-list-ordered-item-terminator t
   "The character that makes a line with leading number an ordered list item.
-Valid values are ?. and ?\).  To get both terminators, use t.
+Valid values are ?. and ?\\).  To get both terminators, use t.
 
 This variable needs to be set before org.el is loaded.  If you
 need to make a change while Emacs is running, use the customize
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index fd66928099..cc3753a7eb 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -8968,7 +8968,7 @@ This function might do hidden buffer changes."
 
 	 (setq at-decl-end
 	       (looking-at (cond ((eq context '<>) "[,>]")
-				 ((not (memq context '(nil top))) "[,\)]")
+				 ((not (memq context '(nil top))) "[,)]")
 				 (t "[,;]"))))
 
 	 ;; Now we've collected info about various characteristics of
@@ -10321,7 +10321,7 @@ comment at the start of cc-engine.el for more info."
 			  ;; Check for `c-opt-block-decls-with-vars-key'
 			  ;; before the first paren.
 			  (c-syntactic-re-search-forward
-			   (concat "[;=\(\[{]\\|\\("
+			   (concat "[;=([{]\\|\\("
 				   c-opt-block-decls-with-vars-key
 				   "\\)")
 			   lim t t t)
@@ -10329,7 +10329,7 @@ comment at the start of cc-engine.el for more info."
 			  (not (eq (char-before) ?_))
 			  ;; Check that the first following paren is
 			  ;; the block.
-			  (c-syntactic-re-search-forward "[;=\(\[{]"
+			  (c-syntactic-re-search-forward "[;=([{]"
 							 lim t t t)
 			  (eq (char-before) ?{))))))
 	    ;; The declaration doesn't have any of the
diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index c2715be537..910c320ab8 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -1281,7 +1281,7 @@ buffer-local values of tags table format variables."
       ;; This regexp matches an explicit tag name or the place where
       ;; it would start.
       (while (re-search-forward
-              "[\f\t\n\r()=,; ]?\177\\\(?:\\([^\n\001]+\\)\001\\)?"
+              "[\f\t\n\r()=,; ]?\177\\(?:\\([^\n\001]+\\)\001\\)?"
 	      nil t)
 	(push	(prog1 (if (match-beginning 1)
 			   ;; There is an explicit tag name.
diff --git a/lisp/progmodes/f90.el b/lisp/progmodes/f90.el
index 3ec145b547..9de80635e9 100644
--- a/lisp/progmodes/f90.el
+++ b/lisp/progmodes/f90.el
@@ -648,7 +648,7 @@ forall\\|block\\|critical\\)\\)\\_>"
 \\|enumerator\\|procedure\\|\
 logical\\|double[ \t]*precision\\|type[ \t]*(\\(?:\\sw\\|\\s_\\)+)\\|none\\)[ \t]*"
       (1 font-lock-keyword-face) (2 font-lock-type-face))
-    '("\\_<\\(namelist\\|common\\)[ \t]*/\\(\\(?:\\sw\\|\\s_\\)+\\)?\/"
+    '("\\_<\\(namelist\\|common\\)[ \t]*/\\(\\(?:\\sw\\|\\s_\\)+\\)?/"
       (1 font-lock-keyword-face) (2 font-lock-constant-face nil t))
     "\\_<else\\([ \t]*if\\|where\\)?\\_>"
     '("\\(&\\)[ \t]*\\(!\\|$\\)"  (1 font-lock-keyword-face))
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 707875d130..5998ac8e39 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -1867,7 +1867,7 @@ It will be properly highlighted even when the call omits parens.")
       ("^[ \t]*def +\\(`\\)" (1 "_"))
       ;; Ternary operator colon followed by opening paren or bracket
       ;; (semi-important for indentation).
-      ("\\(:\\)\\(?:[\({]\\|\\[[^]]\\)"
+      ("\\(:\\)\\(?:[({]\\|\\[[^]]\\)"
        (1 (string-to-syntax ".")))
       ;; Regular expressions.  Start with matching unescaped slash.
       ("\\(?:\\=\\|[^\\]\\)\\(?:\\\\\\\\\\)*\\(/\\)"
diff --git a/lisp/textmodes/artist.el b/lisp/textmodes/artist.el
index d75a1ca2f9..e9b17795a9 100644
--- a/lisp/textmodes/artist.el
+++ b/lisp/textmodes/artist.el
@@ -2895,7 +2895,7 @@ Returns a list of strings."
        dir-list)
       (mapcar
        (lambda (file)
-         (replace-regexp-in-string "\.flf\\'" "" file))
+         (replace-regexp-in-string "\\.flf\\'" "" file))
        result))))
 
 (defun artist-figlet-choose-font ()
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index c6806ba5cd..c990b0659d 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -253,7 +253,7 @@ The following place holders should be present in the string:
                        ;; Git for Windows appends ".windows.N" to the
                        ;; numerical version reported by Git.
                        (string-match
-                        "git version \\([0-9.]+\\)\\(\.windows.[0-9]+\\)?$"
+                        "git version \\([0-9.]+\\)\\(\\.windows.[0-9]+\\)?$"
                         version-string))
                   (match-string 1 version-string)
                 "0")))))
diff --git a/test/lisp/emacs-lisp/cl-print-tests.el b/test/lisp/emacs-lisp/cl-print-tests.el
index e163fb8a8d..406c528dce 100644
--- a/test/lisp/emacs-lisp/cl-print-tests.el
+++ b/test/lisp/emacs-lisp/cl-print-tests.el
@@ -34,7 +34,7 @@
     (let ((print-circle t))
       (should (equal (cl-prin1-to-string `((x . ,x) (y . ,x)))
                      "((x . #1=#s(cl-print--test :a 1 :b 2)) (y . #1#))")))
-    (should (string-match "\\`#f(compiled-function (x) \"[^\"]+\" [^\)]*)\\'"
+    (should (string-match "\\`#f(compiled-function (x) \"[^\"]+\" [^)]*)\\'"
                           (cl-prin1-to-string (symbol-function #'caar))))))
 
 (ert-deftest cl-print-tests-2 ()
diff --git a/test/lisp/emacs-lisp/rx-tests.el b/test/lisp/emacs-lisp/rx-tests.el
index fa3d9b0d5e..7dd5e3b8de 100644
--- a/test/lisp/emacs-lisp/rx-tests.el
+++ b/test/lisp/emacs-lisp/rx-tests.el
@@ -25,7 +25,7 @@
 ;;; Code:
 
 (ert-deftest rx-char-any ()
-  "Test character alternatives with `\]' and `-' (Bug#25123)."
+  "Test character alternatives with `]' and `-' (Bug#25123)."
   (should (string-match
            (rx string-start (1+ (char (?\] . ?\{) (?< . ?\]) (?- . ?:)))
                string-end)
-- 
2.17.2 (Apple Git-113)

Re: Errors indicated by ineffective backslashes in string literals

[Stefan Monnier]

> Sorry to bother you again, but while trying to get a better understanding of
> regexp usage and errors I found several ineffective backslashes in string
> literals (like "\.") and decided to do something about them.

I went through something similar "recently" and added the corresponding
highlighting in elisp-mode (see elisp--font-lock-backslash in lisp-mode.el).

> -     (directory-files-recursively store-dir "\.gpg$"))))
> +     (directory-files-recursively store-dir "\\.gpg$"))))
                                                      ^
should be \\'

[ Yes, I know it's unrelated, but it's another common (minor) error.  ]

> -		     (if file (replace-regexp-in-string "\.pdf$" ".ps" file)
> +		     (if file (replace-regexp-in-string "\\.pdf$" ".ps" file)
                                                               ^
Here's another ;-)


        Stefan

Re: Errors indicated by ineffective backslashes in string literals

[Mattias Engdegård]

16 mars 2019 kl. 19.25 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
> 
>> Sorry to bother you again, but while trying to get a better understanding of
>> regexp usage and errors I found several ineffective backslashes in string
>> literals (like "\.") and decided to do something about them.
> 
> I went through something similar "recently" and added the corresponding
> highlighting in elisp-mode (see elisp--font-lock-backslash in lisp-mode.el).

Nice, thank you. I wrote a script to search for the help-echo property to locate the errors.

>> -     (directory-files-recursively store-dir "\.gpg$"))))
>> +     (directory-files-recursively store-dir "\\.gpg$"))))
>                                                      ^
> should be \\'
> 
> [ Yes, I know it's unrelated, but it's another common (minor) error.  ]

Ah yes, sorry, should have done that while at it. Now I searched for such cases and found lots, and decided to postpone it since it's mostly cosmetic -- unless we are concerned about newlines in file names, which we perhaps should be.

Re: Errors indicated by ineffective backslashes in string literals

[Stefan Monnier]

>> [ Yes, I know it's unrelated, but it's another common (minor) error.  ]
> Ah yes, sorry, should have done that while at it. Now I searched for such
> cases and found lots, and decided to postpone it since it's mostly
> cosmetic --

It's both cosmetic and tricky (you have to understand enough of the
code to be sure that it's not meant to work on multiline text).

> unless we are concerned about newlines in file names, which we
> perhaps should be.

I don't think there's much reason to worry about newline in files, but
it's nice not to need to decide whether we should worry about something
or not.


        Stefan

Re: Errors indicated by ineffective backslashes in string literals

[Paul Eggert]

On 3/16/19 12:04 PM, Mattias Engdegård wrote:
> Now I searched for such cases and found lots, and decided to postpone it since it's mostly cosmetic -- unless we are concerned about newlines in file names, which we perhaps should be.
>
I'm afraid we should be, since newlines in file names are well-known
attack vector
<https://dwheeler.com/essays/fixing-unix-linux-filenames.html>.

Re: Errors indicated by ineffective backslashes in string literals

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 110 bytes --]

I installed that in your name, along with the attached extra glitch-fix. Thanks 
for doing all this checking!

[-- Attachment #2: 0001-Fix-regexp-typo-in-vc-git-program-version.patch --]
[-- Type: text/x-patch, Size: 1038 bytes --]

From dfa0bb35a546367dc1cbcde15fc2acbbc92712bc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 16 Mar 2019 11:38:36 -0700
Subject: [PATCH] Fix regexp typo in vc-git--program-version

* lisp/vc/vc-git.el (vc-git--program-version):
Require a period after ".windows", instead of allowing any char.
---
 lisp/vc/vc-git.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index c990b0659d..6b8ed7e2c1 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -253,7 +253,7 @@ vc-git--program-version
                        ;; Git for Windows appends ".windows.N" to the
                        ;; numerical version reported by Git.
                        (string-match
-                        "git version \\([0-9.]+\\)\\(\\.windows.[0-9]+\\)?$"
+                        "git version \\([0-9.]+\\)\\(\\.windows\\.[0-9]+\\)?$"
                         version-string))
                   (match-string 1 version-string)
                 "0")))))
-- 
2.17.1

Re: Errors indicated by ineffective backslashes in string literals

[Mattias Engdegård]

16 mars 2019 kl. 19.40 skrev Paul Eggert <eggert@cs.ucla.edu>:
> 
> I installed that in your name, along with the attached extra glitch-fix. Thanks for doing all this checking!

I really should have spotted that one. Thank you!