`BasicFormat` indents closing `}` of closure passed as a function parameter

[ahoppen]

The following new test case fails.

func testClosureParameter() {
  let source = """
    myFunc({
        return true
    })
    """
  assertFormatted(source: source, expected: source)
}

The actual formatted source is

myFunc({
    return true
    })

rdar://114044607

[Matejkob]

If I'm not mistaken it's possible that it's the same issue as #1473

[ahoppen]

Yes, that could very well be. Interestingly #1473 also has a double indent of the closure body, which this issue doesn’t.

[Matejkob]

I believe this part has already been fixed. I can try to solve this issue (but I am not sure if I'll be able 😅)

[ahoppen]

Sure, give it a shot. We should somehow make sure that we don’t add another nesting level for this kind of pattern. But in code like this, the closure should continue to get indented.

myFunc(
  x: 1,
  closure: {
    return true
  }
)

[Matejkob]

How about this case:

myFunc(x: 1,
  closure: {
    return true
  })

?
Should we format it in the following way:

myFunc(x: 1,
  closure: {
    return true
})

or keep original?

[ahoppen]

swift-format formats it with indented }) but I’m happy with whichever formatting is easiest to implement.

[stevapple]

How about this case:

myFunc(x: 1,
  closure: {
    return true
  })

?

I think this is okay (give the spacing is set to 2). It a common code style in Swift (especially in an array literal) to have no spacing between the last parameter and right paren. If one wants ) to be in a separate line, he/she can add a newline between } and ), so this will be formatted as:

myFunc(
  x: 1,
  closure: {
    return true
  }
)

---

To conclude, I believe the real problem is #2084. The correct format for

myFunc({
  return true
})

should be

myFunc({
        return true
    })

instead of

myFunc({
    return true
})

if no exceptional rule is applied.

---

If it’s a generated source, I actually recommend the following style, as described in #2084:

myFunc(
  {
    return true
  }
)

[stevapple]

The code could be prettier if we didn’t force a closure to be multi-lined, so the following is allowed:

myFunc({ return true })

which eliminates the need for a special rule for closure in a tuple.

[Matejkob]

To conclude, I believe the real problem is #2084. The correct format for

myFunc({
  return true
})

should be

myFunc({
        return true
    })

instead of

myFunc({
    return true
})

if no exceptional rule is applied.

So, if the spacing were set to 2, the code would look like this, wouldn't it?

myFunc({
    return true
   })
   
myFunc(myLongName: {
    return true
   })   

---

The code could be prettier if we didn’t force a closure to be multi-lined, so the following is allowed:

myFunc({ return true })

which eliminates the need for a special rule for closure in a tuple.

That would work perfectly for situations like this:

if let first = array.first(where: { $0 == "something" }) {
   print(first)
}

the current formatting made by BasicFormat looks like so:

failed - Actual output (+) differed from expected output (-):
-if let first = array.first(where: { $0 == "something" }) {
+if let first = array.first(where: {
+        $0 == "something"
+    }) {
    print(first)
 }

[ahoppen]

I think this is okay (give the spacing is set to 2). It a common code style in Swift (especially in an array literal) to have no spacing between the last parameter and right paren. If one wants ) to be in a separate line, he/she can add a newline between } and ), so this will be formatted as:

myFunc(
  x: 1,
  closure: {
    return true
  }
)

To conclude, I believe the real problem is #2084. The correct format for

myFunc({
  return true
})

should be

myFunc({
        return true
    })

instead of

myFunc({
    return true
})

if no exceptional rule is applied.

I think the rule could be: If the closure starts on the same line as the opening ( of the function arguments, then it shouldn’t add an additional nesting level. That would get us the expected formatting result of

myFunc({
    return true
})

The code could be prettier if we didn’t force a closure to be multi-lined, so the following is allowed:

myFunc({ return true })

which eliminates the need for a special rule for closure in a tuple.

I’m very open to that suggestion. If the closure only contains a single line and it doesn’t contain a signature, we should format it on a single line.