Rule Request: Remove redundant parenthesis from closures

[rogerluan]

I did my research :)

• [x] I searched existing rules, and checked for existing GitHub issues to see if this request wasn't made before

New rule request

This new rule would remove redundant parenthesis from closures, as it's a lot cleaner. See examples below:

What would trigger

let filteredResults = array.filter({ $0.isValid })

[1, 2].map({ $0 + 1 })

[1, 2].map({ $0 + 1 }).forEach({ print($0) })

[1, 2].map({ $0 + 1 })
  .forEach({ print($0) })

[1, 2].map { $0 + 1 }
  .map({ $0 + 2 })
  .map { $0 + 3 }

[1, 2].map({ $0 + 1 })
  .map { $0 + 2 }
  .map({ $0 + 3 })

What wouldn't trigger

// Swift Syntax doesn't allow us to remove the parenthesis in this case
guard let filteredResults = array.filter({ $0.isValid }) else { return }

// Swift Syntax doesn't allow us to remove the parenthesis in this case
if let filteredResults = array.filter({ $0.isValid }) {
    print(filteredResults)
}

guard let validElement = array.first(where: { $0.isValid }) else { return }

let filteredResults = array.filter { $0.isValid }

[1, 2].map { $0 + 1 }

[1, 2].reduce(0) { $0 + $1 }

[1, 2].map { number in
 number + 1 
}

let isEmpty = [1, 2].isEmpty()
UIView.animateWithDuration(0.3, animations: {
   self.disableInteractionRightView.alpha = 0
}, completion: { _ in
   ()
})

• Q: Should the rule be configurable, if so what parameters should be configurable?
  • A: I don't think there's any configuration needed.
• Q: Should the rule be opt-in or enabled by default? Why?
  • A: I think it should be enabled by default if, during implementation, we conclude that it's safe and doesn't have many false positives.

I believe there's a consensus that this is the expected way to write Swifty code, but I could be wrong. In fact I was very surprised that SwiftLint doesn't support this already. I'm curious to hear other opinions.

[nicklockwood]

Does the redundantParens rule not already do this?

[rogerluan]

Nope @nicklockwood 😢 it's not a covered use case/example in the docs, and I also tested and it doesn't modify my code

[nicklockwood]

@rogerluan oh sorry you're right - I was actually thinking of the trailingClosures rule.

I just tested it with all your examples and it seems to work (it's also enabled by default).

[rogerluan]

Oooh. I've explicitly disable trailing closures for some reason, I can't remember why now. I'll test it and try to remember why I disabled it as it might've been some bug 😅 I'll report back. Thanks for the help!

[rogerluan]

Ah, I remembered. In one of the projects I worked on, there was a overloaded function that used to cause issues, so I disabled this rule. It was weird, but an overview of the issue would be:

• When using the trailing closure with parenthesis and explicit argument name, it would use Overloaded Function A, which was OK
• When omitting the trailing closure parenthesis (and thus removing the explicit argument name of the closure), it would pick up Overloaded Function B, which crashed (as it had a preconditionFailure() inside of it, intentionally).

The issue was that due to some poor API design those two overloaded functions were only differing in the name of the closure argument IIRC (can't remember the details or give concrete examples tbh).

I don't think that is an issue that SwiftFormat should resolve haha and I see no reason to not enable this rule in all other projects :) Thanks for pointing this to me! We can close this issue 🚀

[nicklockwood]

@rogerluan this was actually a known issue with the rule, which was fixed in a later version by limiting it to only anonymous closure arguments (from the known issues section of the README):

The trailingClosures rule can generate ambiguous code if a function has multiple optional closure arguments, or if multiple functions have signatures differing only by the name of the closure argument. For this reason, the rule is limited to anonymous closure arguments by default. You can use the --trailingclosures and --nevertrailing arguments to explicitly opt in or out of trailing closure support for specific functions.

So I think it should be safe to re-enable it for your project now.

[rogerluan]

That's great to hear!! 🤩 It was indeed a long time ago and I didn't realize this has been fixed since then 😁

Thank you once again!