Skip to content
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

format_macro_matchers unusual behavior with commas #5984

Open
tgross35 opened this issue Dec 21, 2023 · 3 comments
Open

format_macro_matchers unusual behavior with commas #5984

tgross35 opened this issue Dec 21, 2023 · 3 comments
Labels
a-macros only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@tgross35
Copy link
Contributor

tgross35 commented Dec 21, 2023

For a struct-like macro:

macro_rules! foo {
    (
        foo: $foo:literal,
        bar: $bar:literal,
        baz: $baz:literal,
        qux: $qux:literal,
        quux: $quux:literal,
        corge: $corge:literal,
    ) => {};
}

Formatting with format_macro_matchers = true produces weird results:

macro_rules! foo {
    (
        foo:
        $foo:literal,bar:
        $bar:literal,baz:
        $baz:literal,qux:
        $qux:literal,quux:
        $quux:literal,corge:
        $corge:literal,
    ) => {};
}

It seems like it always removes whitespace after a comma rather than using it as a possible break location. Single-line macros have similarly weird results:

macro_rules! foo {
    (foo: $foo:ident,bar: $bar:ident,baz: $baz:ident,qux: $qux:ident,) => {};
}

rustfmt 1.7.0-nightly (f704f3b9 2023-12-19)

@ytmimi ytmimi added poor-formatting a-macros only-with-option requires a non-default option value to reproduce labels Dec 21, 2023
@ytmimi
Copy link
Contributor

ytmimi commented Dec 21, 2023

Linking the tracking issue for format_macro_matchers (#3354)

@InsertCreativityHere
Copy link

InsertCreativityHere commented Apr 15, 2024

Problem

This seems to be caused by the intersection of 2 things:

an assumption in the macro parsing code, where we always insert a 'separator' before parsing a meta-variable ($foo):

rustfmt/src/macros.rs

Lines 907 to 910 in 7289391

// We always want to add a separator before meta variables.
if !self.buf.is_empty() {
self.add_separator();
}
This 'forces' the formatter to try and keep the $idents at the start of each line.

and, what seems to me, like a bug in the token rules. Running the provided snippet through the macro parser gives the following tokens.

[ParsedMacroArg { kind: Delimited(Parenthesis, [
    ParsedMacroArg { kind: Separator("foo:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "foo") },
    ParsedMacroArg { kind: Separator(", bar:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "bar") },
// below tokens are just repititions...
    ParsedMacroArg { kind: Separator(", baz:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "baz") },
    ParsedMacroArg { kind: Separator(", qux:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "qux") },
    ParsedMacroArg { kind: Separator(", quux:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "quux") },
    ParsedMacroArg { kind: Separator(", corge:", "") },
    ParsedMacroArg { kind: MetaVariable("literal", "corge") },
    ParsedMacroArg { kind: Other(",", "") }
]) }]

What Happens

I believe these are the root of the problem:
ParsedMacroArg { kind: Separator(", bar:", "") },

This expression (, bar:) should be getting parsed into a Separator(", ") and an Other("bar:"), but they're not.
And because of this the entire thing gets treated like it's just a comma.

This is why when it's rewriting the expression, it emits: $foo:literal,bar:
Because as far as it knows it's writing $foo:literal,.

The solution (Part of it at least)

I'm going to try and update the token rules, so that , bar: gets parsed like I suggested:
into Separator(", ") and an Other("bar:").

@ytmimi
Copy link
Contributor

ytmimi commented Apr 15, 2024

@InsertCreativityHere thanks for taking the time to look into this, and for the clear explanation of the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
Development

No branches or pull requests

3 participants