error-msg: impl better suggestion for E0532

[Ezrashaw]

Fixes #106862

No test as there is already a test which is nearly identical to the example in the linked issue.

[rustbot]

r? @WaffleLapkin

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

[WaffleLapkin]

Also I would like to see a test where

• Some names are the same (e.g. Foo(x, a)/struct Foo { x: (), y: () })
• There are patterns that are not simply bindings (e.g. Foo(Some(x)), struct Foo { field: Option<()> })
• The pattern is for an enum (e.g. Enum::Var(x), enum Enum { Var { a: () } })

[Ezrashaw]

@WaffleLapkin I've added a test with all your cases. One of the suggestions was poor (Foo { x: x }), it's been updated.

[WaffleLapkin]

Hm, I thought about another test case: what about the case, when the pattern and the struct have different number of fields? i.e. struct S { x: u32, y: u32 }/S(a); struct S { field: u32 }/S(x, y); struct S {}/S(a); and struct S { field: u32 }/S().

If the struct has more fields than the pattern, we could either suggest .. or excess_field0: _, excess_field1: _ (not sure what's better).

Also, is this code run on expressions too? If so, we should test it too (although it seems like expressions go in PathSource::Expr?... it's unclear from the docs and hard to guess while reviewing on gh)

[Ezrashaw]

@WaffleLapkin The current behaviour for:

• when the tuple has more fields (eg struct S { field: u32 }/S(x, y) is just to drop the excess fields (eg help: use struct pattern syntax instead: `S { field: x }`), which I think is ok.
• when the tuple is missing fields (eg struct S { x: u32, y: u32 }/S(a)) is to ignore extra struct fields, obviously this is bad. I'd suggest just putting them in as S { field1: x, field2, field3 }. People might want to use those fields and IMHO we shouldn't preemptively ignore them; worst case is a unused warning.

Also, is this code run on expressions too?

No? Maybe in some cases? It is quite confusing, although in the expression position the following code:

fn main() {
    struct S { x: u32, y: u32 }
    
    let a = 5;
    let b = 5;
    let x = S(a, b);
}

creates the nonsensical suggestion:

error[E0423]: expected function, tuple struct or tuple variant, found struct `S`
 --> <anon>:6:13
  |
2 |     struct S { x: u32, y: u32 }
  |     --------------------------- `S` defined here
...
6 |     let x = S(a, b);
  |             ^^^^^^^
  |
help: use struct literal syntax instead
  |
6 |     let x = S { x: val, y: val };
  |             ~~~~~~~~~~~~~~~~~~~~
help: a local variable with a similar name exists
  |
6 |     let x = a(a, b);
  |             ~

(note that this suggestion is separate from what this PR touches)

[bors]

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

[Ezrashaw]

Implemented "when the tuple is missing fields". Also rebased and updated tests.

I think that removing the span_to_snippet will be difficult because you have to compare the old and new field; you can't just suggest around the fields.

[WaffleLapkin]

I think that removing the span_to_snippet will be difficult because you have to compare the old and new field; you can't just suggest around the fields.

Alright, I see. I won't block this PR on removing span_to_snippet then.

[Ezrashaw]

@WaffleLapkin Whoops, thought I had already done that but however.

@rustbot ready

[Ezrashaw]

@WaffleLapkin Done. Thanks for all the feedback.

[WaffleLapkin]

@bors r+ rollup

[bors]

📌 Commit bd17322 has been approved by WaffleLapkin

It is now in the queue for this repository.