Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanity script should check for missing return statements #73

Open
MrLotU opened this issue Jul 17, 2020 · 4 comments
Open

Sanity script should check for missing return statements #73

MrLotU opened this issue Jul 17, 2020 · 4 comments
Labels
semver/patch No public API change.

Comments

@MrLotU
Copy link
Contributor

MrLotU commented Jul 17, 2020

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
Copy link
Member

ktoso commented Jul 17, 2020

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 ktoso added help wanted semver/patch No public API change. labels Jul 17, 2020
@ktoso ktoso added this to the 1.3.1 milestone Jul 17, 2020
@Lukasa
Copy link
Contributor

Lukasa commented Jul 17, 2020

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

@MrLotU
Copy link
Contributor Author

MrLotU commented Jul 17, 2020

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 😄

@ktoso ktoso removed this from the 1.3.1 milestone Sep 23, 2020
@tomerd
Copy link
Member

tomerd commented Sep 23, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch No public API change.
Projects
None yet
Development

No branches or pull requests

5 participants