Fix code suggestion for local enum patterns in non-exhaustive matches

[makai410]

Fixes: #137682 .

The fixed version of the output be like:

error[E0004]: non-exhaustive patterns: `make_index::Shadowed::Foo` not covered
 --> reprod.rs:6:11
  |
6 |     match v {}
  |           ^ pattern `make_index::Shadowed::Foo` not covered
  |
note: `make_index::Shadowed` defined here
 --> reprod.rs:8:10
  |
8 |     enum Shadowed {
  |          ^^^^^^^^
9 |         Foo,
  |         --- not covered
  = note: the matched value is of type `make_index::Shadowed`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
  |
6 ~     match v {
7 +         Shadowed::Foo => todo!(),
8 ~     }
  |

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0004`.

I guess only adjusting the code suggestion is better because make_index::Shadowed::Foo is more specific than Shadowed::Foo in this case.

[rustbot]

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
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]

Some changes occurred in match checking

cc @Nadrieril

[compiler-errors]

This is not the right approach. It's not going to work right when the pattern is nested into another type like (Shadowed::Foo,). String manipulation doesn't have enough context to fix a suggestion like this.

You probably want to modify something a bit more close to the root of the problem, like https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_pattern_analysis/rustc/print.rs.html#47-126.

Also, there is no test here.

[compiler-errors]

@rustbot author

[rustbot]

Some changes occurred in exhaustiveness checking

cc @Nadrieril

[makai410]

This is not the right approach. It's not going to work right when the pattern is nested into another type like (Shadowed::Foo,). String manipulation doesn't have enough context to fix a suggestion like this.

You probably want to modify something a bit more close to the root of the problem, like https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_pattern_analysis/rustc/print.rs.html#47-126.

Also, there is no test here.

Thanks! I just changed to a new approach that will force trimming paths (by with_forced_trimmed_paths!) if the scrutinee and the enum have the same "parent" by checking the def_id. Still unsure if there is anything inappropriate. Criticism is welcome :).

[makai410]

@rustbot ready

[Nadrieril]

Can you document what this function is meant to do?

[makai410]

Suggested change

	fn erase_path_if_local<'tcx>(
	/// Determines whether to trim the enum path.
	///
	/// This decision is based on whether the `scrutinee` expression and
	/// the enum are within the same function or method.
	fn trim_paths_if_local<'tcx>(

[Nadrieril]

Doc looks good, I would name it something like should_trim_enum_paths.

[Nadrieril]

I'm not super familiar with how lint scopes work in THIR, but this feels very un-robust: there seem to be many reasons for this expression not to be a scope. Can you not use self.match_lint_level (which is the hir id of the match I believe)?

[makai410]

This function wants a scope expression surrounding the scrutinee, like:

match v {}
      ^ -- scope surrounding this

Then we can get the def id of the scrutinee's owner.

Sorry that I can't find an approach to get the def id by not using lint_level here. :(

[Nadrieril]

Pretty sure a match and its scrutinee always have the same owner def_id; you should be able to use self.match_lint_level.owner.to_def_id().

[Nadrieril]

This feels quite ad-hoc: the underlying issue is that def_path_str does not give a path that can be used in code. Shouldn't there be a function that can return usable paths for a target def_id in the context of a source def_id?

[makai410]

My idea here is that if the scrutinee and the enum both have the same owner (which means they are at the same function or method), then trimming the path is ok because we just want the enum name here(I guess).

[makai410]

Oh perhaps I got what you were worrying about, cc #137845.
It seems that they try to suggest a trimmed paths name in code only requiring that the name is unique, even if the things are in a different module.
Once users apply these code suggestions, then just let the compiler emit the suggestions like "use xxx;" to take users to the right way, which may be the idea of the design (btw it's just my guess, i ain't investigated it for now).

So here the problem is that the path is totally wrong because it is a function name which should not appear.

Idk if I got to the right way but I appreciate it if you're willing to point my problems out.

[Nadrieril]

My worry is that this is a very special-cased fix, where the general underlying issue is "how to emit correct paths in suggestions". Maybe @estebank you would know about this? What's the correct way to get a path that can be used in a suggestion?

[makai410]

I see... No clues at hand right now. But would be happy to be mentored to solve this.