Add support for new error format

[jpernst]

This adds basic support for the new rustc error format used in recent nightlies. It appears to be working, but it's possible I've missed some edge cases, so comments are welcome.

Also, I'm not at all familiar with syntastic or how it works, so I haven't tested that part yet. If someone who uses that plugin could give this a try and let me know if it's broken, that would be great.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @chris-morgan (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[ExpHP]

Also, I'm not at all familiar with syntastic or how it works, so I haven't tested that part yet. If someone who uses that plugin could give this a try and let me know if it's broken, that would be great.

It seems the pieces are not all quite there yet. I don't know too much about it all myself, so here is an image comparison of how syntastic looks in both the current version and your PR, and on stable versus nightly:

http://imgur.com/a/5j4xp

[jpernst]

Ok, I think the reason for this is that the old errorformat for syntastic had an eager match that prevented the subsequent lines of the new format from matching. I've removed that line; see if this works better now.

[ExpHP]

I added two new images to the album. The new error format messages are now properly integrated into the interface (I can write code again!! 🎉) However there is a discrepancy in where matching ends against the old format error messages.

[jpernst]

Yeah, that extra error message is a bit vexing for me as well. Unfortunately, with how the new error format works, it's completely indistinguishable from a "real" error, and there are several such "noise" messages. I can't think of a good way to deal with this without resorting to lots of hardcoded cases to ignore them all individually.

If someone has a better way I'd be interested to know.

[ExpHP]

I recall the PR which added the new error messages also says something about a "json mode"). Rummaging around the options it looks one can use rustc --error-format json (adding -Z unstable-options on stable).

It's certainly a less ambiguous format, though I wouldn't dare try parsing it with a regex... I wonder if there is another way to configure these plugins without regex?

[jpernst]

I mentioned this on the internals board, and they seem to indicate that those extra messages aren't final and may be revised, so I'm inclined to just live with them until the dust settles, then we'll see.

[kesselborn]

Hey, I just implemented the (I think) same thing today, but perhaps it's not exactly the same? I wanted to have proper quickfix browsing in the following cases:

• cargo build reports errors
• cargo build reports warnings
• cargo test reports errors

