Suggest use "{}", self.x instead of {self.x} when resolve x as field of self

[xizheyin]

Fixes #141136

Changes can be seen in the second commit: 9de7fff

r? compiler

[Kivooeo]

Thanks for your fix!
Can we also add tests for println! macro, to make sure that we handling scenarious like println!("{x}") as well

[xizheyin]

println! and format! are actually the same, though the additions do get more comprehensive. :)

[Kivooeo]

Would you mind squashing the commits into one? It helps keep the history cleaner.

Also, while reviewing this, I noticed a small inconsistency in how similar errors are reported for other macros. For example:

Current format! output (really helpful):

error[E0425]: cannot find value `x` in this scope
  --> $DIR/sugg-field-in-format-string-issue-141136.rs:10:31
   |
LL |         let _ = format!("{}", x);
   |                               ^
   |
help: you might have meant to use the available field
   |
LL |         let _ = format!("{}", self.x);

Meanwhile, println! gives:

error[E0425]: cannot find value `x` in this scope
  --> $DIR/sugg-field-in-format-string-issue-141136.rs:11:20
   |
LL |         println!("{x}");
   |                    ^
   |
   = help: you might have meant to use the available field in a format string: `"{}", self.x`

The format! one shows a concrete suggestion inline, while the println! one is more subtle and easy to miss.

This isn’t a blocker for this PR at all, but it got me thinking—maybe in the future we could consider applying similar diagnostics to other macros like writeln!, eprintln!, format_args!, assert!, or panic!. Could be a great follow-up if you're interested (or to track in a separate issue).

Thanks again for the work on this!

[xizheyin]

Committing test in the first commit and seeing the commit changes in the second commit will make reviewing easier. Maybe we can wait for @nnethercote to review before squash.

Additionally, for let _ = format!("{}", x);, inline is given because it originally used the “{}”, x format, while println(“{x}”) is not. Note the placement of x.

[Kivooeo]

Oh, I see now, but I still wonder if this possible to make suggestion like this by any chance

  |
6 |         let _ = format!("{x}");
  |                           ^
  |
help: you might have meant to use the available field
  |
6 |         let _ = format!("{}", self.x);
  | 

[xizheyin]

Thank you for your observations. It's not easy because once the parameters become more numerous, we have to deal with the multi-parameter case, which will be much more complicated. For the inline case, I observe that it is almost always changed in-place because it is easy to show the diff. if it is not changed in-place, the diff will be more confusing. I think the current situation is sufficient.

[nnethercote]

This is looking good to me, I will r+ once the suggestions are made. But I definitely want to keep the commits separate; it's helpful for review and also for anyone who might be looking at the commit history in the future.

[xizheyin]

Maybe it is Ok. @rustbot ready

[nnethercote]

@bors r+

[bors]

📌 Commit 84f67a5 has been approved by nnethercote

It is now in the queue for this repository.

[fmease]

What happens on

struct Foo { x: i32 }

impl Foo {
    fn foo(&self) { _ = {x}; }
}

Does it essentially suggest fn foo(&self) { _ = "{}", self.x } (sic!)?

[fmease]

I've opened issue #141350 to track this regression.

[xizheyin]

It will give this message.

error[E0425]: cannot find value `x` in this scope
 --> src/main.rs:4:26
  |
4 |     fn foo(&self) { _ = {x}; }
  |                          ^
  |
  = help: you might have meant to use the available field in a format string: `"{}", self.x`

For further improvement, below two suggestions may be better?

The first suggestion is

help: you might have meant to use the available field if in a format string: `"{}", self.x`

and keep the origin suggestion

help: you might have meant to use the available field
  |
4 |     fn foo(&self) { _ = {self.x}; }
  |                          +++++

[fmease]

Yeah, the message as emitted right now (on master) is not correct. It should indeed suggest you might have meant to use the available field as we do on beta/stable.