Update switch_case_on_newline rule to allow returnless cases

[gsl-anthonymerle]

Since Swift 5.9, switch cases can return a value without having to explicitly use the return keyword with the condition that they contain only a single line statement.

With this evolution, we tend to see more and more switch cases written on a single line.
This pull request aims to offer a configuration point to the switch_case_on_newline rule so that it skip these specific cases, so that we can allow the following:

switch myEnum {
    case .foo: 42,
    case .bar: 24 
}

but still preventing:

switch myEnum {
    case .foo: return 42,
    case .bar: return 24 
}

[SwiftLintBot]

	17 Messages
📖	Linting Aerial with this PR took 0.72s vs 0.73s on main (1% faster)
📖	Linting Alamofire with this PR took 1.01s vs 1.01s on main (0% slower)
📖	Linting Brave with this PR took 5.84s vs 5.89s on main (0% faster)
📖	Linting DuckDuckGo with this PR took 3.11s vs 3.13s on main (0% faster)
📖	Linting Firefox with this PR took 8.72s vs 8.73s on main (0% faster)
📖	Linting Kickstarter with this PR took 7.09s vs 7.2s on main (1% faster)
📖	Linting Moya with this PR took 0.43s vs 0.44s on main (2% faster)
📖	Linting NetNewsWire with this PR took 1.94s vs 1.96s on main (1% faster)
📖	Linting Nimble with this PR took 0.6s vs 0.6s on main (0% slower)
📖	Linting PocketCasts with this PR took 6.03s vs 6.08s on main (0% faster)
📖	Linting Quick with this PR took 0.3s vs 0.3s on main (0% slower)
📖	Linting Realm with this PR took 3.66s vs 3.66s on main (0% slower)
📖	Linting Sourcery with this PR took 1.8s vs 1.79s on main (0% slower)
📖	Linting Swift with this PR took 3.44s vs 3.44s on main (0% slower)
📖	Linting VLC with this PR took 0.99s vs 1.0s on main (1% faster)
📖	Linting Wire with this PR took 12.76s vs 12.82s on main (0% faster)
📖	Linting WordPress with this PR took 8.39s vs 8.44s on main (0% faster)

Generated by 🚫 Danger

[SimplyDanny]

I wonder if this should really be bound to the return keyword. What this actually wants to achieve is silencing the rule when the switch-case is used as an expression (returned as a whole or assigned to a variable), doesn't it?

[gsl-anthonymerle]

@SimplyDanny That's a good remark 👍
I tried to dive into swift-syntax to see how I could identify when the case is used in an expression vs a statement, but I couldn't find it unfortunately... I couldn't see any difference in the nodes tree in both scenarios
I'll continue working on it, I'm sure there is a way 🤞

[gsl-anthonymerle]

@SimplyDanny I used the rule defined by the Swift-Syntax's pull request introducing if/switch expressions to check wether the current switch case is enclosed in a statement or an expression.

I found out that the following were not considered a switch expression tho:

var myComputedVar: Bool {
    switch someEnum {
        case 1: true
        case 2: false
    }
}

func myFunc() -> Bool {
    switch someEnum {
        case 1: true
        case 2: false
    }
}

To make them be treated as switch expression I have to use an intermediate value to be assigned like so:

var myComputedVar: Bool {
    let result = switch someEnum {
        case 1: true
        case 2: false
    }
    return result
}

func myFunc() -> Bool {
    let result = switch someEnum {
        case 1: true
        case 2: false
    }
    return result
}

I don't know if this is intended or not, because various websites consider these case as being switch expressions, I'll ask this question to the pull request referenced above, but to me the current SwiftLint rule is on the same track as the current Swift-Syntax implementation now 🙏

[SimplyDanny]

I don't know if this is intended or not, because various websites consider these case as being switch expressions, I'll ask this question to the pull request referenced above, but to me the current SwiftLint rule is on the same track as the current Swift-Syntax implementation now 🙏

The parser alone cannot decide if a switch is an expression or a statement. So the rule might not be able to decide whether to trigger or not either. We would need to come up with another condition to handle your special case. I still think, binding it to return keywords is too much of a special case, though.

[gsl-anthonymerle]

Hi @SimplyDanny,

Thanks for your answer, I understand that you don't seem convinced that the proposed solution is suitable to allow for switch and if expressions as the syntax itself won't handle every cases.

I'll close the Pull Request now, thank you for the exchange anyways 🙏
Best regards,
Anthony