Use JSON output for rust and rust-cargo checkers

This commit replaces the error parsers for the rust and rust-cargo checkers to use the JSON output instead. As a side-effect, we skip the "help" messages which promote the use of `rustc --explain` in the tests. I'm assuming this is not critical, since we also always set the error code now. Closes GH-1035.
flycheck · Aug 25, 2016 · b420836 · b420836
1 parent 47f689a
commit b420836
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 50 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -6,6 +6,8 @@
   - Change ``flycheck-eslint-rulesdir`` (string) to
     ``flycheck-eslint-rules-directories`` (list of strings) [GH-1016]
 
+  - Require rust 1.7 or newer for ``rust`` and ``rust-cargo`` [GH-1036]
+
 - New syntax checkers:
 
   - Slim with ``slim-lint`` [GH-1013]
@@ -23,6 +25,8 @@
   - Add default directory for ``haskell-stack-ghc`` and ``haskell-ghc`` checkers
     [GH-1007]
 
+  - ``rust`` and ``rust-cargo`` checkers now support the new error format of
+    rust 1.12 [GH-1016]
 
 28 (Jun 05, 2016)
 =================

diff --git a/doc/languages.rst b/doc/languages.rst
@@ -879,7 +879,7 @@ to view the docstring of the syntax checker.  Likewise, you may use
 
       .. note::
 
-         These syntax checkers require Rust 1.0 or newer.
+         These syntax checkers require Rust 1.7 or newer.
 
       .. _Cargo: http://doc.crates.io/index.html
 

diff --git a/flycheck.el b/flycheck.el
@@ -5440,6 +5440,107 @@ about TSLint."
              (and (not (string-empty-p output))
                   (json-read-from-string output)))))
 
