Implement empty lines surrounding blocks

[richardpringle]

Here's my first shot at solving #5963. Please see the description in that issue to review this PR. I would rather the reviewer have an idea what I'm trying to do before taking a look at my documentation (otherwise, I might miss something in the docs).

I think the code is pretty straight forward, but I might not being following the idioms in this library.

I'm also pretty sure that there must be edge cases that I missed in my testing, please advise if you can think of any.

If the initial review here makes sense, I can go ahead and fill in the documentation and continue from there.

Thanks in advance!

[richardpringle]

Hey @ytmimi, any chance I could bug you to take a quick glance here and tell me if I'm on the right track?

[ytmimi]

I haven't done a thorough review of the code, but I've posed a few questions.

[ytmimi]

All of the test cases here only show one level of nesting. What happens when there are multiple levels of nesting, e.g:

fn main() {
    if true {
        if true {
            if true {
            }
        }
    }
}

[ytmimi]

Do we need to add is_first to get this to work?

[richardpringle]

In my current implementation, yes. I'm not sure if there's a better way to do this, though. Open to suggestions.

[ytmimi]

I don't know if contains_curly_block is the right name for this function. Are you trying to add space around anything with curly braces or just all statements?

[richardpringle]

I'm open to suggestions for names.

Here's the "problem" that I've run into: I've been reviewing code that will have 50 lines without a single line between them. If one looks at any popular open-source repo (including rustfmt), one can see many more empty lines, breaking code up into logical chunks like paragraphs in prose. So the question is, how can a formatter that doesn't have logic context of the code add some of this spacing? Naturally, new scopes (demarcated by curly braces) provide an opportunity to add spacing.

So maybe the name should be something like has_new_scope? Or something along those lines...

[ytmimi]

rustfmt also has the blank_lines_lower_bound and blank_lines_upper_bound configuration options, which seem to operate pretty similar to this option. What happens when using your new option and these other configurations?

[richardpringle]

It behaves as one would expect if the blank_lines_(upper|lower)_bound config takes precedence over surround_blocks_with_empty_lines. I would expect that the bounds take precedence as they affect a larger subset of the codebase. However, maybe that expectation is incorrect, but I couldn't imagine someone setting the upper bound to 0 and using surround_blocks_with_empty_lines, where the former overrides the latter. As for setting a lower-bound greater than 0, I also wouldn't expect its usage together with surround_blocks_with_empty_lines since it actually adds more spacing.