Align error messages for easier comparing

[mdinger]

Closes #18946

This fix is very simple. Aligns, fixes the second set of backticks, and doesn't do color changes as suggested in #18946. Aligning is definitely nice...maybe the expected/found lines should be tabbed out. I'm unsure.

---

Given this:

fn test() -> Option<int> {
    Ok(7) // Should be Some(7)
}
fn main(){ 
    test();
}

Previously returned:

<anon>:2:5: 2:10 error: mismatched types: expected `core::option::Option<int>`, found `core::result::Result<_, _>` (expected enum core::option::Option, found enum core::result::Result)
<anon>:2     Ok(7)
             ^~~~~
error: aborting due to previous error
playpen: application terminated with error code 101

Now returns:

$ ./rustc test.rs
test.rs:2:5: 2:10 error: mismatched types:
 expected `core::option::Option<int>`,
    found `core::result::Result<_, _>`
(expected `enum core::option::Option`,
    found `enum core::result::Result`)
test.rs:2     Ok(7) // Should be Some(7)
              ^~~~~
error: aborting due to previous error

[ftxqxd]

I don’t think that the parenthesised part of the message should have backticks around all of it. In quite a lot of cases, the parenthesised part refers to something like an ‘integral variable’ (e.g., the message from let x: Box<int> = 1;), and should not be surrounded by backticks, as it does not represent code. Even something like ‘enum std::option::Option’ shouldn’t have backticks around the whole message; the actual code part is the ‘std::option::Option’ part, so that alone should have backticks around it. Making that change should be as simple as modifying rustc::middle::ty::ty_sort_string to put backticks around the path of any enum, struct, or trait.

I also suspect that some tests may be affected by this change (and stop passing). Have you run make check on these changes? I suspect that, since this is the first multi-line error message in Rust, that compiletest will have to be updated to recognise that messages can span multiple lines.

[mdinger]

Hmm...I'll probably try to update that backtick part then and try make check but it'll probably be a week. I actually thought make check was automatically run during standard build since compiling takes 2 hours (though tests fails/succeeds never appeared).

[ghost]

@mdinger This will be a vast improvement to the readability of error messages, thanks for tackling this!

[tomjakubowski]

How does spreading the error message across several lines interact with using e.g. :make in Vim or M-x compile in Emacs?

[mdinger]

@jakub- Appreciated thought. Hopefully I'll get it to where it can land...

@tomjakubowski ... I don't understand the question. I've never used :make in Vim and never used Emacs. How would the editor modify a line writing command?

[tomjakubowski]

@mdinger let me give a concrete example. If I'm editing this file foo.rs in Vim:

#![deny(missing_doc)]

pub struct Foo;

pub trait Bar {}

/// Baz
pub trait Baz {
    fn baz();
}
fn main() {
}

if I type :compiler rustc, and then :make, Vim will run the rustc compiler plugin which invokes rustc on the current buffer's file (there is a similar plugin for Cargo). If there are errors, I can jump between them with ]q and [q, and I can open a list of the errors to view in my editor:

The compiler plugin includes instructions for parsing rustc's error messages for needed and relevant information and parsing out the rest. My question is how these changes to spread error messages across lines will affect that, and if the shipped editor plugins will need to be updated to reflect them. (cc @kballard and @chris-morgan who I believe know more about the Vim support files)

I thought we shipped something for Emacs which did more or less the same thing for M-x compile but I can't seem to find it.

[lilyball]

I'm not convinced this is necessarily a readability improvement. I'm worried that it will make it harder to pick apart individual errors when presented with a long error spew. Can you provide some examples of long error spews using this new format?

[lilyball]

Also, examples of errors that come from macro-expanded text, because those present extra error information.

[mahkoh]

#16845 comes to mind.

[mdinger]

