Add implementation of match arm alignment

[Ruin0x11]

This is a draft of the "match_align_arms" functionality described in #894. Regression tests are included.

[nrc]

Thanks for implementing this, it looks pretty good! I have a few high level comments:

I'd prefer to avoid a Preserve option. As a rule I have found it creates difficulties to take into account the original formatting rather than relying purely on the abstract representation. I see the motivation for Preserve - some times vertical alignment makes more sense than others. However, I think we could do better. Perhaps use a heuristic for the maximum amount of whitespace introduced? Or apply only to matches where every arm is single line? It might be worth looking at some examples of where you definitely don't want vertical alignment and seeing if that suggests a rule.

I'm a bit surprised by how much whitespace is introduced. I would expect that the longest pattern always gets exactly one space before the expression, e.g.,

match lorem {
    Lorem::Ipsum => (),
    Lorem::Dolor => (),
    Lorem::Sit   => (),
    Lorem::Amet  => (),
}

Is there a reason not to?

[Ruin0x11]

Yes, I agree that it should always reformat to the minimum amount of spacing. I'll make that change. As for Preserve, I agree that it does make sense to use a heuristic to not reformat sometimes, like in this case:

match lorem {
    Lorem::Ipsum => {
        lorem();
        ipsum();
    },
    Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
    Lorem::Eiusmod => {
        lorem();
        ipsum();
    },
}

It would add a lot of unnecessary whitespace to the other statements.

[Ruin0x11]

I replaced Preserve with a Conservative option and added the config option match_arm_align_threshold. Now if a match statement has patterns with a difference in width greater than match_arm_align_threshold (default 10) with the Conservative option, they won't be reformatted:

match lorem {
    Lorem::Ipsum => (),
    Lorem::Dolor => (),
    Lorem::Sit => (),
    Lorem::Amet => (),
}

match lorem {
    Lorem::Ipsum => {
        lorem();
        ipsum();
    },
    Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
    Lorem::Eiusmod => {
        lorem();
        ipsum();
    },
}

becomes:

match lorem {
    Lorem::Ipsum => (),
    Lorem::Dolor => (),
    Lorem::Sit   => (),
    Lorem::Amet  => (),
}


match lorem {
    Lorem::Ipsum => {
        lorem();
        ipsum();
    },
    Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
    Lorem::Eiusmod => {
        lorem();
        ipsum();
    },
}

[nrc]

Could we have just a single config option like match_arm_align_threshold - 0 would mean never align, a large number (say == max_width) would mean always align, and anything in between would be conservative?

[Ruin0x11]

Sure, sounds good to me.

[nrc]

Rather than add this to Shape, can you pass this as an extra arg to the relevant functions?

[nrc]

@topecongiro would you be able to give this a review please?

@Ruin0x11 also needs rebasing before we can merge

[topecongiro]

@nrc Sure!
@Ruin0x11 I have not tested myself, but I wonder whether the BytePos based alignment works even when the patterns are multi-lined. Could you add tests for those cases?

[Ruin0x11]

I added the tests, and as expected they don't. However I'm wondering what the best way of getting the relative position is using only each arm.

[topecongiro]

@Ruin0x11 Do you have any updates?
I am afraid that using span to calculate the alignment is not a good idea, since the horizontal length may change after rewriting the pattern. The following snippet is an example.

    let x = match op {
        Some(Foo(  ..  )) => 0,
        None => 1,
    };

I think we have to first rewrite all the patterns and calculate the alignment using those results.

[Ruin0x11]

Yes, that was the same conclusion I came to. This turned out to be way harder than I imagined.

[topecongiro]

@Ruin0x11 In case you are not interested in to this anymore, please let me know.

[Ruin0x11]

I think I'll open another PR when I can resolve the above issues.