bug#25529: diagnosis and one approach to a fix

[Tom Tromey <tom@tromey.com>]

>> I think I understand combining the regexps.  But what does using a simple
>> regexp for the closer mean?

Dmitry> Simple handler (not regexp), for the group corresponding to the closer
Dmitry> (what's called a HIGHLIGHTn in syntax-propertize-rules docstring). The
Dmitry> string "\"/", probably.

Ok, I looked at this a bit.  My idea was to change js-syntax-propertize
to incorporate the new regexp I wrote.

However, after looking at this a bit, I think it's actually better the
way it is.  The reason is that the existing regexp in
js-syntax-propertize ends with "[^/*]" -- to prevent recognizing
"let x = /* comment */" as if it were using a regexp literal.

While it's possible to add this to the new regexp, it's kind of ugly,
since the regexp has to duplicate much of the body of the regexp just to
exclude "*" as the first character.

Conversely I don't think there's a drawback to the current approach.

Here's the latest version of my patch.  Let me know what you think.

Tom

commit 2518a9ca4ab59dbbfedc42a023267ae78476fc2e
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Feb 5 11:40:18 2017 -0700

    Recognize JS regexp literals more correctly
    
    * lisp/progmodes/js.el (js--syntax-propertize-regexp-regexp): New
    constant.
    (js-syntax-propertize-regexp): Use it.  Remove "end" argument.
    (js--syntax-propertize-regexp-syntax-table): Remove.
    (js-syntax-propertize): Update.
    * test/lisp/progmodes/js-tests.el (js-mode-regexp-syntax-bug-25529):
    New test.

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e42e014..3d9c1b4 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1687,34 +1687,39 @@ js--font-lock-keywords
                                    js--font-lock-keywords-3)
   "Font lock keywords for `js-mode'.  See `font-lock-keywords'.")
 
-(defconst js--syntax-propertize-regexp-syntax-table
-  (let ((st (make-char-table 'syntax-table (string-to-syntax "."))))
-    (modify-syntax-entry ?\[ "(]" st)
-    (modify-syntax-entry ?\] ")[" st)
-    (modify-syntax-entry ?\\ "\\" st)
-    st))
-
-(defun js-syntax-propertize-regexp (end)
+(defconst js--syntax-propertize-regexp-regexp
+  (rx
+   ;; Start of regexp.
+   "/"
+   (0+ (or
+        ;; Match characters outside of a character class.
+        (not (any ?\[ ?/ ?\\))
+        ;; Match backslash quoted characters.
+        (and "\\" not-newline)
+        ;; Match character class.
+        (and
+         "["
+         (0+ (or
+              (not (any ?\] ?\\))
+              (and "\\" not-newline)))
+         "]")))
+   (group "/"))
+  "Regular expression matching a JavaScript regexp literal.")
+
+(defun js-syntax-propertize-regexp ()
   (let ((ppss (syntax-ppss)))
     (when (eq (nth 3 ppss) ?/)
       ;; A /.../ regexp.
-      (while
-          (when (re-search-forward "\\(?:\\=\\|[^\\]\\)\\(?:\\\\\\\\\\)*/"
-                                   end 'move)
-            (if (nth 1 (with-syntax-table
-                           js--syntax-propertize-regexp-syntax-table
-                         (let ((parse-sexp-lookup-properties nil))
-                           (parse-partial-sexp (nth 8 ppss) (point)))))
-                ;; A / within a character class is not the end of a regexp.
-                t
-              (put-text-property (1- (point)) (point)
-                                 'syntax-table (string-to-syntax "\"/"))
-              nil))))))
+      (goto-char (nth 8 ppss))
+      (when (looking-at js--syntax-propertize-regexp-regexp)
+        (put-text-property (match-beginning 1) (match-end 1)
+                           'syntax-table (string-to-syntax "\"/"))
+        (goto-char (match-end 0))))))
 
 (defun js-syntax-propertize (start end)
   ;; JavaScript allows immediate regular expression objects, written /.../.
   (goto-char start)
-  (js-syntax-propertize-regexp end)
+  (js-syntax-propertize-regexp)
   (funcall
    (syntax-propertize-rules
     ;; Distinguish /-division from /-regexp chars (and from /-comment-starter).
@@ -1736,7 +1741,7 @@ js-syntax-propertize
                            (eval-when-compile (append "=({[,:;" '(nil))))))
            (put-text-property (match-beginning 1) (match-end 1)
                               'syntax-table (string-to-syntax "\"/"))
-           (js-syntax-propertize-regexp end)))))
+           (js-syntax-propertize-regexp)))))
     ("\\`\\(#\\)!" (1 "< b")))
    (point) end))
 
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index 7cb737c..d61f084 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -23,6 +23,7 @@
 
 (require 'ert)
 (require 'js)
+(require 'syntax)
 
 (ert-deftest js-mode-fill-bug-19399 ()
   (with-temp-buffer
@@ -99,6 +100,22 @@
     (forward-line)
     (should (looking-at " \\* test"))))
 
+(ert-deftest js-mode-regexp-syntax-bug-25529 ()
+  (dolist (regexp-contents '("[^[]"
+                             "[/]"
+                             ;; A comment with the regexp on the next
+                             ;; line.
+                             "*comment*/\n/regexp"))
+    (with-temp-buffer
+      (js-mode)
+      (insert "let x = /" regexp-contents "/;\n")
+      (save-excursion (insert "something();\n"))
+      ;; The failure mode was that the regexp literal was not
+      ;; recognized, causing the next line to be given string syntax;
+      ;; but check for comment syntax as well to prevent an
+      ;; implementation not recognizing the comment example.
+      (should-not (syntax-ppss-context (syntax-ppss))))))
+
 (provide 'js-tests)
 
 ;;; js-tests.el ends here