swift-metrics icon indicating copy to clipboard operation
swift-metrics copied to clipboard

Sanity script should check for missing `return` statements

Open MrLotU opened this issue 4 years ago • 4 comments

Since Swift 5.2 (IIRC) you're allowed to omit the return statement in single statement functions and getters as such:

func foo() -> String {
    "bar"
}

Since we also support versions pre 5.2, the sanity script should check for this and enforce the use of return statements everywhere.

Expected behavior

The sanity script should check and correct missing return statements.

Actual behavior

Sanity script does not correct this

If possible, minimal yet complete reproducer code (or URL to code)

See #61 (Does not use return in a couple of places, did pass sanity script)

MrLotU avatar Jul 17 '20 08:07 MrLotU

Hmm... if it's possible to configure swift-format in some way which can catch this that we can enable I think. Do you know if that's possible?

Otherwise I'm not sure about the value/effort ratio; we build on 5.1 as well, so this build would fail on compilation if there's a missing return right?

ktoso avatar Jul 17 '20 08:07 ktoso

I know that swift-format can remove the return statement on 5.2, so one imagines it can add it in 5.1 mode.

Lukasa avatar Jul 17 '20 08:07 Lukasa

Yeah, not at all sure if this is possible, but something that would be helpful if it was. If not, than indeed CI failing on older Swift versions will catch it, but pre-catching things like this is always preferred IMO 😄

MrLotU avatar Jul 17 '20 09:07 MrLotU

+1 to deal with this in swift-format config, this will then be naturally integrated into the sanity check

tomerd avatar Sep 23 '20 20:09 tomerd