rustc_deprecated suggestions don't always cargo fix correctly

[carols10cents]

Rust version: stable 1.51.0

Given the following code that uses the std::str::trim_left function that is marked rustc_deprecated:

fn main() {
    let foo = str::trim_left("   aoeu");
    
    let bar = "   aoeu".trim_left();
}

The current output is:

warning: use of deprecated associated function `core::str::<impl str>::trim_left`: superseded by `trim_start`
 --> src/main.rs:2:15
  |
2 |     let foo = str::trim_left("   aoeu");
  |               ^^^^^^^^^^^^^^ help: replace the use of the deprecated associated function: `trim_start`
  |
  = note: `#[warn(deprecated)]` on by default

warning: use of deprecated associated function `core::str::<impl str>::trim_left`: superseded by `trim_start`
 --> src/main.rs:4:25
  |
4 |     let bar = "   aoeu".trim_left();
  |                         ^^^^^^^^^ help: replace the use of the deprecated associated function: `trim_start`

If I run cargo fix on this example, I get this output:

warning: failed to automatically apply fixes suggested by rustc to crate `foo`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

The following errors were reported:
error[E0425]: cannot find function `trim_start` in this scope
 --> src/main.rs:2:16
  |
2 |     let _foo = trim_start("   aoeu");
  |                ^^^^^^^^^^ not found in this scope

The suggestion from the rustc_deprecated attribute applied correctly to the usage on line 4, but it replaced str::trim_left with just trim_start on line 2. I expected it to change this line to let foo = str::trim_start(" aoeu");.

I'm not sure if the solution is to change the ^^^^^^ span marking to the leaf item that's deprecated. It seems like perhaps if the warning instead was:

2 |     let foo = str::trim_left("   aoeu");
  |                    ^^^^^^^^^ help: replace the use of the deprecated associated function: `trim_start`

then perhaps only the trim_left in str::trim_left would be replaced by rustfix? If so, I'd like to try my hand at fixing this. I've read some rustc_deprecated related issues, however, and I'm getting a strong here-be-dragons vibe from them, so I wanted to check with someone more knowledgeable about if I'm on the right track before the yaks eat me.

[jyn514]

It seems like perhaps if the warning instead was:

2 | let foo = str::trim_left(" aoeu");
| ^^^^^^^^^ help: replace the use of the deprecated associated function: trim_start

then perhaps only the trim_left in str::trim_left would be replaced by rustfix?

Yes, I think that would work - but it seems like a slightly less friendly error. If you suggested str::trim_start instead that should fix the new code while keeping the same span.

[jyn514]

Oh, I guess that doesn't work if it's s.trim_start() instead of str::trim_start(&s). In that case, yes, shrinking the span seems like a good approach.

[Rustin170506]

@rustbot claim

I want to try it.

[hellow554]

This one is very interesting and I'm not really sure if it's still E-easy.

So, my first step was to modify

rust/src/test/ui/deprecation/suggestion.rs

Line 27 in 76a04dd

foo.deprecated(); //~ ERROR use of deprecated

and add something like this:

Foo::replacement(&foo); //~ ERROR use of deprecated

Next the

rust/src/test/ui/deprecation/suggestion.fixed

Line 27 in 76a04dd

foo.replacement(); //~ ERROR use of deprecated

should also print out this:

Foo::replacement(&foo); //~ ERROR use of deprecated

but it does not, instead it will "fix" it to:

replacement(&foo); //~ ERROR use of deprecated

---

So, I'm not entirely sure how to fix this.
First and foremost the span is already provided to

rust/compiler/rustc_middle/src/middle/stability.rs

Line 284 in 76a04dd

pub fn eval_stability(self, def_id: DefId, id: Option<HirId>, span: Span) -> EvalResult {

This is the first point where I'm not sure if that is the correct way. Should eval_stabilty figure out what exactly is depreacted (e.g. the function, the struct, the mod), or should the caller do it?

[hellow554]

hey @hi-rustin how are you doing on this?

[Rustin170506]

hey @hi-rustin how are you doing on this?

Yes, I would like to solve it. But I haven't had a chance to code it yet, so if you already have a solution you can just submit a PR. It's OK for me.

[Rustin170506]

If you want to submit a PR you can let me know and I can unassign myself.

[hellow554]

I haven't done anything yet but the initial post ;) So you can work on it as you like.

[Rustin170506]

I have found a solution to get only the span of the method that already works. I will submit the patch later.