+(defun flycheck-parse-rust (output checker buffer)
+  "Parse rust errors from OUTPUT and return a list of `flycheck-error'.
+
+CHECKER and BUFFER denote the CHECKER that returned OUTPUT and
+the BUFFER that was checked respectively.
+
+The expected format for OUTPUT is a mix of plain text lines and
+JSON lines.  This function ignores the plain text lines and
+parses only JSON lines.  Each JSON line is expected to be a JSON
+object that corresponds to a diagnostic from the compiler.  The
+expected diagnostic format is described there:
+
+https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs#L67-L139"
+  (let* ((json-array-type 'list)
+         (json-false nil)
+         ;; Skip the plain text lines in OUTPUT, keep the JSON lines.
+         (json-lines (seq-filter (lambda (line)
+                                   (string-match-p "^{" line))
+                                 (split-string output "\n")))
+         ;; Each JSON line is a JSON object.
+         (diagnostics (seq-map #'json-read-from-string json-lines))
+         (errors))
+    ;; The diagnostic format is described in the link above.  The gist of it is
+    ;; that each diagnostic can have several causes in the source text; these
+    ;; causes are represented by spans.  The diagnostic has a message and a
+    ;; level (error, warning), while the spans have a filename, line, column,
+    ;; and an optional label.  The primary span points to the root cause of the
+    ;; error in the source text, while non-primary spans point to related
+    ;; causes.  In addition, each diagnostic can also have children diagnostics
+    ;; that are used to provide additional information through their message
+    ;; field, but do not seem to contain any spans (yet).
+    ;;
+    ;; We first iterate over diagnostics and their spans to turn every span into
+    ;; a flycheck error object, that we collect into the `errors' list.
+    (dolist (diagnostic diagnostics)
+      (let ((error-message)
+            (error-level)
+            (error-code)
+            (primary-filename)
+            (primary-line)
+            (primary-column)
+            (spans)
+            (children))
+
+        ;; Nested `let-alist' cause compilation warnings, hence we `setq' all
+        ;; these values here first to avoid nesting.
+        (let-alist diagnostic
+          (setq error-message .message
+                error-level (pcase .level
+                              (`"error" 'error)
+                              (`"warning" 'warning)
+                              (_ 'error))
+                ;; The 'code' field of the diagnostic contains the actual error
+                ;; code and an optional explanation that we ignore
+                error-code .code.code
+                spans .spans
+                children .children))
+
+        (dolist (span spans)
+          (let-alist span
+            ;; Children lack any filename/line/column information, so we use
+            ;; those from the primary span
+            (when .is_primary
+              (setq primary-filename .file_name
+                    primary-line .line_start
+                    primary-column .column_start))
+            (push
+             (flycheck-error-new-at
+              .line_start
+              .column_start
+              ;; Non-primary spans are used for notes
+              (if .is_primary error-level 'info)
+              (if .is_primary
+                  ;; Primary spans may have labels with additional information
+                  (concat error-message (when .label
+                                          (format " (%s)" .label)))
+                .label)
+              :id error-code
+              :checker checker
+              :buffer buffer
+              :filename .file_name)
+             errors)))
+
+        ;; Then we turn children messages into flycheck errors pointing to the
+        ;; location of the primary span.  According to the format, children
+        ;; may contain spans, but they do not seem to use them in practice.
+        (dolist (child children)
+          (let-alist child
+            (push
+             (flycheck-error-new-at
+              primary-line
+              primary-column
+              'info
+              .message
+              :id error-code
+              :checker checker
+              :buffer buffer
+              :filename primary-filename)
+             errors)))))
+    (nreverse errors)))
+
 
 ;;; Error parsing with regular expressions
 (defun flycheck-get-regexp (patterns)
@@ -8282,32 +8383,23 @@ Relative paths are relative to the file being checked."
 (flycheck-define-checker rust-cargo
   "A Rust syntax checker using Cargo.
 
-This syntax checker needs Rust 1.0 or newer, and Cargo with the
+This syntax checker needs Rust 1.7 or newer, and Cargo with the
 rustc command.  See URL `https://www.rust-lang.org'."
   :command ("cargo" "rustc"
             (eval (cond
                    ((string= flycheck-rust-crate-type "lib") "--lib")
                    (flycheck-rust-binary-name
                     (list "--bin" flycheck-rust-binary-name))))
             "--" "-Z" "no-trans"
+            ;; Passing the "unstable-options" flag may raise an error in the
+            ;; future.  For the moment, we need it to access JSON output in all
+            ;; rust versions >= 1.7.
+            "-Z" "unstable-options"
+            "--error-format=json"
             (option-flag "--test" flycheck-rust-check-tests)
             (option-list "-L" flycheck-rust-library-path concat)
             (eval flycheck-rust-args))
-  :error-patterns
-  ((error line-start (file-name) ":" line ":" column ": "
-          (one-or-more digit) ":" (one-or-more digit) " error: "
-          (or
-           ;; Multiline errors
-           (and (message (minimal-match (one-or-more anything)))
-                " [" (id "E" (one-or-more digit)) "]")
-           (message))
-          line-end)
-   (warning line-start (file-name) ":" line ":" column ": "
-            (one-or-more digit) ":" (one-or-more digit) " warning: "
-            (message) line-end)
-   (info line-start (file-name) ":" line ":" column ": "
-         (one-or-more digit) ":" (one-or-more digit) " " (or "note" "help") ": "
-         (message) line-end))
+  :error-parser flycheck-parse-rust
   :modes rust-mode
   :predicate (lambda ()
                ;; Since we build the entire project with cargo rustc we require
@@ -8319,30 +8411,21 @@ rustc command.  See URL `https://www.rust-lang.org'."
 (flycheck-define-checker rust
   "A Rust syntax checker using Rust compiler.
 
-This syntax checker needs Rust 1.0 or newer.  See URL
+This syntax checker needs Rust 1.7 or newer.  See URL
 `https://www.rust-lang.org'."
   :command ("rustc" "-Z" "no-trans"
             (option "--crate-type" flycheck-rust-crate-type)
+            ;; Passing the "unstable-options" flag may raise an error in the
+            ;; future.  For the moment, we need it to access JSON output in all
+            ;; rust versions >= 1.7.
+            "-Z" "unstable-options"
+            "--error-format=json"
             (option-flag "--test" flycheck-rust-check-tests)
             (option-list "-L" flycheck-rust-library-path concat)
             (eval flycheck-rust-args)
             (eval (or flycheck-rust-crate-root
                       (flycheck-substitute-argument 'source-inplace 'rust))))
-  :error-patterns
-  ((error line-start (file-name) ":" line ":" column ": "
-          (one-or-more digit) ":" (one-or-more digit) " error: "
-          (or
-           ;; Multiline errors
-           (and (message (minimal-match (one-or-more anything)))
-                " [" (id "E" (one-or-more digit)) "]")
-           (message))
-          line-end)
-   (warning line-start (file-name) ":" line ":" column ": "
-            (one-or-more digit) ":" (one-or-more digit) " warning: "
-            (message) line-end)
-   (info line-start (file-name) ":" line ":" column ": "
-         (one-or-more digit) ":" (one-or-more digit) " " (or "note" "help") ": "
-         (message) line-end))
+  :error-parser flycheck-parse-rust
   :modes rust-mode
   :predicate (lambda ()
                (and (not flycheck-rust-crate-root) (flycheck-buffer-saved-p))))

diff --git a/test/flycheck-test.el b/test/flycheck-test.el
@@ -3835,19 +3835,15 @@ Why not:
   (let ((flycheck-disabled-checkers '(rust-cargo)))
     (flycheck-ert-should-syntax-check
      "language/rust/src/syntax-error.rs" 'rust-mode
-     '(4 5 error "unresolved name `bla`" :checker rust :id "E0425")
-     '(4 5 info "run `rustc --explain E0425` to see a detailed explanation"
-         :checker rust))))
+     '(4 5 error "unresolved name `bla`" :checker rust :id "E0425"))))
 
 (flycheck-ert-def-checker-test rust rust multiline-error
   (let ((flycheck-disabled-checkers '(rust-cargo)))
     (flycheck-ert-should-syntax-check
      "language/rust/src/multiline-error.rs" 'rust-mode
-     '(7 9 error "mismatched types" :checker rust :id "E0308")
-     '(7 9 info "run `rustc --explain E0308` to see a detailed explanation"
-         :checker rust)
-     '(7 9 info "expected type `u8`" :checker rust)
-     '(7 9 info "found type `i8`" :checker rust))))
+     '(7 9 error "mismatched types (expected u8, found i8)" :checker rust :id "E0308")
+     '(7 9 info "expected type `u8`" :checker rust :id "E0308")
+     '(7 9 info "found type `i8`" :checker rust :id "E0308"))))
 
 (flycheck-ert-def-checker-test rust rust warning
   (let ((flycheck-disabled-checkers '(rust-cargo)))
@@ -3860,20 +3856,15 @@ Why not:
   (let ((flycheck-disabled-checkers '(rust-cargo)))
     (flycheck-ert-should-syntax-check
      "language/rust/src/note-and-help.rs" 'rust-mode
-     '(11 9 info "value moved here" :checker rust)
-     '(12 9 error "use of moved value: `x`" :checker rust :id "E0382")
-     '(12 9 info "run `rustc --explain E0382` to see a detailed explanation"
-          :checker rust)
-     '(12 9 info "move occurs because `x` has type `NonPOD`, which does not implement the `Copy` trait"
-          :checker rust))))
+     '(11 9 info "value moved here" :checker rust :id "E0382")
+     '(12 9 error "use of moved value: `x` (value used here after move)" :checker rust :id "E0382")
+     '(12 9 info "move occurs because `x` has type `NonPOD`, which does not implement the `Copy` trait" :checker rust :id "E0382"))))
 
 (flycheck-ert-def-checker-test rust rust crate-root-not-set
   (let ((flycheck-disabled-checkers '(rust-cargo)))
     (flycheck-ert-should-syntax-check
      "language/rust/src/importing.rs" 'rust-mode
-     '(1 5 error "unresolved import `super::imported`. There are too many initial `super`s." :checker rust :id "E0432")
-     '(1 5 info "run `rustc --explain E0432` to see a detailed explanation"
-         :checker rust))))
+     '(1 5 error "unresolved import `super::imported`. There are too many initial `super`s." :checker rust :id "E0432"))))
 
 (flycheck-ert-def-checker-test sass sass nil
   (flycheck-ert-should-syntax-check