Borrow/BorrowMut auto importing completions are confusing for newcomers

[Veykril]

People tend to use the autoimporting trait method completion for borrow/borrow_mut when they meant to use the inherent method completion for RefCell causing confusion as the method now changes meaning.

See #8468 (comment) for previous discussions

[flodiebold]

Making it harder to accidentally autoimport BorrowMut or other traits shadowing inherent methods is one thing (I think we should complete a qualified method call with the full qualified path in such cases, and maybe even more importantly downrank that completion heavily). Maybe we can also do something for the case where someone already imported BorrowMut though? At least we should, when completing an inherent method that's shadowed by some trait method, complete the qualified method call. And maybe we could have a note diagnostic with an assist that suggests to remove the import and replace the qualified method call by a normal one.

[flodiebold]

So to clarify, I would see the following possible action items:

• detect when a trait autoimport would shadow an inherent method and downrank those completions
• detect when a trait autoimport would shadow an inherent method and instead complete a fully qualified method call (e.g. foo.borrow_<|> -> std::borrow::BorrowMut::borrow_mut(foo))
• detect when there's an inherent method currently shadowed by a trait method, and complete the fully qualified method call instead (e.g. when there's use std::borrow::BorrowMut; already, provide RefCell::borrow_mut(foo) as a completion)
• when using the UFCS -> method call assist, detect if the method call would be shadowed and remove e.g. the BorrowMut import automatically (maybe only if it's unused)

[leddoo]

suggestion: rank all completions that do require an import after the ones that don't.
imo, that's the common case, reusing something that's already available.

this would resolve the borrow issue, as the borrow method on RefCell is already accessible. using Borrow::borrow would require an import.
(NB: if Borrow is also in scope, you could consider the "inherent methods over trait methods" completion ordering, but that's not necessary to resolve this specific issue, as we're talking about confusing auto-imports)

it would also resolve the std::intrinstics::unreachable issue, where that unsafe function is imported (in an unsafe block afaict) instead of suggesting the already available unreachable!() macro.

implementation wise, i don't know how the ranking algorithm works, but i suspect these two borrow functions have the same priority. in which case, "doesn't require an import" could be used as a tie breaker.

[kornelski]

Here someone ended fighting the borrow checker with &mut Rc because of a wrong .borrow_mut().

This issue doesn't even need to have a proper in-depth fix. It's an exceptionally bad footgun for std::borrow::Borrow/Mut specifically, so RA could just hardcode an exception for these.