Add NoEmptyLinesOpeningClosingBraces rule

[mateusrodriguesxyz]

This rule removes empty lines after opening braces and before closing braces:

// original

struct P {
            
    let x: Int
  
    let y: Int
          
}

// formatted

struct P {  
    let x: Int
  
    let y: Int   
}

That's similar to vertical_whitespace_opening_braces and vertical_whitespace_closing_braces.

It's a style used in official swift projects such as swift-syntax and sourcekit-lsp.

[allevato]

My initial reaction to this was that I would like to keep rules involving whitespace inside the pretty printer (at least that was my plan when I got around to implementing this rule one day, which I've wanted to do for a while, so thanks for saving me the trouble!). But, that might end up being more complex looking at the logic there, so maybe this is fine the way it is—it removes the blank lines before the pretty printer ever sees them, which does the job.

[allevato]

It's too bad we don't have a convenient way to just visit anything that conforms to BracedSyntax here.

Can you add PrecedenceGroupDeclSyntax and SwitchExprSyntax, which look like the only two other BracedSyntax node types that are missing here?

[ahoppen]

To visit anything conforming to BracedSyntax you can do the following

public override func visitAny(_ node: Syntax) -> Syntax? {
  if let braced = node.as(BracedSyntax.self) {
    // do things
  } else {
    super.visitAny(node)
  }
}

[allevato]

In general that would work for a regular visitor, but generate-swift-format scans the rules to figure out which concrete nodes they visit so that it can generate the pipeline visitor.

It might not be difficult to add visitAny support to the pipeline, but it would be better to do it separately from this PR.

[ahoppen]

👍🏽 Just wanted to put this pattern out there because it’s not terribly obvious.

[mateusrodriguesxyz]

I've wished more than once that override func f(value: some T) in a class would override any concrete method that matches.

[allevato]

This will change the behavior of the default configuration (and also any existing configuration that doesn't explicitly reference the new rule). Is that something you intend to do with this PR?

Adding public override class var isOptIn: Bool { return true } would be safer for now. Then, we should probably come up with a policy for how we want to handle changes to the default configuration in the future.

[mateusrodriguesxyz]

Sorry, It wasn’t my intention.

[allevato]

It looks like this function is never used, only the one with the underscore below is. Delete this and remove the underscore from the other one?

[mateusrodriguesxyz]

Yeah, I forget this 😅

[allevato]

Can you please add a doc comment for this rule using a similar format to the other rules? RuleDocumentation.md should be generated from that when you run generate-swift-format; you don't need to update it manually.

[allevato]

Nit: Update the ending copyright date to 2024 here.

[mateusrodriguesxyz]

@allevato I've updated the PR addressing your comments.

[allevato]

Windows CI is failing because of the new source file. Can you please add Rules/NoEmptyLinesOpeningClosingBraces.swift to Sources/SwiftFormat/CMakeLists.txt?

[mateusrodriguesxyz]

Done!

[allevato]

Thanks!