False positive in UnusedOptionalBindingRule

[mlilback]

This warning is triggered when using the try? syntax for error handling.

    enum DummyErrors: Error { case oops }
    func alwaysThrow() throws -> Int {
        throw DummyErrors.oops
    }
    func testSwiftlintFalsePositive() {
        guard let _ = try? alwaysThrow() else { return } // Unused Optional Binding Violation
    }

[marcelofabri]

I don't think this is necessarily a false positive.

You could do this instead:

guard try? alwaysThrow() != nil else { return }

I don't think anyone thought about try? when this rule was created, but I think the same idea applies.

[Sega-Zero]

Did you ever tried to compile that code in swift 3? It will never compile:

func alwaysThrow() throws {
}

guard try? alwaysThrow() != nil else { return }

[marcelofabri]

You need to wrap in parenthesis, I just forgot to do it.

func alwaysThrow() throws {
}

guard (try? alwaysThrow()) != nil else { return }

[Sega-Zero]

So, to silence that kind of warning, I better wrap every try? in parenthesis? Yeah, the code will be much cleaner, readable and safe.
I think that only uglify my code, I better silence that warning rather than using parenthesis.

[marcelofabri]

Well, I'd argue that try? should in general be avoided so errors are properly treated anyways.

I marked this as a discussion, so more people can jump in and share their thoughts. I'd like to say:

1. You're free to disable this rule in this snippet if it's an occasional thing.
2. Feel free to send a PR to make this behavior customizable, so you can change it on your configuration.
3. Let's avoid ironies and keep the conversation polite, shall we?

[Sega-Zero]

Right now I had to disable the rule in the config, 'cause I don't want to put too much of // swiftlint:disable in my code.

try? is a 100% normal construct. There are lot of cases when you don't really care if the method throws an error (like in FileManager routines). throws doesn't force a programmer to catch all the errors a function may throw, this is an indicator for a programmer, that something may have be wrong and it is up to a programmer to process it or ignore it.
try? also as good as early exit with guard: you can avoid too many nested ifs or silencing an error with do { try } catch {}. You just check, if a function throws an error, and don't execute the code next to it.
try? is not a try! which is totally should be avoided, IMHO.

I believe, that UnusedOptionalBindingRule better either be split in two rules (the one for unused lets and the one for guard let _ =) or a guard let _ = case should be treated as a false positive.

Anyway, the parenthesis option is useless: it does the same thing, but without let. You still have try?, you still get a result. But instead of simply ignoring a result value if a function succeeds, we compare the result with nil. I wish we could write it without let, but guard operator will not allow us to do it.
The code without parenthesis is much simpler and cleaner.

[marcelofabri]

I believe, that UnusedOptionalBindingRule better either be split in two rules (the one for unused lets and the one for guard let _ =) or a guard let _ = case should be treated as a false positive.

Feel free to send a PR with implementing this as a configuration or splitting into a new rule! 💯

[Sega-Zero]

@marcelofabri, I need some help with the tests in script/cibuild.
Trying to understand the reason of test failure. Did the test case similar to testTrailingWhitespace, but it still fails in the script and not fails in XCode.
What do I do wrong?

[marcelofabri]

Closed in #1480