Rust: Suggest changing `_x` to `x` instead of a use of `x` to `_x`

[oli-obk]

Variables can be marked as unused by prepending with an underscore. If at some point such a variable is used again, the compiler suggests to name the use _x, even though every single time I have encountered this, what I wanted is to change the definition of _x to x.

fn main() {
    let _x = 42;
    let y = x;
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0465]: multiple rlib candidates for `debug_unreachable` found
  |
  = note: candidate #1: /playground/target/debug/deps/libdebug_unreachable-aa3385f4476cae47.rlib
  = note: candidate #2: /playground/target/debug/deps/libdebug_unreachable-661505f3db863158.rlib

error[E0425]: cannot find value `x` in this scope
 --> src/main.rs:3:13
  |
3 |     let y = x;
  |             ^ help: a local variable with a similar name exists: `_x`

error: aborting due to 2 previous errors

Some errors occurred: E0425, E0465.
For more information about an error, try `rustc --explain E0425`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

[estebank]

I feel we should have a counterpart to the unused lint that checks for variables that are used but use the unused bar convention. That wouldn't be useful for the case presented here where you're using an unused bar for the first time, but seems like a reasonable addition.

[oli-obk]

Such a lint is problematic within macros or other compiler-expanded code, since it's a common pattern to work around not knowing if there are any uses by naming the variables appropriately (for an example look at the for loop lowering code)

[estebank]

@oli-obk I agree but I'm pretty sure that _most_ lints _should_ ignore code with spans in macro context already.

[xldenis]

Hi, I'd like to work on this!

[oli-obk]

@xldenis sorry for not responding earlier. I only have very broad instructions right now, but don't hesitate to ask here or on the wg-diagnostics stream on zulip if you need more help.

1. find the place where the diagnostic "a local variable with a similar name exists" is emitted
   
   
   
   • note that the "local variable" part may be a placeholder {} and the rest of the string might be broken across multiple lines
   
   
2. see if that place has more info than just the name of the local variable
3. if not, add a Span to whatever datastructure holds the name
   
   
   
   • I'm not sure how the name is obtained right now, but there should be a way to get at the Span there, too. E.g. if the name is an Ident, then it has a Span field.
   
   
4. when emitting the "similar name" diagnostic, first check whether the suggested name starts with an underscore, but the name used in the code does not
5. if that chek fails, keep the old message
6. otherwise, emit the suggestion not on the span that it is currently emitted on, but on the definition span (and change the suggestion to remove the underscore)

[xldenis]

Hi @oli-obk I've already done most of that :) I think I'll have a working PR next week.

[saleemjaffer]

@xldenis @oli-obk I can work on this if you guys are caught up with something else :)

[oli-obk]

@saleemjaffer sure. Just @rustbot claim once you start working on it. Maybe you can even get the WIP branch from @xldenis fork