assigning_clones complains about static str -> String assignment.

[eric-seppanen]

Summary

The assigning_clones lint fires when I assign to a String using a const &str and .to_owned().

The suggested replacement is harder to read and obfuscates the meaning of the code.

Lint Name

assigning_clones

Reproducer

I tried this code:

const INIT_FOO: &str = "foo";

pub struct Foo {
    x: String,
}

impl Foo {
    pub fn load(&mut self) {
        self.x = INIT_FOO.to_owned();
    }
}

I saw this happen:

warning: assigning the result of `ToOwned::to_owned()` may be inefficient
 --> src/lib.rs:9:9
  |
9 |         self.x = INIT_FOO.to_owned();
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `INIT_FOO.clone_into(&mut self.x)`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assigning_clones
  = note: `#[warn(clippy::assigning_clones)]` on by default

I expected to see this happen:

Nothing.

Version

rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-unknown-linux-gnu
release: 1.78.0
LLVM version: 18.1.2

Additional Labels

No response

[GnomedDev]

Is this not more efficient though, as self.0's buffer can be reused and potentially save a free -> alloc?

[eric-seppanen]

Yes, there may be an efficiency gain, but the suggested change hurts the code readability a lot. I can't imagine trying to teach a new Rust progammer to use INIT_FOO.clone_into(&mut self.x). I would like to recommend clippy when teaching people Rust, but if clippy's default lints suggest things that make the language harder to learn then that feels like a bad tradeoff.

Additionally, in some cases (e.g. self.0 is currently an empty String), there is no allocation to be reused. If the programmer knows this then the lint is just noise.

[y21]

FWIW, this lint was recently moved to pedantic (allow-by-default) for that reason. See also #12778 (and linked issues/PRs).

I'm not sure what can be done here though now that it's in pedantic. Pedantic lints are ones that when you do blanket-#![warn(clippy::pedantic)] them, that you are expected to also allow certain ones you don't agree with and thats fine.

The lint is not wrong here. It could be more efficient when the string has enough capacity. So when someone enables this lint and wants to be warned by it, I imagine they would also like to be warned for &'static strs. The readability argument made here sounds like it applies to the lint in general and not to static strs specifically?

[eric-seppanen]

@y21, thanks, I wasn't aware of the recent move to pedantic (#12779). For future reference, assigning_clones is warn by default in 1.78.

[y21]

I don't know if it's too late but we might be able to beta-nominate/backport the category change PR so that it makes it into 1.79. If it doesn't get beta backported, it will stay warn-by-default until 1.80 most likely (which is in two months).