Error not shown at the appropriate place when the error is in a macro invocation

[utkarshkukreti]

Steps to reproduce:

$ cargo new a --bin
$ cd a
$ atom .
# edit src/main.rs
$ cat src/main.rs
fn main() {
    assert_eq!("one", 1);
}

Run linter in Atom in any of these modes: build, check, test, rustc, and you'll see the following error inserted by this package:

Both the line numbers and the file name are wrong. Clicking on the at line 5 col 6 ... link takes me to a new file buffer with name <std macros>.

This is the output of cargo test:

cargo test
   Compiling a v0.1.0 (file:///private/tmp/a)
error[E0277]: the trait bound `&str: std::cmp::PartialEq<{integer}>` is not satisfied
 --> src/main.rs:2:5
  |
2 |     assert_eq!("one", 1);
  |     ^^^^^^^^^^^^^^^^^^^^^ trait `&str: std::cmp::PartialEq<{integer}>` not satisfied
  |
  = help: the following implementations were found:
  = help:   <str as std::cmp::PartialEq<std::ffi::OsString>>
  = help:   <str as std::cmp::PartialEq<std::ffi::OsStr>>
  = help:   <str as std::cmp::PartialEq<std::string::String>>
  = help:   <&'a str as std::cmp::PartialEq<std::string::String>>
  = help: and 3 others
  = note: this error originates in a macro outside of the current crate

error: aborting due to previous error

error: Could not compile `a`.

To learn more, run the command again with --verbose.

which does include the appropriate place to insert the error: src/main.rs:2:5.

[utkarshkukreti]

Just realized that linter-rust uses the output of --message-format json now instead of parsing the textual output of cargo test. I'm investigating the cause. Here's the output of cargo test --message-format json for future reference: https://gist.github.com/utkarshkukreti/66f48babff1a3b7d5689fc313ef4f432

[utkarshkukreti]

With the following changes I think I may have fixed this bug (I hope I haven't introduced a new one).

diff --git a/lib/mode.coffee b/lib/mode.coffee
index fdbf65a..b109958 100644
--- a/lib/mode.coffee
+++ b/lib/mode.coffee
@@ -37,6 +37,8 @@ parseJsonMessages = (messages, {disabledWarnings}) ->
     continue unless input and input.spans
     primary_span = input.spans.find (span) -> span.is_primary
     continue unless primary_span
+    if primary_span.expansion && primary_span.expansion.span
+      primary_span = primary_span.expansion.span
     range = [
       [primary_span.line_start - 1, primary_span.column_start - 1],
       [primary_span.line_end - 1, primary_span.column_end - 1]

I would really appreciate if anyone could confirm all this before I make a PR!

[White-Oak]

@utkarshkukreti thanks for the great report and the research put into the problem! Yes, that's been haunting me quite for a while, but I haven't managed to put an example to test it. Unfortunately, tests are still in the process of writing, so it's quite hard to say whether this breaks something or not.

Also, there's been a similar problem with old messages -- to check them run rustup override set 1.11.0 for the directory that holds the example, restart (or uncheck Cache Versions in settings) Atom and relint the code. The code responsible for a parsing of old messages is in parseOldMessages in mode.coffee. Would be great if you fixed that as well in your PR or confirmed that its not fixable/works like expected.

Btw, just FYI, you can use <details> tag to hide big chunks of code behind a spoiler. Writing that just because it was not fun scrolling through error JSON on mobile :)

[utkarshkukreti]

I can reproduce the same error with 1.11.0, but since I don't use <= 1.11.0 myself, I'm not sure if changing that would break anything I wouldn't notice. Also, since my changes don't really break existing code, and the fact that 1.11.0 will be an ancient version soon, would you merge just a fix for the new one?

I've been using this patch since my last comment and everything seems to be fine (for latest Rust) although I haven't worked on multi file projects since then. I'll try it out on some huge projects before sending a PR.

Btw, just FYI, you can use <details> tag to hide big chunks of code behind a spoiler. Writing that just because it was not fun scrolling through error JSON on mobile :)

Sorry about that. <details> doesn't seem to have any styling for Firefox so I've just moved the logs to a gist.

[White-Oak]

Yeah, it's ok, just make a PR

[White-Oak]

Reopening as the bug is not fixed for old messages, I'm going to investigate that one.

[utkarshkukreti]

@White-Oak One downside of my approach that I just realized is that now only the macro invocation is highlighted, which is sometimes not so helpful. For example, here's one screenshot:

It would have been more helpful if "bytes" was highlighted here.

Maybe we should highlight all spans? Just an idea. I have not considered this in detail.

[White-Oak]

@utkarshkukreti there is actually a big bug considering highlights as well. Unfortunately, JSON format is not really convenient. Yes, I think it would be nice to have all spans, but it means we will have to add a note per exapnsion for an error.