Improve shadowed private field diagnostics

[chenyukang]

Fixes #149546

I tried to extend the diagnostic to more scenarios, like method call, type mismatch errors.

r? @estebank

[estebank]

I would like to add a bit more information. I think that with these small changes then we'll be giving the absolute best information we can. I don't think I could explain it any better in person :)

View changes since this review

[estebank]

I think the only things missing are I'd like to add is modify the message slightly to make it clearer that there's a field on a that is usize which is private, but that there's another field in a deref of a that was ultimately chosen instead, to point at the field we are talking about (maybe even both the bool and the usize with a label) and ideally the impl Deref as well. Something like:

note: there is a field `field` on `A` with type `usize` but it is private; `field` from `B` was accessed through auto-deref instead
   |
LL |     pub struct A {
   |                - in this struct
LL |         field: usize,
   |         ^^^^^ if this field wasn't private, it would be accessible
...
LL |     pub struct B {
   |                - this struct is accessible through auto-deref
LL |         field: bool,
   |         ----- this is the field that was accessed
...
LL |     impl std::ops::Deref for A {
   |     -------------------------- the field was accessed through this `Deref`

The above can be done by taking the span for A::field, calling into on it to turn it into a MultiSpan, then do push_span_label on it with the labels and the secondary spans, and then use that span on a span_note.

All of this adds a bunch of verbosity, but without it I fear that it'll be hard to figure things out. We could consider gating this only on local crates, where the information is most useful, but I feel that one could easily miss the subtle privacy issue when looking at a dependency expecting to be able to access the field, so I'd consider showing this always.

[chenyukang]

yes, seems much clear, the code is updated.

[estebank]

Let's make this an FxHashSet first until we're done pushing HirIds, and then turn it into a Vec that you can sort. Otherwise, as is contains does a linear scan of all the collected ids on every push, which is O(n2) if I'm not mistaken.

[chenyukang]

fixed.

[estebank]

@bors r+

[rust-bors]

📌 Commit ae899cc has been approved by estebank

It is now in the queue for this repository.