Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent handling of to_string() removal suggestion between field and method access #13726

Open
ecpost opened this issue Nov 25, 2024 · 0 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@ecpost
Copy link

ecpost commented Nov 25, 2024

Summary

I have 2 values to pass to a function. The first is borrowed (indirectly) from a field in the second, and the function needs a mutable reference of the second. So I am making a copy (to_string()) of the first, to avoid the conflicting borrows. But clippy suggests removing to_string(), which would result in a compiler error.

However, this suggestion only happens if the field is borrowed via a method. If done directly with the field instead of an accessor method, clippy does not make the (invalid) suggestion.

Lint Name

unnecessary_to_owned

Reproducer

Here is a complete example:

struct Something {
    value: String,
    // other fields
}

impl Something {
    fn value(&self) -> &str {
        &self.value
    }
}

fn use_it(_value: &str, _thing: &mut Something) {
    // do stuff
}

fn main() {
    let mut thing = Something { value: "abc".to_string() };

    // This (using field `value` directly) is fine, does not generate a warning:
    use_it(&thing.value.to_string(), &mut thing);

    // But this (using method `value()` to read field `value`) generates a warning:
    use_it(&thing.value().to_string(), &mut thing);

    // Using the suggestion results in a compiler error because of the immutable and mutable borrows together:
    // use_it(thing.value(), &mut thing);
}

The first call to use_it (using the field directly) does NOT result in a warning. But the second call results in:

warning: unnecessary use of `to_string`
  --> src/main.rs:23:12
   |
23 |     use_it(&thing.value().to_string(), &mut thing);
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `thing.value()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
   = note: `#[warn(clippy::unnecessary_to_owned)]` on by default

And the third call to use_it with the suggested change (commented out) would result in a compiler error because of the conflicting borrows.

Version

rustc 1.81.0 (eeb90cda1 2024-09-04)
binary: rustc
commit-hash: eeb90cda1969383f56a2637cbd3037bdf598841c
commit-date: 2024-09-04
host: x86_64-apple-darwin
release: 1.81.0
LLVM version: 18.1.7

Additional Labels

@rustbot label +I-suggestion-causes-error

@ecpost ecpost added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Nov 25, 2024
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

No branches or pull requests

2 participants