@kballard Does this look better (didn't change the code at all...just conjecture)?

<anon>:2:5: 2:10 error: mismatched types: expected `core::option::Option<int>`,
                                             found `core::result::Result<_, _>`
                                         (expected enum `core::option::Option`,
                                             found enum `core::result::Result`)
<anon>:2     Ok(7)
             ^~~~~
error: aborting due to previous error
playpen: application terminated with error code 101

I find the types much easier to compare with both my examples. The left alignment of the first makes the parent error harder to distinguish.

I think I can get longer error strings but I don't have macro errors. If someone can provide them, I'll try them when I get to it.

[lilyball]

@mdinger Indented continuation lines like that will probably not play well with word wrapping, especially if the first line has a long enough pathname that it wraps before the line continuation would have even started.

[mdinger]

Yes, this is a concern but to me personally, it is irrelevant because any linewrap at all is confusing (disconcerting) so I make my compilation terminal window really really wide so wrap never happens.

(my opinion regarding wordwrap may not be the majority though)

[chris-morgan]

@mdinger: you must have a wide screen to cope with types like core::iter::Map<'_, collections::string::String, collections::string::String, core::iter::Filter<'_, collections::string::String, core::iter::Map<'_, collections::string::String, collections::string::String, collections::vec::MoveItems<collections::string::String>>>>! (And that’s by no means the longest I’ve seen, or even used myself.)

[mdinger]

Okay, fine. Even without botched wrapping though, it'd still be better if they were stacked. Two of these are difficult to compare when side-by-side. Then, any competent rust build environment (like an IDE) should "hopefully" support side scrolling instead of wrapping.

(without that, the window can be made wider than the screen and then panned. definitely not ideal though...)

Sigh... sounds like the world isn't ready for multiline error messages...

[chris-morgan]

It’s not rational to expect a build environment to support side scrolling instead of wrapping. I’m confident in saying that a plain terminal will be the most popular build output location for a long time, and it does not support side scrolling. (Feeding it through a pager with support for scrolling is not really a reasonable alternative for the primary interface, it’s not how anyone would expect it to work and it would confuse people and turn them off it.)

[sinistersnare]

Im for aligning messages, on windows cmd you cant change the width of the terminal easily, and having nicely formatted errors would be very nice than a giant spew of sadness. Maybe less newline though, something like:

 expected `core::option::Option<int>`, found `core::result::Result<_, _>`
(expected `enum core::option::Option`, found `enum core::result::Result`)
test.rs:2     Ok(7) // Should be Some(7)

Good idea!

[mdinger]

@chris-morgan Okay. What about a compiler flag or something stating I want multiline error messaging. Then an IDE or anything else could opt into nicer errors and disable wrapping accordingly.

@sinistersnare The reason I formatted it as before was because the first is difficult to find what is different between the two. See how with the second, it is straightforward to see where the only difference lies. Ignore whether this error is even a valid type.

expected `core::iter::Map<collections::string::String, int>`, found ` core::iter::Map<collections::string::String, i32>`
// Compare ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
// And ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^

// Observe that these are straightforward to compare:
expected `core::iter::Map<collections::string::String, int>`,
   found `core::iter::Map<collections::string::String, i32>`
// Here ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 

[mdinger]

An extra compiler flag would require an RFC correct?

---

Closing as it does not cope well with word wrap and really long type names.

An alternate possibility is to enable multiline comments via a compiler flag so that the user or IDE can request when desired. An extra compiler flag for only one error type may seem excessive so it should be determined if there are any other errors that would benefit from being multiline.

[ftxqxd]

It’s sad to see this pull request closed. I’ve wanted multi-line error messages (particularly this ‘mismatched types’ one) for quite some time now, and I know I’m not the only one who does. It is true that long type names don’t play well with word wrapping, but that’s true for the current error messages as well. I believe that multi-line error messages would significantly improve readability of long type names, if done properly. Compare:

<anon>:2:5: 2:10 error: mismatched types:
    expected: `core::option::Option<SomeReallyLongTypeNameWith<Lots, Of, Generic, Parameters, Too>>`
    found:    `core::option::Result<SomeReallyLongTypeNameWith<Lots, Of, Generic, Parameters, Too>, SomeOtherType>`
    (expected enum `core::option::Option`, found enum `core::result::Result`)

And:

<anon>:2:5: 2:10 error: mismatched types: expected `core::option::Option<SomeReallyLongTypeNameWith<Lots, Of, Generic, Parameters, Too>>`, found core::option::Result<SomeReallyLongTypeNameWith<Lots, Of, Generic, Parameters, Too>, SomeOtherType>` (expected enum `core::option::Option`, found enum `core::result::Result`)

With or without word wrap, the former is much easier to read. I honestly don’t see what word wrap has to do with the argument.

---

(An extra compiler flag shouldn’t require an RFC. RFCs are only for language and major library changes: the compiler frontend is unrelated to either of those two.)

[chris-morgan]

@P1start: the closing of this PR doesn’t mean the rejection of the feature, but rather that the issues with the implementation here are too significant for it to be ready for approval soon, and the approach to use for fixing it could do with more attention. For the feature request as a whole there is #18946. Closing this PR is a reasonable action.

[mdinger]

@P1start: I tend to agree but think a flag to enable optionally would sidestep the issue completely and be more likely to land. As yet, I also don't know how it interacts with Vim/Emacs so that may be better single-line also.

[mdinger]

@kballard: Here are some longer strings of errors. Overall, I think the types are much easier to decipher in After but it is easier to distinguish separate errors in Before (but still not difficult). Aside from macro errors, do you want more elaborate errors or is this what you were interested in?

---

Before:

$ rustc test.rs
test.rs:163:9: 172:10 error: method `next` has an incompatible type for trait: expected enum core::option::Option, found enum core::result::Result [E0053]
test.rs:163         fn next(&mut self) -> Result<&'a str, int> {
test.rs:164             self.curr = self.next;
test.rs:165             
test.rs:166             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
test.rs:167                 self.next = if self.all.char_at(self.next) == '(' { close }
test.rs:168                 else { open }
            ...
test.rs:166:21: 166:31 error: mismatched types: expected `core::result::Result<uint,int>`, found `core::option::Option<<generic #6>>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:166             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
                                ^~~~~~~~~~
test.rs:166:33: 166:44 error: mismatched types: expected `core::result::Result<uint,int>`, found `core::option::Option<<generic #7>>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:166             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
                                            ^~~~~~~~~~~
test.rs:171:40: 171:76 error: mismatched types: expected `core::result::Result<&'a str,int>`, found `core::option::Option<&str>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:171             if self.curr != self.len { Some(self.all[self.curr..self.next]) } else { None }
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.rs:171:86: 171:90 error: mismatched types: expected `core::result::Result<&'a str,int>`, found `core::option::Option<<generic #19>>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:171             if self.curr != self.len { Some(self.all[self.curr..self.next]) } else { None }
                                                                                                 ^~~~
test.rs:207:14: 207:18 error: mismatched types: expected `core::result::Result<uint,int>`, found `core::option::Option<uint>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:207             (open, close)
                         ^~~~
test.rs:207:20: 207:25 error: mismatched types: expected `core::result::Result<uint,int>`, found `core::option::Option<uint>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:207             (open, close)
                               ^~~~~
test.rs:212:21: 212:31 error: mismatched types: expected `core::result::Result<uint,int>`, found `core::option::Option<<generic #5>>` (expected enum core::result::Result, found enum core::option::Option)
test.rs:212             if let (Some(open), _) = Parens::find_parens(self.all, 0) {
                                ^~~~~~~~~~
test.rs:212:13: 214:28 error: mismatched types: expected `core::option::Option<&'a int>`, found `core::option::Option<&str>` (expected int, found str)
test.rs:212             if let (Some(open), _) = Parens::find_parens(self.all, 0) {
test.rs:213                 Some(self.all[0..open])
test.rs:214             } else { None }
test.rs:301:40: 301:50 error: mismatched types: expected `Box<translate::Entity>`, found `collections::vec::Vec<<generic #0>>` (expected box, found struct collections::vec::Vec)
test.rs:301         pub fn new() -> Entity { Group(Vec::new()) }
                                                   ^~~~~~~~~~
test.rs:361:43: 361:50 error: type `&mut Box<translate::Entity>` does not implement any method in scope named `push`
test.rs:361                 Group(ref mut vec) => vec.push(e),
                                                      ^~~~~~~
test.rs:368:43: 368:69 error: type `&mut Box<translate::Entity>` does not implement any method in scope named `push`
test.rs:368                 Group(ref mut vec) => vec.push(Inner(s.to_string())),
                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to 12 previous errors

---

After:

$ ./rustc test.rs
test.rs:162:9: 171:10 error: method `next` has an incompatible type for trait: expected `enum core::option::Option`,
    found `enum core::result::Result` [E0053]
test.rs:162         fn next(&mut self) -> Result<&'a str, int> {
test.rs:163             self.curr = self.next;
test.rs:164             
test.rs:165             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
test.rs:166                 self.next = if self.all.char_at(self.next) == '(' { close }
test.rs:167                 else { open }
            ...
test.rs:165:21: 165:31 error: mismatched types:
 expected `core::result::Result<uint, int>`,
    found `core::option::Option<_>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:165             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
                                ^~~~~~~~~~
test.rs:165:33: 165:44 error: mismatched types:
 expected `core::result::Result<uint, int>`,
    found `core::option::Option<_>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:165             if let (Some(open), Some(close)) = Parens::find_parens(self.all, self.next) {
                                            ^~~~~~~~~~~
test.rs:170:40: 170:76 error: mismatched types:
 expected `core::result::Result<&'a str, int>`,
    found `core::option::Option<&str>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:170             if self.curr != self.len { Some(self.all[self.curr..self.next]) } else { None }
                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.rs:170:86: 170:90 error: mismatched types:
 expected `core::result::Result<&'a str, int>`,
    found `core::option::Option<_>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:170             if self.curr != self.len { Some(self.all[self.curr..self.next]) } else { None }
                                                                                                 ^~~~
test.rs:206:14: 206:18 error: mismatched types:
 expected `core::result::Result<uint, int>`,
    found `core::option::Option<uint>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:206             (open, close)
                         ^~~~
test.rs:206:20: 206:25 error: mismatched types:
 expected `core::result::Result<uint, int>`,
    found `core::option::Option<uint>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:206             (open, close)
                               ^~~~~
test.rs:211:21: 211:31 error: mismatched types:
 expected `core::result::Result<uint, int>`,
    found `core::option::Option<_>`
(expected `enum core::result::Result`,
    found `enum core::option::Option`)
test.rs:211             if let (Some(open), _) = Parens::find_parens(self.all, 0) {
                                ^~~~~~~~~~
test.rs:211:13: 213:28 error: mismatched types:
 expected `core::option::Option<&'a int>`,
    found `core::option::Option<&str>`
(expected `int`,
    found `str`)
test.rs:211             if let (Some(open), _) = Parens::find_parens(self.all, 0) {
test.rs:212                 Some(self.all[0..open])
test.rs:213             } else { None }
test.rs:300:48: 300:58 error: mismatched types:
 expected `Box<translate::Entity>`,
    found `collections::vec::Vec<_>`
(expected `box`,
    found `struct collections::vec::Vec`)
test.rs:300         pub fn new() -> Entity { Entity::Group(Vec::new()) }
                                                           ^~~~~~~~~~
test.rs:360:51: 360:58 error: type `&mut Box<translate::Entity>` does not implement any method in scope named `push`
test.rs:360                 Entity::Group(ref mut vec) => vec.push(e),
                                                              ^~~~~~~
test.rs:367:51: 367:85 error: type `&mut Box<translate::Entity>` does not implement any method in scope named `push`
test.rs:367                 Entity::Group(ref mut vec) => vec.push(Entity::Inner(s.to_string())),
                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to 12 previous errors

[mdinger]

I'm not sure how to proceed...a compiler flag might hide the issue but would require everyone who uses it to signal it somehow but cargo doesn't support arbitrary flags. This leaves globally enabling it.

I'm gonna try to check on Vim integration soon. I still don't have any macros that I can test it against though.

Another option is to tab out the newlines until after test.rs:2: to aid with understanding huge error sets like so:

$ ./rustc test.rs
test.rs:2:5: 2:10 error: mismatched types:
           expected `core::option::Option<int>`,
              found `core::result::Result<_, _>`
          (expected `enum core::option::Option`,
              found `enum core::result::Result`)
test.rs:2     Ok(7) // Should be Some(7)
              ^~~~~
error: aborting due to previous error

Though, it doesn't seem so bad in the previous comment.

---

Oh, @mahkoh: I don't understand the #16845 that you linked in case that was directed at me.

[mdinger]

@tomjakubowski I'm doing something wrong with Vim...every time I do :make I see errors. As soon as I hit [ or ], the errors go away. It doesn't just tab through them.

I assume I installed the vim plugins properly by just copying the folders from src/etc/vim/ to ~/.vim/ but am not sure. There aren't any instructions.

[ftxqxd]

@mdinger I think you need to run :cw after running :make to open up the quickfix window.

[mdinger]

@P1start That works. Thanks. [q and ]q still don't work but :cnext and :cprevious do.

[mdinger]

@tomjakubowski Vim seems to work fine here. :cn and :cp took me to the next and previous errors respectively without any issues.

---

I'm not sure how else I can move this (or this type of error messaging) forward. I don't think compiler flags will help.

[passcod]

Regarding cargo compatibility (--flags can't be passed), envvars work fine, and can be made permanent by the user using their shell's rc file.

[tomjakubowski]

@mdinger Great! Thanks for looking into that.