Skip to content
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

Add spans to clippy.toml error messages #10607

Merged
merged 1 commit into from
Jun 2, 2023
Merged

Conversation

beetrees
Copy link
Contributor

@beetrees beetrees commented Apr 7, 2023

Adds spans to errors and warnings encountered when parsing clippy.toml.

changelog: Errors and warnings generated when parsing clippy.toml now point to the location in the TOML file the error/warning occurred.

@rustbot
Copy link
Collaborator

rustbot commented Apr 7, 2023

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

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 7, 2023
@beetrees beetrees force-pushed the toml-spans branch 2 times, most recently from 3449295 to 11ed4ac Compare April 8, 2023 13:55
@bors
Copy link
Collaborator

bors commented May 9, 2023

☔ The latest upstream changes (presumably #10751) made this pull request unmergeable. Please resolve the merge conflicts.

@beetrees beetrees force-pushed the toml-spans branch 2 times, most recently from 9fb8f08 to 26e1ec5 Compare May 15, 2023 17:06
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Sorry for the late reviewing. Overall looks good, thanks! I made one comment.

#[derive(Debug)]
struct ConfError(String);
pub struct ConfError(pub String, pub Option<Span>);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it would be easy to understand when accessing fields if its field has name, like error.message and error.span.

Suggested change
pub struct ConfError(pub String, pub Option<Span>);
pub struct ConfError {
pub message: String,
pub span: Option<Span>,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@giraffate Done

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jun 2, 2023

📌 Commit 6f13a37 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 2, 2023

⌛ Testing commit 6f13a37 with merge 50ab3ce...

@bors
Copy link
Collaborator

bors commented Jun 2, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 50ab3ce to master...

@bors bors merged commit 50ab3ce into rust-lang:master Jun 2, 2023
4 checks passed
@beetrees beetrees deleted the toml-spans branch June 3, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants