Allow more required line breaks

[natestedman]

Currently, when respectsExistingLineBreaks is set to false, swift-format squishes most of code onto single lines when it will fit under the line length limit. I'd like to use respectsExistingLineBreaks so that output is fully deterministic based on the source code structure, but I've found readability is negative impacted by formatting like this:

func foo(bar: Bool) { if bar { print("Bar!") } }

Instead, I'd prefer more line breaks:

func foo(bar: Bool) {
    if bar {
        print("Bar!")
    }
}

So, I want to add more options to force the addition of line breaks. Specifically, the ones that I'm currently looking at are:

• lineBreakBeforeEachFuncBody: after the opening { of a func, always use a line break.
• lineBreakBeforeEachIfElseBody: after the { following if or else, always use a line break.
• lineBreakBeforeEachLoopBody: same, but for loops.
  • This could be configurable per-loop-type, but would anyone want that?
• lineBreakBeforeEachSwitchCaseOrDefaultBody: after case ...: or default:, always use a line break.

The other constructs I can think of are guard and var. I don't need these, as I think the squished style is more typical for simple, short cases of these, but they could be added as well for the sake of completeness.

I think that this is basically https://bugs.swift.org/browse/SR-13458, though these changes won't add more flexibility, only more required line breaks.

I'm putting this up with just lineBreakBeforeEachSwitchCaseOrDefaultBody so that the idea and the implementation can be discussed, if moving forward, I'll stack the other commits and also add tests (testing locally by formatting my Swift code, it appears to work).

I'm also currently targeting the swift-5.3-branch because I'm using Xcode 12 locally, but I expect later on I'll need to rebase to target the main branch?

I also noticed that running the compiled swift-format, like this:

find Sources -name '*.swift' | xargs ./.build/debug/swift-format -i

...against all of the Swift files made a lot of changes, so I manually formatted the code (just the now-long after call). Is swift-format being used on the repo, and if so, how should I run it?

[allevato]

Thanks for working on this! I think better control of when things get one-lined or not is definitely something we need.

Part of me wonders if we could just make logic smarter somehow, with less manual interference. For example, I think many of the cases that look less-than-ideal, like you mentioned, are when you have nested curly-brace structures that happen to be so short that the whole thing can occur on one line:

func log(_ msg: String) { if (debugging) { print(msg} } }

If the body of this function had multiple statements, this problem resolves itself because there's a hard break between them that forces the break after the open { of the function.

But in this case, there's only a single statement in the function body (we only look at the direct children, so we don't see the nested statements), so that logic never occurs.

So instead of having separate configuration settings for different types of syntactic constructs, I wonder if we should just try to detect the presence of nested curly brace structures and force the break(s) on the outer one(s) if we do. (And also for the body of statements after a case ...: clause; that should be treated as if it has invisible braces around it for the purposes of this logic.)

Is that something you'd be interested in trying? Do you think it would cover the cases you're concerned about?

[natestedman]

So, the main thing that I'd want is deterministic output based on the code structure (thus no respectsExistingLineBreaks), but without the "squishing" that comes with a line limit of 120 (pretty typical for iOS) for simple classes, extensions, etc.

But, I don't know that nested braces alone get exactly my ideal formatting. It's pretty subjective, but single-line functions:

func example(arg: Int) -> Int { return arg }

and single line if:

if something { print("something!") }

Don't feel "normal" (whereas single-line computed var and guard/else do... can't explain it!). Maybe the former is different with implicit single-expression returns from func being allowed now though!

But, it's less important than avoiding the excessive squishing. I think overall I prefer the result of this, but:

1. If the complexity of special-casing so many constructs is too much, a simpler approach is definitely valid, and will get most of the way there.
2. I made them all separate settings, just going off everything existing being a pretty small config option. But, to reduce complexity somewhat (in config, not so much in the implementation), they could just be one "more line breaks!" option to reduce the number of variations in outputs. Depends whether you value consistency or flexibility in swift-format (e.g. black is very inflexible, clang-format is very flexible).

