Auto merge of #8857 - smoelius:fix-8855, r=flip1995

Add test for #8855

Fix #8855

Here is what I think is going on.

First, the expression `format!("{:>6} {:>6}", a, b.to_string())` expands to:
```rust
{
    let res =
        ::alloc::fmt::format(::core::fmt::Arguments::new_v1_formatted(&["",
                            " "],
                &[::core::fmt::ArgumentV1::new_display(&a),
                            ::core::fmt::ArgumentV1::new_display(&b.to_string())],
                &[::core::fmt::rt::v1::Argument {
                                position: 0usize,
                                format: ::core::fmt::rt::v1::FormatSpec {
                                    fill: ' ',
                                    align: ::core::fmt::rt::v1::Alignment::Right,
                                    flags: 0u32,
                                    precision: ::core::fmt::rt::v1::Count::Implied,
                                    width: ::core::fmt::rt::v1::Count::Is(6usize),
                                },
                            },
                            ::core::fmt::rt::v1::Argument {
                                position: 1usize,
                                format: ::core::fmt::rt::v1::FormatSpec {
                                    fill: ' ',
                                    align: ::core::fmt::rt::v1::Alignment::Right,
                                    flags: 0u32,
                                    precision: ::core::fmt::rt::v1::Count::Implied,
                                    width: ::core::fmt::rt::v1::Count::Is(6usize),
                                },
                            }], unsafe { ::core::fmt::UnsafeArg::new() }));
    res
}
```
When I dump the expressions that get past the call to `has_string_formatting` [here](b312ad7d0c/clippy_lints/src/format_args.rs (L83)), I see more than I would expect.

In particular, I see this subexpression of the above:
```
                &[::core::fmt::ArgumentV1::new_display(&a),
                            ::core::fmt::ArgumentV1::new_display(&b.to_string())],
```

This suggests to me that more expressions are getting past [this call](b312ad7d0c/clippy_lints/src/format_args.rs (L71)) to `FormatArgsExpn::parse` than should.

Those expressions are then visited, but no `::core::fmt::rt::v1::Argument`s are found and pushed [here](b312ad7d0c/clippy_utils/src/macros.rs (L407)).

As a result, the expressions appear unformatted, hence, the false positive.

My proposed fix is to restrict `FormatArgsExpn::parse` so that it only matches `Call` expressions.

cc: `@akanalytics`

changelog: none
This commit is contained in:
bors 2022-08-20 18:02:34 +00:00
commit 41309df8ef
3 changed files with 55 additions and 1 deletions

View file

@ -122,3 +122,27 @@ fn issue8643(vendor_id: usize, product_id: usize, name: &str) {
name
);
}
// https://github.com/rust-lang/rust-clippy/issues/8855
mod issue_8855 {
#![allow(dead_code)]
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);
dbg!(x);
let x = format!("{:>6} {:>6}", a, b.to_string());
dbg!(x);
}
}

View file

@ -122,3 +122,27 @@ fn issue8643(vendor_id: usize, product_id: usize, name: &str) {
name
);
}
// https://github.com/rust-lang/rust-clippy/issues/8855
mod issue_8855 {
#![allow(dead_code)]
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);
}
}

View file

@ -126,5 +126,11 @@ error: `to_string` applied to a type that implements `Display` in `println!` arg
LL | println!("{foo}{bar}", bar = "bar", foo = "foo".to_string());
| ^^^^^^^^^^^^ help: remove this
error: aborting due to 21 previous errors
error: `to_string` applied to a type that implements `Display` in `format!` args
--> $DIR/format_args.rs:142:38
|
LL | let x = format!("{} {}", a, b.to_string());
| ^^^^^^^^^^^^ help: remove this
error: aborting due to 22 previous errors