Skip to content

to_string() in a format string when format specifiers are used (eg :<20) is NOT redundant #8855

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

Closed
akanalytics opened this issue May 20, 2022 · 1 comment · Fixed by #8857
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

Comments

@akanalytics
Copy link

akanalytics commented May 20, 2022

Summary

It is not true that
a) println!("{:<20}", struct.to_string());
gives the same result as
b) println!("{:<20}", struct)

It depend on how the formatter is written.
Line a) uses the default "{}" display to render a string then renders the string using the formatting rules :<20
Whereas b) uses the formatting rules of the struct's display, not the string's display trait impl.

Many dsiplay impls for objects do not have logic to cater for alignment for instance, but using struct.to_string() and then formatting with alignment format specifiiers DOES give alignment.

Clippy "fixing" such lines results in behavioural change.

I think clippy should flag only when the format is "{}"

Lint Name

to_string_in_format_args

Reproducer

I tried this code:

     write!(
                f,
                "{:>6} {:>10} {:>3} {:>2}",
                self.bm.uci(),
                self.score.to_string(),
                self.depth,
                self.nt
            )

I saw this happen:

6   | #![warn(clippy::perf)]
    |         ^^^^^^^^^^^^
    = note: `#[warn(clippy::to_string_in_format_args)]` implied by `#[warn(clippy::perf)]`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args

warning: `to_string` applied to a type that implements `Display` in `println!` args
   --> src/comms/bench.rs:118:19
    |
118 |                 cp.to_string(),
    |                   ^^^^^^^^^^^^ help: remove this

This is related to #7729

Version

rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0

Additional Labels

No response

@akanalytics akanalytics 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 May 20, 2022
@matthiaskrgr
Copy link
Member

Interesting 🙂

test:

struct A {}

impl std::fmt::Display for A {
  fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        write!(f, "test")
    }
}

fn main() {

let a = A{};
let b = A{}; 

let x = format!("{} {}", a, b.to_string());
dbg!(x);

let x = format!("{:>6} {:>6}", a, b.to_string());
dbg!(x);

}
[src/main.rs:15] x = "test test"
[src/main.rs:18] x = "test   test"

smoelius added a commit to smoelius/rust-clippy that referenced this issue Aug 20, 2022
@bors bors closed this as completed in 41309df Aug 20, 2022
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants