auto merge of #9015 : MicahChalmer/rust/emacs-fixes-round-2, r=nikomatsakis

Here are fixes for more problems mentioned in #8787.  I think I've addressed everything mentioned there except for @nikomatsakis's comment about match/patterns now.  (This also fixes the bug in struct alignment that @pnkfelix mentioned from my earlier pull request #8872.)

The biggest change here is to make fill-paragraph (M-q) and auto-fill-mode work inside different variations of multi-line and doc comments.  Because of the way emacs paragraph fills work (callbacks interacting with global regexp variables that are used in odd ways) there were quite a few edge cases that I had to work around.

The only way I was able to keep it all straight was to create some regression tests.  They use the emacs lisp regression testing tool ERT, and are included as the last commit here.  I added a few tests for indentation as well.  I have not attempted to integrate the tests into the overall rust compiler build process, since I can't imagine anyone would want the compiler build to have a dependency on emacs.  Maybe at some point tools like this get their own repositories?  Just a thought.

One other thought related to the tests: should there be a place to put these types of style samples that isn't specific to one text editor?  Maybe as part of an official rust style guide, but in a form that would allow tools like this to pull out the samples and use them for tests?

4 files changed, 568 insertions, 35 deletions

diff --git a/src/etc/emacs/README.md b/src/etc/emacs/README.md
index 508ac7f1af2..63fbe1f681c 100644
--- a/src/etc/emacs/README.md
+++ b/src/etc/emacs/README.md
@@ -68,6 +68,12 @@ marking, press x, and ELPA will install the packages for you (under
 
 * or using <kbd>M-x package-install rust-mode
 
+### Tests via ERT
+
+The file `rust-mode-tests.el` contains tests that can be run via ERT.  You can
+use `run_rust_emacs_tests.sh` to run them in batch mode, if emacs is somewhere
+in your `$PATH`.
+
 ### Known bugs
 
 * Combining `global-whitespace-mode` and `rust-mode` is generally glitchy.
diff --git a/src/etc/emacs/run_rust_emacs_tests.sh b/src/etc/emacs/run_rust_emacs_tests.sh
new file mode 100755
index 00000000000..afb98fde7b0
--- /dev/null
+++ b/src/etc/emacs/run_rust_emacs_tests.sh
@@ -0,0 +1,3 @@
+# This runs the test for emacs rust-mode.
+# It must be possible to find emacs via PATH.
+emacs -batch -l rust-mode.el -l rust-mode-tests.el -f ert-run-tests-batch-and-exit
diff --git a/src/etc/emacs/rust-mode-tests.el b/src/etc/emacs/rust-mode-tests.el
new file mode 100644
index 00000000000..4bfdee50c6b
--- /dev/null
+++ b/src/etc/emacs/rust-mode-tests.el
@@ -0,0 +1,397 @@
+;;; rust-mode-tests.el --- ERT tests for rust-mode.el
+
+(require 'rust-mode)
+(require 'ert)
+(require 'cl)
+
+(setq rust-test-fill-column 32)
+
+(defun rust-compare-code-after-manip (original point-pos manip-func expected got)
+  (equal expected got))
+
+(defun rust-test-explain-bad-manip (original point-pos manip-func expected got)
+  (if (equal expected got)
+      nil
+    (list 
+     ;; The (goto-char) and (insert) business here is just for
+     ;; convenience--after an error, you can copy-paste that into emacs eval to
+     ;; insert the bare strings into a buffer
+     "Rust code was manipulated wrong after:"
+     `(insert ,original)
+     `(goto-char ,point-pos)
+     'expected `(insert ,expected)
+     'got `(insert ,got)
+     (loop for i from 0 to (max (length original) (length expected))
+           for oi = (if (< i (length got)) (elt got i))
+           for ei = (if (< i (length expected)) (elt expected i))
+           while (equal oi ei)
+           finally return `(first-difference-at
+                            (goto-char ,(+ 1 i))
+                            expected ,(char-to-string ei)
+                            got ,(char-to-string oi))))))
+(put 'rust-compare-code-after-manip 'ert-explainer
+     'rust-test-explain-bad-manip)
+
+(defun rust-test-manip-code (original point-pos manip-func expected)
+  (with-temp-buffer
+    (rust-mode)
+    (insert original)
+    (goto-char point-pos)
+    (funcall manip-func)
+    (should (rust-compare-code-after-manip
+             original point-pos manip-func expected (buffer-string)))))
+
+(defun test-fill-paragraph (unfilled expected &optional start-pos end-pos)
+  "We're going to run through many scenarios here--the point should be able to be anywhere from the start-pos (defaults to 1) through end-pos (defaults to the length of what was passed in) and (fill-paragraph) should return the same result.
+
+Also, the result should be the same regardless of whether the code is at the beginning or end of the file.  (If you're not careful, that can make a difference.)  So we test each position given above with the passed code at the beginning, the end, neither and both.  So we do this a total of (end-pos - start-pos)*4 times.  Oy."
+  (let* ((start-pos (or start-pos 1))
+         (end-pos (or end-pos (length unfilled)))
+         (padding "\n     \n")
+         (padding-len (length padding)))
+    (loop
+     for pad-at-beginning from 0 to 1
+     do (loop for pad-at-end from 0 to 1
+           with padding-beginning = (if (= 0 pad-at-beginning) "" padding)
+           with padding-end = (if (= 0 pad-at-end) "" padding)
+           with padding-adjust = (* padding-len pad-at-beginning)
+           with padding-beginning = (if (= 0 pad-at-beginning) "" padding)
+           with padding-end = (if (= 0 pad-at-end) "" padding)
+           ;; If we're adding space to the beginning, and our start position
+           ;; is at the very beginning, we want to test within the added space.
+           ;; Otherwise adjust the start and end for the beginning padding.
+           with start-pos = (if (= 1 start-pos) 1 (+ padding-adjust start-pos))
+           with end-pos = (+ end-pos padding-adjust)
+           do (loop for pos from start-pos to end-pos
+                 do (rust-test-manip-code
+                     (concat padding-beginning unfilled padding-end)
+                     pos
+                     (lambda () 
+                       (let ((fill-column rust-test-fill-column))
+                         (fill-paragraph)))
+                     (concat padding-beginning expected padding-end)))))))
+
+(ert-deftest fill-paragraph-top-level-multi-line-style-doc-comment-second-line ()
+  (test-fill-paragraph 
+   "/**
+ * This is a very very very very very very very long string
+ */"
+   "/**
+ * This is a very very very very
+ * very very very long string
+ */"))
+
+
+(ert-deftest fill-paragraph-top-level-multi-line-style-doc-comment-first-line ()
+  (test-fill-paragraph
+   "/** This is a very very very very very very very long string
+ */"
+   "/** This is a very very very
+ * very very very very long
+ * string
+ */"))
+
+(ert-deftest fill-paragraph-multi-paragraph-multi-line-style-doc-comment ()
+  (let
+      ((multi-paragraph-unfilled
+        "/**
+ * This is the first really really really really really really really long paragraph
+ *
+ * This is the second really really really really really really long paragraph
+ */"))
+    (test-fill-paragraph
+     multi-paragraph-unfilled
+     "/**
+ * This is the first really
+ * really really really really
+ * really really long paragraph
+ *
+ * This is the second really really really really really really long paragraph
+ */"
+     1 89)
+    (test-fill-paragraph
+     multi-paragraph-unfilled
+     "/**
+ * This is the first really really really really really really really long paragraph
+ *
+ * This is the second really
+ * really really really really
+ * really long paragraph
+ */"
+     90)))
+
+(ert-deftest fill-paragraph-multi-paragraph-single-line-style-doc-comment ()
+  (let
+      ((multi-paragraph-unfilled
+        "/// This is the first really really really really really really really long paragraph
+///
+/// This is the second really really really really really really long paragraph"))
+    (test-fill-paragraph
+     multi-paragraph-unfilled
+     "/// This is the first really
+/// really really really really
+/// really really long paragraph
+///
+/// This is the second really really really really really really long paragraph"
+     1 86)
+    (test-fill-paragraph
+     multi-paragraph-unfilled
+     "/// This is the first really really really really really really really long paragraph
+///
+/// This is the second really
+/// really really really really
+/// really long paragraph"
+     87)))
+
+(ert-deftest fill-paragraph-multi-paragraph-single-line-style-indented ()
+  (test-fill-paragraph
+   "     // This is the first really really really really really really really long paragraph
+     //
+     // This is the second really really really really really really long paragraph"
+   "     // This is the first really
+     // really really really
+     // really really really
+     // long paragraph
+     //
+     // This is the second really really really really really really long paragraph" 1 89))
+
+(ert-deftest fill-paragraph-multi-line-style-inner-doc-comment ()
+  (test-fill-paragraph
+   "/*! This is a very very very very very very very long string
+ */"
+   "/*! This is a very very very
+ * very very very very long
+ * string
+ */"))
+
+(ert-deftest fill-paragraph-single-line-style-inner-doc-comment ()
+  (test-fill-paragraph
+   "//! This is a very very very very very very very long string"
+   "//! This is a very very very
+//! very very very very long
+//! string"))
+
+(ert-deftest fill-paragraph-prefixless-multi-line-doc-comment ()
+  (test-fill-paragraph
+   "/**
+This is my summary. Blah blah blah blah blah. Dilly dally dilly dally dilly dally doo.
+
+This is some more text.  Fee fie fo fum.  Humpty dumpty sat on a wall.
+*/"
+   "/**
+This is my summary. Blah blah
+blah blah blah. Dilly dally
+dilly dally dilly dally doo.
+
+This is some more text.  Fee fie fo fum.  Humpty dumpty sat on a wall.
+*/" 4 90))
+
+(ert-deftest fill-paragraph-with-no-space-after-star-prefix ()
+  (test-fill-paragraph
+   "/** 
+ *This is a very very very very very very very long string
+ */"
+   "/** 
+ *This is a very very very very
+ *very very very long string
+ */"))
+
+(defun test-auto-fill (initial position inserted expected)
+  (rust-test-manip-code 
+   initial
+   position
+   (lambda ()
+     (unwind-protect
+         (progn
+           (let ((fill-column rust-test-fill-column))
+             (auto-fill-mode)
+             (goto-char position)
+             (insert inserted)
+             (syntax-ppss-flush-cache 1)
+             (funcall auto-fill-function)))
+       (auto-fill-mode t)))
+   expected))
+
+(ert-deftest auto-fill-multi-line-doc-comment ()
+  (test-auto-fill
+   "/**
+ * 
+ */"
+   8
+   "This is a very very very very very very very long string"
+   "/**
+ * This is a very very very very
+ * very very very long string
+ */"))
+
+(ert-deftest auto-fill-single-line-doc-comment ()
+  (test-auto-fill
+   "/// This is the first really
+/// really really really really
+/// really really long paragraph
+///
+/// "
+   103
+   "This is the second really really really really really really long paragraph"
+    "/// This is the first really
+/// really really really really
+/// really really long paragraph
+///
+/// This is the second really
+/// really really really really
+/// really long paragraph"
+    ))
+
+(ert-deftest auto-fill-multi-line-prefixless ()
+  (test-auto-fill
+   "/*
+
+ */"
+   4
+   "This is a very very very very very very very long string"
+   "/*
+This is a very very very very
+very very very long string
+ */"
+   ))
+
+(defun test-indent (indented)
+  (let ((deindented (replace-regexp-in-string "^[[:blank:]]*" "      " indented)))
+    (rust-test-manip-code
+     deindented
+     1
+     (lambda () (indent-region 1 (buffer-size)))
+     indented)))
+
+
+(ert-deftest indent-struct-fields-aligned ()
+  (test-indent
+"
+struct Foo { bar: int,
+             baz: int }
+
+struct Blah {x:int,
+             y:int,
+             z:~str}"))
+
+(ert-deftest indent-doc-comments ()
+  (test-indent
+"
+/**
+ * This is a doc comment
+ *
+ */
+
+/// So is this
+
+fn foo() {
+    /*!
+     * this is a nested doc comment
+     */
+    
+    //! And so is this
+}"))
+
+(ert-deftest indent-inside-braces ()
+  (test-indent
+   "
+// struct fields out one level:
+struct foo {
+    a:int,
+    // comments too
+    b:char
+}
+
+fn bar(x:~int) {   // comment here should not affect the next indent
+    bla();
+    bla();
+}"))
+
+(ert-deftest indent-top-level ()
+  (test-indent
+   "
+// Everything here is at the top level and should not be indented
+#[attrib]
+mod foo;
+
+pub static bar = Quux{a: b()}
+
+use foo::bar::baz;
+
+fn foo() { }
+"))
+
+(ert-deftest indent-params-no-align ()
+  (test-indent
+   "
+// Indent out one level because no params appear on the first line
+fn xyzzy(
+    a:int,
+    b:char) { }
+
+fn abcdef(
+    a:int,
+    b:char)
+    -> char
+{ }"))
+
+(ert-deftest indent-params-align ()
+  (test-indent
+   "
+// Align the second line of params to the first
+fn foo(a:int,
+       b:char) { }
+
+fn bar(   a:int,
+          b:char)
+          -> int
+{ }
+
+fn baz(   a:int,  // shoudl work with a comment here
+          b:char)
+          -> int
+{ }
+"))
+
+(ert-deftest indent-square-bracket-alignment ()
+  (test-indent
+   "
+fn args_on_the_next_line( // with a comment
+    a:int,
+    b:~str) {
+    let aaaaaa = [
+        1,
+        2,
+        3];
+    let bbbbbbb = [1, 2, 3,
+                   4, 5, 6];
+    let ccc = [   10, 9, 8,
+                  7, 6, 5];
+}
+"))
+
+(ert-deftest indent-nested-fns ()
+  (test-indent
+   "
+fn nexted_fns(a: fn(b:int,
+                    c:char)
+                    -> int,
+              d: int)
+              -> uint
+{
+    0
+}
+"
+))
+
+(ert-deftest indent-multi-line-expr ()
+  (test-indent
+   "
+fn foo()
+{
+    x();
+    let a =
+        b();
+}
+"
+))
diff --git a/src/etc/emacs/rust-mode.el b/src/etc/emacs/rust-mode.el
index 92b85247e48..a61d916f46f 100644
--- a/src/etc/emacs/rust-mode.el
+++ b/src/etc/emacs/rust-mode.el
@@ -30,8 +30,11 @@
 
     table))
 
+(defgroup rust-mode nil "Support for Rust code.")
+
 (defcustom rust-indent-offset 4
-  "*Indent Rust code by this number of spaces.")
+  "*Indent Rust code by this number of spaces."
+  :group 'rust-mode)
 
 (defun rust-paren-level () (nth 0 (syntax-ppss)))
 (defun rust-in-str-or-cmnt () (nth 8 (syntax-ppss)))
@@ -45,6 +48,15 @@
     (if (/= starting (point))
         (rust-rewind-irrelevant))))
 
+(defun rust-align-to-expr-after-brace ()
+  (save-excursion
+    (forward-char)
+    ;; We don't want to indent out to the open bracket if the
+    ;; open bracket ends the line
+    (when (not (looking-at "[[:blank:]]*\\(?://.*\\)?$"))
+      (when (looking-at "[[:space:]]") (forward-to-word 1))
+      (current-column))))
+
 (defun rust-mode-indent-line ()
   (interactive)
   (let ((indent
@@ -52,13 +64,17 @@
            (back-to-indentation)
            (let ((level (rust-paren-level)))
              (cond
-              ;; A function return type is 1 level indented
-              ((looking-at "->") (* rust-indent-offset (+ level 1)))
+              ;; A function return type is indented to the corresponding function arguments
+              ((looking-at "->")
+               (save-excursion
+                 (backward-list)
+                 (or (rust-align-to-expr-after-brace)
+                     (* rust-indent-offset (+ 1 level)))))
 
               ;; A closing brace is 1 level unindended
               ((looking-at "}") (* rust-indent-offset (- level 1)))
 
-              ; Doc comments in /** style with leading * indent to line up the *s
+              ;; Doc comments in /** style with leading * indent to line up the *s
               ((and (nth 4 (syntax-ppss)) (looking-at "*"))
                (+ 1 (* rust-indent-offset level)))
 
@@ -75,37 +91,25 @@
                (let ((pt (point)))
                  (rust-rewind-irrelevant)
                  (backward-up-list)
-                 (cond 
-                  ((and
-                      (looking-at "[[(]")
-                      ; We don't want to indent out to the open bracket if the
-                      ; open bracket ends the line
-                      (save-excursion 
-                        (forward-char)
-                        (not (looking-at "[[:space:]]*\\(?://.*\\)?$"))))
-                   (+ 1 (current-column)))
-                  ;; Check for fields on the same line as the open curly brace:
-                  ((looking-at "{[[:blank:]]*[^}\n]*,[[:space:]]*$")
-                   (progn
-                    (forward-char)
-                    (forward-to-word 1)
-                    (current-column)))
-                  (t (progn
-                     (goto-char pt)
-                     (back-to-indentation)
-                     (if (looking-at "\\<else\\>")
-                         (* rust-indent-offset (+ 1 level))
-                       (progn
-                         (goto-char pt)
-                         (beginning-of-line)
-                         (rust-rewind-irrelevant)
-                         (end-of-line)
-                         (if (looking-back "[,;{}(][[:space:]]*\\(?://.*\\)?")
-                             (* rust-indent-offset level)
-                           (back-to-indentation)
-                           (if (looking-at "#")
+                 (or (and (looking-at "[[({]")
+                          (rust-align-to-expr-after-brace))
+                     (progn
+                       (goto-char pt)
+                       (back-to-indentation)
+                       (if (looking-at "\\<else\\>")
+                           (* rust-indent-offset (+ 1 level))
+                         (progn
+                           (goto-char pt)
+                           (beginning-of-line)
+                           (rust-rewind-irrelevant)
+                           (end-of-line)
+                           (if (looking-back
+                                "[[,;{}(][[:space:]]*\\(?://.*\\)?")
                                (* rust-indent-offset level)
-                             (* rust-indent-offset (+ 1 level)))))))))))
+                             (back-to-indentation)
+                             (if (looking-at "#")
+                                 (* rust-indent-offset level)
+                               (* rust-indent-offset (+ 1 level))))))))))
 
               ;; Otherwise we're in a column-zero definition
               (t 0))))))
@@ -206,6 +210,114 @@
 
          collect `(,(rust-re-item-def item) 1 ,face))))
 
+(defun rust-fill-prefix-for-comment-start (line-start)
+  "Determine what to use for `fill-prefix' based on what is at the beginning of a line."
+  (let ((result 
+         ;; Replace /* with same number of spaces
+         (replace-regexp-in-string
+          "\\(?:/\\*+\\)[!*]" 
+          (lambda (s)
+            ;; We want the * to line up with the first * of the comment start
+            (concat (make-string (- (length s) 2) ?\x20) "*"))
+          line-start)))
+       ;; Make sure we've got at least one space at the end
+    (if (not (= (aref result (- (length result) 1)) ?\x20))
+        (setq result (concat result " ")))
+    result))
+
+(defun rust-in-comment-paragraph (body)
+  ;; We might move the point to fill the next comment, but we don't want it
+  ;; seeming to jump around on the user
+  (save-excursion
+    ;; If we're outside of a comment, with only whitespace and then a comment
+    ;; in front, jump to the comment and prepare to fill it.
+    (when (not (nth 4 (syntax-ppss)))
+      (beginning-of-line)
+      (when (looking-at (concat "[[:space:]\n]*" comment-start-skip))
+        (goto-char (match-end 0))))
+
+    ;; We need this when we're moving the point around and then checking syntax
+    ;; while doing paragraph fills, because the cache it uses isn't always
+    ;; invalidated during this.
+    (syntax-ppss-flush-cache 1)
+    ;; If we're at the beginning of a comment paragraph with nothing but
+    ;; whitespace til the next line, jump to the next line so that we use the
+    ;; existing prefix to figure out what the new prefix should be, rather than
+    ;; inferring it from the comment start.
+    (let ((next-bol (line-beginning-position 2)))
+      (while (save-excursion
+              (end-of-line)
+              (syntax-ppss-flush-cache 1)
+              (and (nth 4 (syntax-ppss))
+                   (save-excursion 
+                     (beginning-of-line)
+                     (looking-at paragraph-start))
+                   (looking-at "[[:space:]]*$")
+                   (nth 4 (syntax-ppss next-bol))))
+        (goto-char next-bol)))
+
+    (syntax-ppss-flush-cache 1)
+    ;; If we're on the last line of a multiline-style comment that started
+    ;; above, back up one line so we don't mistake the * of the */ that ends
+    ;; the comment for a prefix.
+    (when (save-excursion
+            (and (nth 4 (syntax-ppss (line-beginning-position 1)))
+                 (looking-at "[[:space:]]*\\*/")))
+      (goto-char (line-end-position 0)))
+    (funcall body)))
+
+(defun rust-with-comment-fill-prefix (body)
+  (let*
+      ((line-string (buffer-substring-no-properties 
+                     (line-beginning-position) (line-end-position)))
+       (line-comment-start
+        (when (nth 4 (syntax-ppss)) 
+          (cond
+           ;; If we're inside the comment and see a * prefix, use it
+           ((string-match "^\\([[:space:]]*\\*+[[:space:]]*\\)"
+                          line-string)
+            (match-string 1 line-string))
+           ;; If we're at the start of a comment, figure out what prefix
+           ;; to use for the subsequent lines after it
+           ((string-match (concat "[[:space:]]*" comment-start-skip) line-string)
+            (rust-fill-prefix-for-comment-start 
+             (match-string 0 line-string))))))
+       (fill-prefix 
+        (or line-comment-start
+            fill-prefix)))
+    (funcall body)))
+
+(defun rust-find-fill-prefix ()
+  (rust-with-comment-fill-prefix (lambda () fill-prefix)))
+
+(defun rust-fill-paragraph (&rest args)
+  "Special wrapping for `fill-paragraph' to handle multi-line comments with a * prefix on each line."
+  (rust-in-comment-paragraph
+   (lambda () 
+     (rust-with-comment-fill-prefix
+      (lambda ()
+        (let
+            ((fill-paragraph-function
+              (if (not (eq fill-paragraph-function 'rust-fill-paragraph))
+                  fill-paragraph-function)))
+          (apply 'fill-paragraph args)
+          t))))))
+
+(defun rust-do-auto-fill (&rest args)
+  "Special wrapping for `do-auto-fill' to handle multi-line comments with a * prefix on each line."
+  (rust-with-comment-fill-prefix
+   (lambda ()
+     (apply 'do-auto-fill args)
+     t)))
+
+(defun rust-fill-forward-paragraph (arg)
+  ;; This is to work around some funny behavior when a paragraph separator is
+  ;; at the very top of the file and there is a fill prefix.
+  (let ((fill-prefix nil)) (forward-paragraph arg)))
+
+(defun rust-comment-indent-new-line (&optional arg)
+  (rust-with-comment-fill-prefix
+   (lambda () (comment-indent-new-line arg))))
 
 ;; For compatibility with Emacs < 24, derive conditionally
 (defalias 'rust-parent-mode
@@ -215,6 +327,7 @@
 ;;;###autoload
 (define-derived-mode rust-mode rust-parent-mode "Rust"
   "Major mode for Rust code."
+  :group 'rust-mode
 
   ;; Basic syntax
   (set-syntax-table rust-mode-syntax-table)
@@ -230,7 +343,21 @@
   ;; Misc
   (set (make-local-variable 'comment-start) "// ")
   (set (make-local-variable 'comment-end)   "")
-  (set (make-local-variable 'indent-tabs-mode) nil))
+  (set (make-local-variable 'indent-tabs-mode) nil)
+
+  ;; Allow paragraph fills for comments
+  (set (make-local-variable 'comment-start-skip) 
+       "\\(?://[/!]*\\|/\\*[*!]?\\)[[:space:]]*")
+  (set (make-local-variable 'paragraph-start)
+       (concat "[[:space:]]*\\(?:" comment-start-skip "\\|\\*/?[[:space:]]*\\|\\)$"))
+  (set (make-local-variable 'paragraph-separate) paragraph-start)
+  (set (make-local-variable 'normal-auto-fill-function) 'rust-do-auto-fill)
+  (set (make-local-variable 'fill-paragraph-function) 'rust-fill-paragraph)
+  (set (make-local-variable 'fill-forward-paragraph-function) 'rust-fill-forward-paragraph)
+  (set (make-local-variable 'adaptive-fill-function) 'rust-find-fill-prefix)
+  (set (make-local-variable 'comment-multi-line) t)
+  (set (make-local-variable 'comment-line-break-function) 'rust-comment-indent-new-line)
+  )
 
 
 ;;;###autoload