[allevato]

Since there's no "blessed" style at this time, I'm open to some degree of configurability, but I also want to avoid having so much that it becomes difficult to maintain, since the current architecture of the pretty printer is such that some options can interact with each other and that causes a combinatorial explosion of edge cases that we'd need to test thoroughly.

The only real constraints (at this time) are that new features do not affect the behavior of the formatter under the default configuration, so that people using it without a configuration don't have their formatter change suddenly.

It's not exactly what you had in mind, but I think I'd be more inclined to partition curly brace behavior based on whether the braces define the body of a type declaration, the body of a non-type declaration (computed property, function), or the body of a statement (if, while, etc.) and allow customization of those categories. That seems better from the point of view of users defining a configuration, so they don't have to audit each individual structure.

[hfossli]

I want to add tuples to this discussion. I would love to see long unnamed and named tuple arguments break into several lines just as functions do. Why? There's a big movement away from protocols over to structs. Our codebase uses this a lot and many of the tuples are hard to read.

❌ Current result

struct PeopleSearchService {
  var query:
    (
      _ query: String?, _ orderBy: String?, _ ascending: Bool?, _ limit: Int?,
      _ offset: Int?
    ) -> [String]
}

struct CarSearchService {
  var query:
    (
      _ args: (
        query: String?, orderBy: String?, ascending: Bool?,
        limit: Int?, offset: Int?
      )
    ) -> [String]
}

✅ Wanted result

struct PeopleSearchService {
  var query:
    (
      _ query: String?,
      _ orderBy: String?,
      _ ascending: Bool?,
      _ limit: Int?,
      _ offset: Int?
    ) -> [String]
}

struct CarSearchService {
  var query:
    (
      _ args: (
        query: String?,
        orderBy: String?,
        ascending: Bool?,
        limit: Int?,
        offset: Int?
      )
    ) -> [String]
}

See full example here

{
  "respectsExistingLineBreaks": false,
  "lineBreakBeforeControlFlowKeywords": true,
  "lineBreakBeforeEachArgument": true,
  "lineLength": 80,
  "maximumBlankLines": 1,
  "respectsExistingLineBreaks": false,
  "prioritizeKeepingFunctionOutputTogether": false,
  "lineBreakAroundMultilineExpressionChainComponents": true,
  "rules": {
    "BlankLineBetweenMembers": false,
    "MultiLineTrailingCommas": true,
    "OneCasePerLine": true,
    "OneVariableDeclarationPerLine": true,
    "OnlyOneTrailingClosureArgument": true,
    "UseSingleLinePropertyGetter": true,
  },
  "tabWidth": 4,
  "version": 1
}

protocol CompanySearchService {
  func query(
    query: String?,
    orderBy: String?,
    ascending: Bool?,
    limit: Int?,
    offset: Int?
  ) -> [String]
}

struct MyCompanySearchService: CompanySearchService {
  func query(
    query: String?,
    orderBy: String?,
    ascending: Bool?,
    limit: Int?,
    offset: Int?
  ) -> [String] { return ["a", "b", "c"] }
}

struct PeopleSearchService {
  var query:
    (
      _ query: String?, _ orderBy: String?, _ ascending: Bool?, _ limit: Int?,
      _ offset: Int?
    ) -> [String]
}

let myPeopleSearchService = PeopleSearchService(query: {
  (query, orderBy, ascending, limit, offset) in return ["a", "b", "c"]
})

struct CarSearchService {
  var query:
    (
      _ args: (
        query: String?, orderBy: String?, ascending: Bool?,
        limit: Int?, offset: Int?
      )
    ) -> [String]
}

let myCarSearchService = CarSearchService(query: { params in
  let (query, orderBy, direction, limit, offset) = params
  return ["a", "b", "c"]
})

[marius-se]

@allevato are there any plans on reviving this PR? Would be great to have it!