Rustfmt: Spaces before colons in macro_rules?

[stjepang]

Before formatting:

macro_rules! foo {
    ($a:ident : $b:ty) => {};
    ($a:ident $b:ident $c:ident) => {};
}

After formatting:

macro_rules! foo {
    ($a: ident: $b: ty) => {};
    ($a: ident $b: ident $c: ident) => {};
}

This formatting makes the macro patterns difficult to read.

I understand why rustfmt adds a space after every :, and it makes a lot of sense when arguments are separated by commas or semicolons. But if arguments are separated by spaces or other kinds of delimiters, the end result is not great. :(

cc @RReverser

[RReverser]

I also agree it's not perfect when it's not a list of comma-separated arguments but that's how it was implemented before me in initial version https://github.com/rust-lang-nursery/rustfmt/commit/f86f6dcd9c33fa7b2b1bcda29edbed4d4fd9cba2 by @nrc , so I decided not to change that bit when extending support for macros 1.1/1.0.

[topecongiro]

As much as I prefer to keep formatting consistent between function definitions and macro_rules, IMHO we should not add a space after : by default, because it makes parsing arguments by human eyes harder. Unlike function definitions where arguments are separated by commas, arguments for macros can be separated by anything, or even there could be no separators.
Also I rarely find a code where a space is added after comma in the wild.

[stjepang]

This was fixed by adding a space before the colon. But I wonder: should we add spaces before some other characters, too?

Here it sort of looks like ident is a function name because paratheses are sticked to it:

($name: ident($($dol: tt $var: ident)*) $($body: tt)*) => {};

But if we add a space, it's clearer that ident is actually the type of $name:

($name: ident ($($dol: tt $var: ident)*) $($body: tt)*) => {};

I'd probably prefer to always add a space after the token type, unless it is followed by one of ,;)]}.

[RReverser]

I'd probably rather not add spaces neither before nor after column (which seems what @topecongiro agreed with above but implemented differently?).

That way rustfmt would be consistent with most of the existing macro code in the wild, including the formatting in the Rust Book that probably inspired everyone else. https://doc.rust-lang.org/book/first-edition/macros.html

[dtolnay]

Reopening because the commit that closed this did not address where the discussion seemed to be heading -- that as much as it may be motivated by the formatting of function definitions, macro syntax has different requirements and we should not be inserting whitespace after the metavariable colon. I agree with @topecongiro's and @RReverser reasoning.

macro_rules! foo {
    ($a:ident : $b:ty) => {};
    ($a:ident $b:ident $c:ident) => {};
}

[petrochenkov]

I always reformat macros as functions when I touch them in code (and use appropriate lower/upper-case for macro parameters), it makes them looking much more like normal code and therefore more readable for a non-macro-expert.

I agree with @stjepang though that colons used as separators and not as a part of $param: type should have an extra space, i.e.

macro_rules! foo {
    ($a: ident : $T: ty) => {};
    ($a: ident $b: ident $c: ident) => {};
}

including the formatting in the Rust Book that probably inspired everyone else

We should fix the book, IMO.

[topecongiro]

@dtolnay The first PR adds a space before non-metavariable colon which comes after metavariable. The following PR (#2549) removes a space after metavariable colon. So the issue is fixed on the master.

We have not settled on how we should format macro arguments. I think that we have three options to choose from:

1. Never put a space after metavariable colon ($a:ident).
2. Always put a space after metavariable colon ($a: ident).
3. Heuristically choose whether to put a space after metavariable colon (e.g. put a space if metavariables are separated by commas).

Currently rustfmt follows 1.

[dtolnay]

Thank you! That PR had not been linked to this issue before. Looking forward to a release.

[aturon]

Related to https://github.com/rust-lang-nursery/rustfmt/issues/2753, in that we may want to punt on this formatting via a flag for now.

[nrc]

Removing from milestone since we no longer format macro matchers by default (90c5792565d937b871d0767dd4fbfc317afab707)

[RReverser]

we no longer format macro matchers by default

@nrc Why not? The attached commit just mentions #2543 but that seems unrelated to macro matchers.

[nrc]

So we can decide on the formatting later - there is no clear answer here and we're approaching the cut-off for 1.0, so I think we need to punt.

[RReverser]

I see. Too bad, I really wanted macros to be autoformatted (which is why I PRd it in the first place) but understandable that exact rules need more care and time. I will still be able to opt-in to their formatting with rustfmt.toml in the meanwhile, right?

[RReverser]

Oh wait, if I'm reading the diff correctly, this also affects just the matching definitions, but bodies will be still formatted?

[nrc]

this also affects just the matching definitions, but bodies will be still formatted?

That's the defaults, yes. You'll be able to change both in rustfmt.toml

[RReverser]

Sounds perfect and reasonable, thanks.