apple/swift-metrics

Sanity script should check for missing `return` statements

MrLotU opened this issue · 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)

ktoso commented

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?

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

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 😄

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