Don't prefix full path specification when suggesting changing & to &mut.

[iago-lito]

(moved (again) from rust-lang/rust#79405)

With the following code:

fn f(vec: &Vec<usize>) {
    vec.push(5);
}

I get a suggestion to "change this to a mutable reference", but if I do, I get:

fn f(vec: &mut std::vec::Vec<usize>) {
    vec.push(5);
}

That's a bit overkill, right? I would rather expect

fn f(vec: &mut Vec<usize>) {
    vec.push(5);
}

It actually becomes worse. I've decided to file this bug report after my argument:

shells: &LocatedShells,

was changed to

shells: &mut topology::located_slice::LocatedBoxedSlice<std::collections::HashMap<usize, shell::Shell, std::hash::BuildHasherDefault<fnv::FnvHasher>>>,

instead of just

shells: &mut LocatedShells,

That's definitely overkill, so I end up changing the & to &mut by hand.
Is this something expected? Or something I can configure not to happen?

[ebroto]

Clippy does not have the ability to add imports, so we generally suggest the fully-qualified path to avoid generating compile errors.

That being said, if I'm not mistaken rustc used to have a similar problem which was solved somehow (e.g. Vec is not fully-qualified anymore if not needed) so we could check how this was solved upstream.

If we manage to use the same solution, it should be applied generally as this lint is not the only case where we do that.

[iago-lito]

Well, I'm curious about the process here, because I don't understand why the ability to add imports interferes :(

My understanding is that types are checked before mutability. If this is correct, then this lint appearing already means that the type is correctly qualified, so there is no need to query the current namespace, and just changing & to &mut should never generate a compile error, right? What am I missing?

[ebroto]

Oh my bad, ignore me, I confused this case with the case where we suggest something that may not be on scope.

This should be doable.

[xFrednet]

I could reproduce the suggestion with the full type specification in the Rust Playground. The message that this suggestion most likely originates from is:

error[E0596]: cannot borrow `*me` as mutable, as it is behind a `&` reference
 --> src/main.rs:3:5
  |
2 | fn not_using_mut(me: &Vec<u8>) {
  |                      -------- help: consider changing this to be a mutable reference: `&mut Vec<u8>`
3 |     me.push(4);
  |     ^^ `me` is a `&` reference, so the data it refers to cannot be borrowed as mutable

error: aborting due to previous error

I couldn't find the message in the clippy code base. However, I found it in the rustc code in diagnostics/mutability_errors.rs line 390. So, I would think that this actually comes from the rust compiler. (I've also commented this on the rust issue)

[iago-lito]

@xFrednet Well, I'm confused, since I don't actually read the full type spec in your playground, or in your snippet, I read &mut Vec<u8> where full type spec would be &mut std::vec::Vec<u8>.

This said, I can reproduce in your playground if I eventually click clippy, so I think the problem does belong to this repo, here's what happens to me:

• navigate to your playground
• click Run
  • -> error appears: consider changing this to be a mutable reference: `&mut Vec<u8>` : simple type spec.
• click Tools -> Clippy
  • -> error appears: consider changing this to be a mutable reference: `&mut std::vec::Vec<u8>` : full type spec.

So I think the problem does not occur with rustc while it does occur with clippy.

[xFrednet]

Ohhh yes you're completely right. I mixed up the output from Run and Tools -> Clippy because they looked so similar. Thank you for pointing that out @iago-lito!

Than it's most likely a problem with Clippy or with the combination of the two. I'll try to look into this in a week or two. Someone else can also take this issue if it sounds interesting :)

Update: I didn't look further into this. This issue can be taken up if someone is interested