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

Or pattern formatting style weirdly inconsistent for lines of the same length #4556

Closed
ratmice opened this issue Nov 25, 2020 · 3 comments
Closed
Labels

Comments

@ratmice
Copy link

ratmice commented Nov 25, 2020

Input
Given lines of the same or longer length but different contributing factors (enum name vs variant field name elem vs element).

impl<T> PartialEq for FooZZZZZZZZZZZZZZ<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (FooZZZZZZZZZZZZZZ::Foo(x), FooZZZZZZZZZZZZZZ::Foo(y))
            | (FooZZZZZZZZZZZZZZ::Foo(x), FooZZZZZZZZZZZZZZ::Bar { count: _, elem: y })
            | (FooZZZZZZZZZZZZZZ::Bar { count: _, elem: x }, FooZZZZZZZZZZZZZZ::Foo(y))
            | (FooZZZZZZZZZZZZZZ::Bar { count: _, elem: x }, FooZZZZZZZZZZZZZZ::Bar { count: _, elem: y })
            => x == y,
        }
    }
}

impl<T> PartialEq for Fooent<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Fooent::Foo(x), Fooent::Foo(y))
            | (Fooent::Foo(x), Fooent::Bar { count: _, elem: y })
            | (Fooent::Bar { count: _, elem: x }, Fooent::Foo(y))
            | (Fooent::Bar { count: _, elem: x }, Fooent::Bar { count: _, elem: y })
            => x == y,
        }
    }
}

impl<T> PartialEq for Foo<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Foo::Foo(x), Foo::Foo(y))
            | (Foo::Foo(x), Foo::Bar { count: _, element: y })
            | (Foo::Bar { count: _, element: x }, Foo::Foo(y))
            | (Foo::Bar { count: _, element: x }, Foo::Bar { count: _, element: y })
            => x == y,
        }
    }
}

Output

impl<T> PartialEq for FooZZZZZZZZZZZZZZ<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (FooZZZZZZZZZZZZZZ::Foo(x), FooZZZZZZZZZZZZZZ::Foo(y))
            | (FooZZZZZZZZZZZZZZ::Foo(x), FooZZZZZZZZZZZZZZ::Bar { count: _, elem: y })
            | (FooZZZZZZZZZZZZZZ::Bar { count: _, elem: x }, FooZZZZZZZZZZZZZZ::Foo(y))
            | (
                FooZZZZZZZZZZZZZZ::Bar { count: _, elem: x },
                FooZZZZZZZZZZZZZZ::Bar { count: _, elem: y },
            ) => x == y,
        }
    }
}

impl<T> PartialEq for Fooent<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Fooent::Foo(x), Fooent::Foo(y))
            | (Fooent::Foo(x), Fooent::Bar { count: _, elem: y })
            | (Fooent::Bar { count: _, elem: x }, Fooent::Foo(y))
            | (Fooent::Bar { count: _, elem: x }, Fooent::Bar { count: _, elem: y }) => x == y,
        }
    }
}

impl<T> PartialEq for Foo<T>
where
    T: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Foo::Foo(x), Foo::Foo(y))
            | (
                Foo::Foo(x),
                Foo::Bar {
                    count: _,
                    element: y,
                },
            )
            | (
                Foo::Bar {
                    count: _,
                    element: x,
                },
                Foo::Foo(y),
            )
            | (
                Foo::Bar {
                    count: _,
                    element: x,
                },
                Foo::Bar {
                    count: _,
                    element: y,
                },
            ) => x == y,
        }
    }
}

Expected output

The last ends up very poor, and should preferably be rendered similarly to the first two blocks.
note that line length on the formatting of the 2nd block actually increases.
I tend to prefer the way that the first/longest block lays out the => expression on the next line,
over the middle which pushes the beginning of the => all the way to column 86

Meta
I looked didn't see a duplicate for this, with #4005 being the closest that I found.

  • rustfmt version: rustfmt 1.4.24-stable (eb894d5 2020-11-05)
  • From where did you install rustfmt?: rustup
@calebcartwright
Copy link
Member

Thank you for reaching out @ratmice. However, this is working as designed. The formatting emitted by rustfmt is governed by the Rust Style Guide which defines explicit formatting prescriptions for all code constructs, and accordingly rustfmt is a pretty-printer style code formatter that reformats each node in the AST according to those respective rules.

The formatting varies here because by changing the struct field name from elem to element which results in the struct literals exceeding the allowed max width for a one line struct width. That in turn requires multilining the tuples which in turn impacts how the match arms have to be formatted.

https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/expressions.md#struct-literals
https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/expressions.md#tuple-literals
https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/expressions.md#match

Going to close this accordingly

@ratmice
Copy link
Author

ratmice commented Nov 29, 2020

@calebcartwright what I don't understand is the lines are literally the same length (in the 2nd and 3rd example), it changes Foo to Fooent and element to elem, so I don't see how the max struct width reasoning here make sense unless it ignores the constructor name for some reason?

@calebcartwright
Copy link
Member

what I don't understand is the lines are literally the same length (in the 2nd and 3rd example), it changes Foo to Fooent and element to elem, so I don't see how the max struct width reasoning here make sense unless it ignores the constructor name for some reason?

The lit width is used to determine the layout of the "body" (e.g. the fields) based on formatted length of the fields/structrest elements; the path (name) doesn't come into the picture for that check but of course does for in other width limit scenarios like total line width, or a containing element width constraint.

By default, use_small_heuristics is enabled which applies the more granular "small" checks to various literal constructs, function calls, etc. The default value for the struct lit width is 18% of the max_width value, and since the default max_width is 100 the default width limit for a struct literal (again, path not relevant at this level) is 18. Unfortunately the struct_lit_width configuration value that's used for these checks is private/internal in the released 1.x versions of rustfmt, so users can't change just that value. However, we have made that option public in the unreleased 2.x version of rustfmt and will probably backport that to a 1.x version that's available via rustup. More information on that option can be found here

count: _, elem: x comes in at 17, under the threshold but count: _, element: y is 20 and thus requires the fields be multilined. If you're still skeptical you can try adding/removing a few characters from those respective field names and/or values, as well as tweaking max_width a bit to see how the formatting will change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants