rustfmt erroneously removes brackets around statement arg in macro

[ColonelThirtyTwo]

Example code:

macro_rules! unwrap_or {
    ($v:expr, $s:stmt) => {
        match $v {
            Some(v) => v,
            None => { $s },
        }
    };
}

fn main() {
    let _asdf = unwrap_or!(Some(123), return);
}

Compiles fine. Running rustfmt 1.5.2-nightly (1e225413 2023-01-28) removes the braces around $s, as such:

macro_rules! unwrap_or {
    ($v:expr, $s:stmt) => {
        match $v {
            Some(v) => v,
            None => $s,
        }
    };
}

fn main() {
    let _asdf = unwrap_or!(Some(123), return);
}

The code no longer compiles:

   Compiling playground v0.0.1 (/playground)
error: expected expression, found `return;`
  --> src/main.rs:5:21
   |
5  |             None => $s,
   |                  -- ^^ expected expression
   |                  |
   |                  while parsing the `match` arm starting here
...
11 |     let _asdf = unwrap_or!(Some(123), return);
   |                 ----------------------------- in this macro invocation
   |
   = note: this error originates in the macro `unwrap_or` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `playground` due to previous error

[ytmimi]

Thanks for reaching out! confirming I can reproduce this using rustfmt 1.5.2-nightly (adfdf1c5 2023-01-26).

To fix this we can prevent flattening match arm's when rewriting macro definitions. We determine if we can flatten_arm_bodies here, and the RewriteContext has a is_macro_def field we can use to return early from flatten_arm_bodies without flattening the block.

[ColonelThirtyTwo]

FWIW in this specific case, I should have been using an expr matcher instead of a stmt, in which case flattening the braces would be OK to do and compiles fine.

I think the braces only need to be preserved if the match arm is a statement token. Though I don't know if rustfmt parses enough of the macro to tell if that's the case or not.

[ytmimi]

@ColonelThirtyTwo thanks for the follow up. rustfmt doesn't use the token specifiers to inform how it'll reformat the code. I think to be safe we shouldn't try to remove the {} when reformatting macro bodies.

[calebcartwright]

My only concern with a broad brush approach here would be modifying rustfmt behavior in a manner that fails to adhere to the style guide. Would want some clarity on that front before moving ahead, as once we make this change we'd be unable to restore or add anything more selective/targeted

[raheja15]

@rustbot claim

[ytmimi]

@raheja15 I'd encourage you to take a look at a different issue since @calebcartwright mentioned wanting to make sure that any changes we make here adhere to the style guide. I don't think that point has been addressed so it would be hard to move forward with a fix right now.

[raheja15]

Hi,

Actually, I was searching for a better and more selective approach to this issue. So, I assigned it to myself.

But yeah, I couldn't find any as far. I will just switch to other issue for now, if more selective approach does not exist.

Thank You

[raheja15]

@rustbot release-assignment