Why does clippy want two to_string() calls in a row?

help
1

I'm Rust newbie and am trying to understand how to use ansi_term to create a string that contains a few different escape sequences.

I have the following code:

use ansi_term::ANSIString;
use ansi_term::Colour::{Blue, Red};

fn main() {
    let some_value = format!("{:b}", 42);

    let strings: &[ANSIString<'static>] = &[
        Red.paint("["),
        Blue.bold().paint(some_value),
        Red.paint("]"),
    ];

    println!("{}", get_display(strings));
}

fn get_display(strings: &[ANSIString<'static>]) -> String {
    let mut display = String::new();
    for s in strings {
        // Double to_string() isn't strictly necessary, but otherwise clippy complains
        display.push_str(&s.to_string());
        display.push('\n');
    }
    display
}

and, while it works, when I run cargo clippy on it, I get the following:

warning: unnecessary use of `to_string`
  --> src/main.rs:19:26
   |
19 |         display.push_str(&s.to_string());
   |                          ^^^^^^^^^^^^^^ help: use: `s`
   |
   = 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

warning: `ansi-color-test` (bin "ansi-color-test") generated 1 warning

Of course, if I remove the to_string() the code works, but I don't get ansi escape sequences, so no color.

If, on the other hand, I add another to_string(), everything works and clippy doesn't complain.

What is going on? From Type Definition ansi_term::ANSIString, I found "Although not technically a string itself, it can be turned into one with the to_string method."

(Since I'm a newbie, feel free to point me to a better place to ask this question.)

2

I feel it's just clippy being stupid here, since it might somehow deduce s is similiar to str, and of course you should replace &s.to_string() with s if that's the case.

Sidenode: your code actually could avoid s.to_string. You can write write!(&mut display, "{}\n", s) instead.

2 Likes
3

Minimum repo

use std::{fmt::Display, ops::Deref};

fn main() {
    let f = Foo;

    foo(&f.to_string())
}

fn foo(s: &str) {
    println!("{}", s);
}

struct Foo;

impl Deref for Foo {
    type Target = str;

    fn deref(&self) -> &Self::Target {
        "str"
    }
}

impl Display for Foo {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "Foo")
    }
}

/* This will also do.
impl Foo {
    fn to_string(&self) -> &str {
        "Foo"
    }
}
*/

Clippy senses Foo deref to str, so it emitted warning on &f.to_string(). But with Foo implementing Display, the deref doesn't happen, so it's false alarm. Consider file an issue to clippy.

2 Likes
4

Thanks!

Your minimum repo produces a similar error, but I'm not sure it is the same. Adding another to_string() in this case does not silence clippy, but causes the following output:

warning: redundant clone
 --> src/main.rs:6:23
  |
6 |     foo(&f.to_string().to_string())
  |                       ^^^^^^^^^^^^ help: remove this
  |
note: this value is dropped without further use
 --> src/main.rs:6:10
  |
6 |     foo(&f.to_string().to_string())
  |          ^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
  = note: `#[warn(clippy::redundant_clone)]` on by default

Nevertheless, it does show that the first to_string() is necessary even though clippy complains about it. I'll submit it for that bug.

1 Like
5

I think you can get rid of get_display completely. According to the ansi_term documentation here, you can construct multiple ANSIString instances in an ANSIStrings wrapper. Here would be your example:

use ansi_term::{ANSIString, ANSIStrings};
use ansi_term::Colour::{Blue, Red};

fn main() {
    let some_value = format!("{:b}", 42);

    let strings: &[ANSIString<'static>] = &[
        Red.paint("["),
        Blue.bold().paint(some_value),
        Red.paint("]"),
    ];

    println!("{}", ANSIStrings(strings));
}

This should fix your problem with clippy and hopefully produce the output you desire.

-- Playground

2 Likes
6

@jofas, you are right, of course, that the get_display() was not needed. I was using it to test if ANSIString would behave the way I wanted by inserting the CR between the individual ANSIString structs.

7

btw, the github bug is here: erroneous "unnecessary use of to_string"

2 Likes
8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.