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

Invalid code when formatting matches!() expressions with leading pipe also for the first alternative #5860

Open
tsdh opened this issue Jul 26, 2023 · 1 comment · May be fixed by #5554
Open
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.

Comments

@tsdh
Copy link

tsdh commented Jul 26, 2023

When I format the following code using rustfmt src/cmds.rs (with rustfmt 1.5.2-stable (8ede3aa 2023-07-12)) I get invalid code.

Original valid code:

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(
            self,
            | SwayrCommand::GetWindowsAsJson { .. }
            | SwayrCommand::ForEachWindow { .. }
        )
    }
}

Invalid code after formatting:

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(self, |SwayrCommand::GetWindowsAsJson { .. }| {
            SwayrCommand::ForEachWindow { .. }
        })
    }
}

The issue seems to be caused by the pipe | before the first alternative which is probably uncommon but legal.


I hoped I could fix the subjectively strange indentation of matches!() where the 2nd to last alternative are indented much more than the first that way. I.e., that's what I get now

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(
            self,
            SwayrCommand::GetWindowsAsJson { .. }
                | SwayrCommand::ForEachWindow { .. }
                | SwayrCommand::ForEachWindow2 { .. }
                | SwayrCommand::ForEachWindow4 { .. }
        )
    }
}

and what would please me more would be

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(
            self,
            SwayrCommand::GetWindowsAsJson { .. }
            | SwayrCommand::ForEachWindow { .. }
            | SwayrCommand::ForEachWindow2 { .. }
            | SwayrCommand::ForEachWindow4 { .. }
        )
    }
}

and by prefixing even the first alternative with a pipe, I would get

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(
            self,
            | SwayrCommand::GetWindowsAsJson { .. }
            | SwayrCommand::ForEachWindow { .. }
            | SwayrCommand::ForEachWindow2 { .. }
            | SwayrCommand::ForEachWindow4 { .. }
        )
    }
}

which is as aesthetically pleasing as the wished-for version.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 31, 2023

I hoped I could fix the subjectively strange indentation of matches!() where the 2nd to last alternative are indented much more than the first that way. I.e., that's what I get now

Support for matches isn't perfect. see #5547 and the other linked issues.

For what it's worth, I've already started work to improve matches! formatting in #5554. I started working on that before the formation of the official style team, so I'll likely need to work with them to make sure the formatting is correct before we move forward.

I also just double checked and the code changes proposed in #5554 produce the following with your first input snippet:

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(self, SwayrCommand::GetWindowsAsJson { .. } | SwayrCommand::ForEachWindow { .. })
    }
}

The larger input snippet is formatted as:

impl SwayrCommand {
    fn is_scripting_command(&self) -> bool {
        matches!(
            self,
            SwayrCommand::GetWindowsAsJson { .. }
            | SwayrCommand::ForEachWindow { .. }
            | SwayrCommand::ForEachWindow2 { .. }
            | SwayrCommand::ForEachWindow4 { .. }
        )
    }
}

@ytmimi ytmimi linked a pull request Jul 31, 2023 that will close this issue
@ytmimi ytmimi added bug Panic, non-idempotency, invalid code, etc. a-macros labels Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants