Handling of primary vs. secondary spans in checkers

[dpc]

rustc output must have changed, and now every time I compile my code and call a method with a wrong number of arguments, Vim will jump to the definition of the function that is called incorrectly, and not the place where it is being called. There's no easy way jump to the actual code that I need to fix, which is slowing me down a lot: I have to manually inspect the compiler output in another terminal, and manually go to the right place.

It seems to me that the output of the compiler is wrong in the first place: the main error file:line message should point to the broken code, not the definition of the function that is called (which is useful, but secondary). Ideally I'd like rust.vim to be able to display both in the compilation error list.

[estebank]

The plugin should point at the first primary span, not at the first span, as sometimes diagnostics will have a secondary span before the main span.

[da-x]

@dpc, is this still relevant with Rust 1.27? I vaguely remember seeing this myself (I work with ALE as a checker) but I wasn't able to create a small working demo. The linked playground from the rust-lang/rust issue shows a single error at the correct place (the call).

[dpc]

It still happens here with neomake

rustc 1.28.0-nightly (f28c7aef7 2018-06-19)

   Compiling brainwiki v0.1.0 (file:///home/dpc/lab/brainwiki)
error[E0061]: this function takes 1 parameter but 2 parameters were supplied                                                                       
  --> src/main.rs:78:19
   |
78 |     state.write().insert_from_dir(&opts.data_dir, 1).unwrap();
   |                   ^^^^^^^^^^^^^^^ expected 1 parameter
   | 
  ::: src/data.rs:78:5
   |
78 |     pub fn insert_from_dir(&mut self, dir_path: &Path) -> Result<()> {                                                                        
   |     ---------------------------------------------------------------- defined here   

I don't quite understand why, since the right place is first. I have a neomake set to do the check on write.

[da-x]

Thanks - I managed to reproduce the issue - this problem also afflicts ALE.

Thinking of it, it affects all checkers that sort by line number and don't really do anything with the is_primary boolean in the error message JSON. I am not sure what we can do about Syntastic and Neomake right now, but I have already issued a PR in ALE:

dense-analysis/ale#1696

Ignoring the secondary spans is one way to solve it. Unfortunately ALE sorts by line numbers too - if we don't want to ignore the secondary spans we will have teach it to sort by an additional key. @w0rp what do you think?

[w0rp]

I don't know what you mean about sorting by an additional key. ALE sorts items by buffer or filename, then by line, then by column.

[w0rp]

Why is there an error message showing where the function is defined at all? Who cares? There's no fault at the function if a function is called incorrectly. That's like blaming Ford if someone crashes a car because they were speeding.

[estebank]

@w0rp it's not about blame, it's about context. We show you the method definition so you have the signature front and center so you can fix your code without having to search for it. If we don't have the span (which we don't for external crates), we show only the compiler's knowledge of the signature (only method name and types) as a note.

We also use secondary spans for many things that can be considered "irrelevant", but that are useful to try and keep head-scratching to a minimum. If you do not care at all about these, you can use the short error format instead (rust-lang/rust#49546) if you're calling rustc from the terminal. For tools such as your's, we differentiate the types of spans precisely so you can ignore the secondary labels if you need to.

The compiler team is making a balancing act between avoiding overwhelming newcomers to the language, not being too obscure for them and not spamming experienced programmers. Because of this the long term plan is to support 3 levels of verbosity: short which may as well have been called oneline (opt-in, linked above), the default which is what you currently see where the compiler has error messages with hints and maybe a short explanation, and teach (opt-in, rust-lang/rust#47652) to provide a ton of information for people that are just starting, comparable to the text in the error code index.

[dpc]

This problem still exists, and it is still being a painful thorn in the Rust in Vim experience.

~~I think Rust should at very list a flag that or something that would make all the existing software work OK.~~~ (If that's the --short one, @w0rp please just use that if you don't agree with the original PR).