Skip to content

Enhance error reporting for write!/writeln! macros #139371

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reez12g
Copy link
Contributor

@reez12g reez12g commented Apr 4, 2025

Closes #139051

@rustbot
Copy link
Collaborator

rustbot commented Apr 4, 2025

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 4, 2025
@reez12g
Copy link
Contributor Author

reez12g commented Apr 4, 2025

@rustbot label -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2025
@rust-log-analyzer

This comment has been minimized.

);
*err.long_ty_path() = file;
err.note("type does not implement the `write_fmt` method");
err.help("try adding `use std::fmt::Write;` to bring the trait into scope");
Copy link
Member

@bjorn3 bjorn3 Apr 4, 2025

Choose a reason for hiding this comment

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

The user could also have intended to use std::io::Write.

@reez12g reez12g marked this pull request as ready for review April 5, 2025 03:56
@reez12g
Copy link
Contributor Author

reez12g commented Apr 5, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 5, 2025
Copy link
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

This doesn't fix the issue imo. This PR just causes a bunch of dead code further down that happens to include the diagnostic that points to the body of the macro. Also, the call to

self.suggest_missing_writer(rcvr_ty, rcvr_expr)
is now unreachable - that's a really helpful error message that shouldn't be disabled.

I think the problem is elsewhere, and the method that recommend similar names (or the caller(s) of it) isn't checking whether it's in a (foreign) macro expansion.

Comment on lines -29 to +31
note: must implement `io::Write`, `fmt::Write`, or have a `write_fmt` method
--> $DIR/missing-writer.rs:5:12
|
LL | write!("{}_{}", x, y);
| ^^^^^^^
help: a writer is needed before this format string
--> $DIR/missing-writer.rs:5:12
|
LL | write!("{}_{}", x, y);
| ^
= note: type does not implement the `write_fmt` method
= help: try adding `use std::fmt::Write;` or `use std::io::Write;` to bring the appropriate trait into scope
= note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a regression, bringing the Write trait into scope doesn't fix the problem because the problem is that the user forgot to even use a writer.

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: type does not implement the `write_fmt` method
= help: try adding `use std::fmt::Write;` or `use std::io::Write;` to bring the appropriate trait into scope
Copy link
Contributor

Choose a reason for hiding this comment

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

MyStruct doesn't implement either Write, so this doesn't help. The point of showing an error message that suggests importing traits is that we know the type implements the trait, but that the user cannot call its methods because the trait is not in scope. That's not the case here.

@reez12g
Copy link
Contributor Author

reez12g commented Apr 6, 2025

Thank you for the review. I'll reconsider the fix, including the points you mentioned.

@wesleywiser wesleywiser added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2025
@bors
Copy link
Collaborator

bors commented May 25, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing suggestions for error[E0599]: cannot write into String
8 participants