If I try out the current pull request, this does not work for me ... or is this PR only for syntastic (which I don't really use) only?

Adding the following snippet to ftplugin/rust.vim works for me correctly with the old and the new error formats (nightly and stable on September 4th, 2016):

" order matters:
" newer rustc errors and warnings
setl errorformat=%Eerror%m,%Z\ \ -->\ %f:%l:%c,%Wwarning%m,%Z\ \ \ -->\ %f:%l:%c
" older rustc errors and warnings
setl errorformat+=%f:%l:%c:\ %*[0-9]:%*[0-9]\ %trror:%m,%f:%l:%c:\ %*[0-9]:%*[0-9]\ %tarning:%m
" cargo test error messages:
setl errorformat+=%E----%m,%Z%m\ %f:%l

[jpernst]

This PR should work (and does for me) with cargo; I'm not sure why it isn't for you. I get errors and warnings from cargo just fine, except for "test" which I haven't added, since I don't usually run that from inside vim.

[MarkSwanson]

kesselborn, your little snippet works great for me (but not quite perfect). Scenarios I tried (via vim :mak):
(I use vim-localvimrc to set makeprog to use a shell script to exec cargo)

• syntax error in current src/file.rs -> takes me to the correct line number. Great!
• syntax error in different src/file2.rs -> takes me to the correct line number in the other file. Great!
• test syntax error in current test/file.rs -> fail. Does not move the cursor to the error line reported by cargo.

kesselborn, my test output is rather large and might be breaking things for you. Here is a snippet of what happens when I try to compile test/file.rs on my machine:

Compiling file v1.0.4 (file:///home/maswanso/src/file/test/file.rs)
     Running `rustc file.rs --crate-name file -g --test -C metadata=2ac560e36b429231 <800 characters...>
error: expected item, found `ufse`
  --> file.rs:34:1
   |
34 | ufse file_logger::spdlog_wrapper::{Spdlog, SpdlogProxy, FileLogger};

error: aborting due to previous error

error: Could not compile `file`.

Caused by:
  Process didn't exit successfully: `rustc file.rs <800 characters...>

jpernst - I'm sure it works fine for you on your machine. :-) That's just how it goes sometimes :-) But your patches don't yet handle the new format on my machine.

[MarkSwanson]

Er... it's late and I must be tired because it's no longer taking me to the right file or line number.

I've opened up the vim help on errorformat and am playing around. I note the example Python multiline format string uses \C. Playing...

[MarkSwanson]

Ok, I've played around with things a bit. Here's what works for me 100% in every case:

setl errorformat=%Eerror:\ %m,%Z\ \ -->\ %f:%l:%c

I do have 'error' in the ~11000 (eleven thousand) characters that vim has to regex search through. I thought adding the ':\ ' might help but I don't think it did because when I start putting things back like this:

setl errorformat=%Eerror:\ %m,%Z\ \ -->\ %f:%l:%c,%Wwarning%m,%Z\ \ \ -->\ %f:%l:%c

Then things don't work anymore.

I'm a bit lost in the weeds I think :-)
Hope this helps. If you want me to try stuff please let me know.

Thanks!

[MarkSwanson]

Ok, I circled back and noticed a bug in the Rust code that generates the error.

Bug: It 'non-deterministically' uses 2 or 3 spaces in front of "-->"

error[E0425]: unresolved name `MY_ERROR_V1`
   --> src/server.rs:111:11

This is why it 'stopped' working for me earlier - different errors have different spaces.

What seems to work for me:

setl errorformat=\ \ \ -->\ %f:%l:%c,\ \ -->\ %f:%l:%c

Obviously it's a hack for now - but I'm still figuring out the different regex syntax (to support 0-n spaces before '-->') and all of the vim errorformat mechanisms...

[jpernst]

One possibility for why it's not working for some: In the original plugin (and in my PR), the errorformat is set for the cargo compiler, which must explicitly be enabled when editing to take effect. In my init.vim I have an autocmd that enables it as follows:
autocmd FileType rust compiler cargo
If cargo is being invoked some other way, the proper errorformat might not be present. Just a hunch anyway.

As for detecting the padding, %# after the space should have the effect of a regex * and capture any number of spaces. I believe the arrow's offset is determined by the number of digits in the linenumber, but that's just what I've observed, and can't say for sure if that always the case.

[MarkSwanson]

Ok, the %# worked like a charm. Thanks!

setl errorformat=%Eerror%m,%Z\ %#-->\ %f:%l:%c

Interesting hunch about the autocmd. I actually don't set 'compiler'.
I've re-skimmed the docs on compiler a couple of times and still don't understand how that interacts with makeprg.

I submit to you that there is tremendous value in allowing people to have a rust compiler other than cargo. Using localvimrc enables me to dynamically change the makeprg. Examples:

• editing test/file.rs => I can run some variant of 'cargo test' against specific Fns with custom settings that are tuned to specific crates/files. I don't have to leave vim.
• I can do things like 'sudo' before running the test - and I'm prompted within vim for the root password. I don't have to leave vim.
• I'm doing other custom things like dynamically setting LD_LIBRARY_PATH or setting other variables.
• Various other things to make life easier.

Is it possible to just use:

autocmd FileType rust

(without the 'compiler cargo' qualification) ?

Thanks!

[euclio]

@jpernst I think that the rustfmt error parsing needs to be updated as well (or removed entirely...? Shouldn't the errors be reported by rustc anyways?). Since it can't parse the new error format, if there's a syntax error in the rustfmt'd code, it just sends the error to the console.

[jpernst]

@MarkSwanson autocmd FileType rust just means "run the following command when you enter a buffer with a rust file", in this case the command being compiler cargo. It should be noted that the cargo compiler inherits settings from the rustc compiler, so the errorformat should be set properly for both. For other compiler configurations, the errorformat could simply be copied into them. Supporting rustc and cargo will probably cover 99% of most out-of-box uses. Adding different compiler configurations is probably something best discussed in another issue/PR.

@euclio Huh, I've never even used rustfmt before so I have no idea what's going on there. Perhaps I'll take a look, but if someone else is more familiar with rustfmt and what's supposed to happen wrt errors there, please chime in.

[euclio]

Maybe @grncdr can comment since he wrote that code.

Either way, a workaround for now is to set g:rustfmt_fail_silently=1.

[grncdr]

to say I wrote it is a bit of an exaggeration, I only hacked the vim-go plugin with some regexes that seemed to work with the toolchain I was using at the time. (it looks like vim-go hasn't changed much since then)

IMO the most sensible approach is using rustc (with --error-format json) to check for syntax errors before running rustfmt. I don't know for sure what errors rustfmt might emit after that, but those are the only ones the line of code in question should be handling.

[steveklabnik]

Well, the new errors are out, and I'm a bad maintainer. What's the state of this PR?

[jpernst]

I just updated this yesterday to show all note lines, since they tend to be important, and to filter out various extraneous status messages from rustc/cargo. It's working well for me so far, but more testing is always welcome, particularly from people who use the syntastic support.

[steveklabnik]

I am inclined to merge this, then. It seems small enough.

Others in this thread, what do you think?

[ExpHP]

It has been looking pretty good on nightly.

I just dropped into a stable crate to test with Rust 1.12 and this bit of noise about -Z still shows up.

(on a tangential but closely related note, it might be a good idea to start pushing for stabilization of -Zparse-only? 😛 )

[jpernst]

Hmm, I forgot about that. I'm a little conflicted about filtering that since it indicates that the syntastic plugin will eventually break, but perhap's that's a problem for another day. I'll look into silencing it.

[ExpHP]

I do rather feel the same way! Perhaps keeping it around might make people a little more motivated... ;)

[jpernst]

@ExpHP You make a good point; for now I'll leave that warning in. Also, I pushed some minor adjustments to the syntastic matcher, could you give it a try when you get a chance to make sure everything is still sane? If so, then this should be ready to merge.

[ExpHP]

Still looks good to me (or at least, the parts I use)!

I did some comparisons trying to find what changed, and I see that the new commit filters out a bit of noise that I've apparently become blind to (the || aborting due to previous error), so that's good too. :)

[jpernst]

Yeah, having "fake" errors in the list can really mess things up, especially when it tries to jump to the error with no associated location, so I decided it was imporant to remove those from the list finally.

Sounds like things are pretty solid now, so unless someone else comes forward with a bug, I think we're good to go.

[jpernst]

After using this for a little while longer, I've been forced to unhide all of the source context in the error messages. This adds back a significant amount of noise, but unfortunately it sometimes contains critical information that isn't otherwise shown, such as the exact types in a type mismatch. Lacking that information hurts more than sifting through the extra noise.

[mbrubeck]

cc #88

[steveklabnik]

Okay! It's been a day. Let's so that it doesn't get lost again. We can always tweak things if they need fixing.

Thanks all!

[irfansharif]

@ExpHP: newbie to rust and rust.vim here, concerning the option Z is unstable.. noise, how do i get rid of it?

edit: addressed this by using the nightly rustc build.

[withoutboats]

I'd really like to see the unstable flag error filtered out of Syntastic's error list. Exposing this to rust.vim users is very frustrating. It is certainly not end users' responsibility to be concerned about the stability of this plugin's integration with rustc.

[Rolandde]

@withoutboats : Another way to silence the message is to use Synstatic's option

 let g:syntastic_quiet_messages = {"regex": 'is unstable and should only be used on the nightly compiler, but it is currently accepted for backwards compatibility; this will soon change, see issue #31847 for more details'}

I'm not sure how this affects speed, but I doubt it will match any other errors.