Extra whitespace in nested pub extern

[rillian]

The following snippet formats with an extra space between pub and extern "C" if the arguments continue across multiple lines. i.e. rustfmt produces pub␣␣extern "C" instead of pub␣extern "C".

pub struct SomeCallback(
    pub extern "C" fn( 
        long_argument_name_to_avoid_wrap: u32,
        second_long_argument_name: u32,
        third_long_argument_name: u32,
    ),
);

Checked with

• rustfmt 1.5.1-stable (4b91a6e 2022-08-08) from 1.63.0 stable
• rustfmt 1.5.1-nightly (c07a8b4 2022-08-26) from a recent nightly

[ytmimi]

For anyone interested in working on this here's a little bit of what I found:

The Rewrite impl for ast::FieldDef is implemented by calling rewrite_struct_field.

To calculate the field_str, rewrite_struct_field calls rewrite_assign_rhs and from what I can tell that's where the error is. https://github.com/rust-lang/rustfmt/blob/38659ec6ad5f341cf8eb3139725bf695872c6de7/src/items.rs#L1784

Following the calls made after rewrite_assign_rhs you'll get to rewrite_assign_rhs_with, which just concatenates the lhs and rhs. In this case the lhs is 'pub ' and the rhs is ' extern "C" ...'

https://github.com/rust-lang/rustfmt/blob/38659ec6ad5f341cf8eb3139725bf695872c6de7/src/expr.rs#L1977-L1988

[jmj0502]

@ytmimi I would like to work on this issue! ✌️

[jmj0502]

@rustbot claim

[jmj0502]

@ytmimi in regard to this issue, I noticed that there are several tests with the following format:

 struct MyTuple(
    #[cfg(unix)] // some comment
    pub  u64,
    #[cfg(not(unix))] /*block comment */ pub(crate)  u32,
 );

Is that alright? Or should I correct such tests?

[ytmimi]

@jmj0502 Normally changing existing tests isn't something we want to do, but in this case it seems like the bug slipped into those tests. I think it should be fine to update the affected tests, but there's a chance we need to version gate the fix.

[jmj0502]

@ytmimi I understand 👍! I asked because the issue truly seems like a bug and there are only two affected tests. However, I think the version gate fix is the way to go, since that won't "break" the format of any existing project.

[daprilik]

Just ran into this myself.

type ThisIsAVeryLongTypeNameThatWillCauseWrappingToKickIn = usize;

#[derive(serde_derive::Serialize)]
pub struct Foo(
    #[serde(rename = "ThisIsAVeryLongTypeNameThatWillCauseWrappingToKickIn")]
    pub  ThisIsAVeryLongTypeNameThatWillCauseWrappingToKickIn,
);

Is this something that'll get fixed via https://github.com/rust-lang/rustfmt/pull/5529 or https://github.com/rust-lang/rustfmt/pull/5708?

[ytmimi]

@daprilik I just checked and https://github.com/rust-lang/rustfmt/pull/5708 should resolve your issue. At the moment the team is focused on higher priority issues like let-else formatting, so unfortunately I can't give a clear answer for when a fix will be released.

The linked PR is currently waiting on a review. Although maintainers are the only ones who can merge approved PRs anyone in the community is welcome to review and provide feedback on any PR 😁