Add test for #8855

[smoelius]

Fix #8855

Here is what I think is going on.

First, the expression format!("{:>6} {:>6}", a, b.to_string()) expands to:

{
    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, 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 to FormatArgsExpn::parse than should.

Those expressions are then visited, but no ::core::fmt::rt::v1::Arguments are found and pushed here.

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

[rust-highfive]

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

[flip1995]

Thanks for this. As you pointed out in #9349 this is closely related to the other format/write/print PRs. So I'll mark this a blocked until those are merged and will then test if this fix is still necessary or if the rewrite already resolved it somehow.

[bors]

☔ The latest upstream changes (presumably #9349) made this pull request unmergeable. Please resolve the merge conflicts.

[smoelius]

#9349 does seem to have resolved the issue.

I think the only question that remains is whether you would like this PR's test.

[flip1995]

Ah great! Thanks for revisiting this. I would have gone back to this once the write.rs rewrite was merged. We definitely want the regression test.

@bors r+

[bors]

📌 Commit 6f3d398 has been approved by flip1995

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6f3d398 with merge 41309df...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 41309df to master...