Skip to content

Batch of improvements to errors for new error format #33619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 16, 2016

Conversation

sophiajt
Copy link
Contributor

This is a batch of improvements to existing errors to help get the most out of the new error format.

  • Added labels to primary spans (^^^) for a set of errors that didn't currently have them
  • Highlight the source blue under the secondary notes for better readability
  • Move some of the "Note:" into secondary spans+labels
  • Fix span_label to take &mut instead, which makes it work the same as other methods in that set

@rust-highfive
Copy link
Contributor

r? @jroesch

(rust_highfive has picked a reviewer for you, use r? to override)

@sophiajt
Copy link
Contributor Author

cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

span,
E0411,
"use of `Self` outside of an impl or trait");
err = err.span_label(span, &format!("Used outside of impl or trait"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this should probably not start w/ a capital letter

@nikomatsakis
Copy link
Contributor

r=me with nit fixed

more than once at a time",
nl, new_loan_msg)
.span_label(
let mut err =struct_span_err!(self.bccx, new_loan.span, E0499,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after =

@sophiajt
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented May 15, 2016

📌 Commit 65cb5f7 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented May 15, 2016

⌛ Testing commit 65cb5f7 with merge 5ebe418...

bors added a commit that referenced this pull request May 15, 2016
…ikomatsakis

Batch of improvements to errors for new error format

This is a batch of improvements to existing errors to help get the most out of the new error format.

* Added labels to primary spans (^^^) for a set of errors that didn't currently have them
* Highlight the source blue under the secondary notes for better readability
* Move some of the "Note:" into secondary spans+labels
* Fix span_label to take &mut instead, which makes it work the same as other methods in that set
@bors bors merged commit 65cb5f7 into rust-lang:master May 16, 2016
self.cmt_to_string(&err.cmt)),
format!("mut {}", snippet));
db.span_label(span,
&format!("use `mut {}` here to make mutable", snippet));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaced span_suggestion with span_label, which is undesirable. See #33691.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should chat about lint implications. For the actual error, moving it to a label text appears to improve readability "at a glance" of the errors. That said, we may want to distinguish them in some way that lints and autocorrection type tools can find and use them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think span_suggestion should be used so that machines can read them, but it should be rendered like label for humans.

@sophiajt sophiajt deleted the improve_structured_errors branch May 19, 2016 12:17
Centril added a commit to Centril/rust that referenced this pull request Apr 17, 2019
Resolve inconsistency in error messages between "parameter" and "variable".

The inconsistency was introduced in 104fe1c (rust-lang#33619), when a label saying `type variable` was added to an error with a message talking about `type parameters`.
Given that `parameter` is far more prevalent when referring to generics in the context of Rust, IMO it should be that in both the message and the label.

r? @nikomatsakis or @estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants