New rule redundantClosure

[danthorpe]

:wave: Hi! Just wanted to raise an issue to highlight something my team have encountered with swiftformat recently.

We integrate SwiftFormat into our project using Homebrew via Tuist. Homebrew doesn't really support pinning versions, so this isn't great, and the version of the tool that a team-member uses, really just depends on when they join the team or manager their own Homebrew installation. It's not possible to install an old version of a tool, that's for sure. This is an issue with Homebrew, and the way that this project integrates its tooling dependencies - but it's also probably a very common path.

The latest version of SwiftFormat introduces some new rules, and they are turned on by default. Old versions of SwiftFormat fail if they don't recognise rule names. I was kinda hoping that SwiftFormat would just ignore rule configurations which it didn't recognise. This means that we cannot disable new rules, because it will fail for people (or CI) who haven't got that version of SwiftFormat. This feels like a choice on the part of SwiftFormat (to enable new rules by default, instead of have them be disabled), but I can see that you want to drive adoption of rules - so, I guess I can understand that.

The specific problem with redundantClosure is that it can render code not compiling, in particular, output from Sourcery. Consider that this code:

lazy var foo: String = { fatalError("no default value has been set") }()

gets formatted into:

lazy var foo: String = fatalError("no default value has been set")

which no longer compiles.

Lazy vars, autoclosures and redundant closures are a pretty common pattern for mocking or mockable interfaces. If it's going to be enabled by default - feels like maybe it should not format these cases?

[nicklockwood]

You can specify an explicit set of rules using the --rules config option instead of using --disable to opt-out. That avoids the problem of having to mention unknown rules in the config.

[nicklockwood]

@calda would you be able to take a look at the redundantClosures issue? My guess is that because fatalError returns Never It can't be substituted for a Void closure? Probably the best we can do is disable the rule for a known blacklist of functions. Maybe even provide a way for users to specify an exclusion list?

[calda]

Ah! This makes sense, I'll put up a fix.

[calda]

fixed in #1098 -- thanks for the report @danthorpe!

[austinwright]

Hi there - I have another use case where this rule is formatting code and causing it to no longer compile:

     lazy var triggerSomething: Void = {
        logger.trace("log some stuff before Triggering")
        TriggerClass.triggerTheThing()
        logger.trace("Finished triggering the thing")
     }()

is formatted to:

     lazy var triggerSomething: Void = logger.trace("log some stuff before Triggering")
     TriggerClass.triggerTheThing()
     logger.trace("Finished triggering the thing")

which no longer compiles.

[calda]

Thanks for sharing, will take a look 👍

[calda]

@austinwright, fixed in #1124.

[nicklockwood]

@danthorpe @austinwright fix landed in 0.49.2