Improve string_to_string lint in case it is in a map call

[GuillaumeGomez]

Fixes #14175.

There are two "big" cases to handle: .map(|x| x.to_string()) and .map(String::to_string). If the closure has more than one expression, we should not suggest .cloned().

changelog: Improve string_to_string lint in case it is in a map call

r? @samueltardieu

[GuillaumeGomez]

Finally fixed dogfood. :D

[samueltardieu]

Thanks, this is an nice new message.

One thing that bothers me a bit is the HIR node to which the new message applies. Shouldn't it be associated with the .map() call node, since this is what the message suggests to rewrite with .cloned()?

At some point, we'll need the node anyway if we are going to propose an autofix.

[GuillaumeGomez]

One thing that bothers me a bit is the HIR node to which the new message applies. Shouldn't it be associated with the .map() call node, since this is what the message suggests to rewrite with .cloned()?

At some point, we'll need the node anyway if we are going to propose an autofix.

I kept the current formatting: it's always underlining the caller. I can send a follow-up to fix that if you want?

[samueltardieu]

I kept the current formatting: it's always underlining the caller.

Yes, but it also only proposed to modify the caller itself, never the enclosing map expression.

I can send a follow-up to fix that if you want?

I would say that the message probably belongs to the .map() call. If the .map() is linted for any other reason and we switch to autofixes, we may have problems.

[GuillaumeGomez]

I updated to emit a suggestion on the method call.

[samueltardieu]

@GuillaumeGomez Did you miss the proposal in #14396 (comment)? Right now, s.map(|x| { x.to_string() }) will suggest x.clone(), while it could suggest s.cloned(), as if the block was not present.

[GuillaumeGomez]

I completely missed it, sorry. Updating.

[GuillaumeGomez]

Added the suggested improvement, thanks!

[flip1995]

Some additional comments.

[GuillaumeGomez]

So in the end, I just removed the comment in the ui test. ^^'