Invert guard let scoping

I'm proposing the introduction of an inverted scope for the "guard let..." pattern. The problem occurs most commonly when propagating errors to completion handlers.

Example using URLSession.dataTask(with:completionHandler:):

class Network {
    let session: URLSession
    func callServer(completion: @escaping (Data?, Error?) -> Void) {
        let task = urlSession.getDataTask(request: request) {  (data, response, error in
        if let error = error {
            log("ERROR: \(error)")
            completion(nil, error)
            // Oops, I forgot to return here
        }
        ... additional error checking, deserialization, and data validation ...
        completion(data)
    }
}

Using the if-let pattern above allows me to have a non-optional "error" variable, however it becomes easy to miss the intended return.

If I switch to guard-let, the non-optional "error" variable is not available while in the "else" scope. This is a minor inconvenience in the log example above, but if I wish to use more type-safe call signatures, it requires me to use force-unwraps:

class Network {
    let session: URLSession
    func callServer(completion: @escaping (Result<Data,Error>) -> Void) {
        let task = urlSession.getDataTask(request: request) {  (data, response, error in
        guard error == nil else {
            log("ERROR: \(error!)") // why must I force-unwrap here?
            completion(.failure(error!))
            return
        }
        ... additional error checking, deserialization, and data validation ...
        completion(data)
    }
}

By switching to 'guard', I can now enforce return in the error handler, but I'm left with an optional when I know it is not.

If I wish to enforce returns as well as having an unwrapped optional, I'm left with having to do something like this:

class Network {
    let session: URLSession
    func callServer(completion: @escaping (Result<Data,Error>) -> Void) {
        let task = urlSession.getDataTask(request: request) {  (data, response, error in
        guard error == nil else {
            guard let error = error else { assertFailure("This can never happen"); return }
            log("ERROR: \(error)")
            completion(.failure(error))
            return
        }
        ... additional error checking, deserialization, and data validation ...
        completion(data)
    }
}

I propose either:

  • a new keyword (e.g.) nguard that allows the scope of the 'let' variable to be inverted, or
  • guard to be smarter about knowing that I've tested specifically for a nil case and therefore automatically lose optionality in the respective scope.

An example usage of nguard would be:

class Network {
    let session: URLSession
    func callServer(completion: @escaping (Result<Data,Error>) -> Void) {
        let task = urlSession.getDataTask(request: request) {  (data, response, error in
        nguard let error = error else {
            log("ERROR: \(error)")  // error is non-optional here
            completion(.failure(error))
            return
        }
        // error is optional here... the compiler can also infer that error must be nil here.

        ... additional error checking, deserialization, and data validation ...
        completion(data)
    }
}

The introduction of a new keyword would have minimal impact on existing code. The "n" denotes negative or inverted scope. (Also because nguard is punny :slight_smile: -- I'm sure someone else can come up with a better keyword/modifier)

8 Likes

This is very similar to an issue I recently felt was worth complaining about where the inability to negate the semantics of if let forced an undesirable order of conditional blocks:

if let error = error {
    completion(.failure(error))
} else if let statusCode = (response as? HTTPURLResponse)?.statusCode, statusCode != 200 {
    completion(.failure(NetworkError.badResponseCode(statusCode)))

} else if let data = data, let value = String(data: data, encoding: .utf8) {
    // was unable to write this to follow all the failure cases, must be in the middle
    ...
    let authorization = process(value)
    completion(.success(authorization))

} else {
    completion(.failure(NetworkError.badResponseData))
}

instead of something like:

if let error = error {
    completion(.failure(error))
} else if let statusCode = (response as? HTTPURLResponse)?.statusCode, statusCode != 200 {
    completion(.failure(NetworkError.badResponseCode(statusCode)))
} else ifnot let data = data, let value = String(data: data, encoding: .utf8) {
    completion(.failure(NetworkError.badResponseData))
    
} else { // ahhh, this success case can follow all the failure cases
    ... 
    let authorization = process(value)
    completion(.success(authorization))
}

Where this thread's question asks for a variation on guard ... else with the inverse semantics: positive test and assigned vars valid inside the block, I was wishing for an if let variant with inverse semantics: negative test and assigned vars valid following the block.

Was it perl that had an unless statement? How about adding unless to swift? It can cover the inverse if semantics, and can lead to a somewhat* natural extension of guard that can support its inverse semantics too.

I'm thinking extending guard syntax to support guard unless and guard if. The former would just be another way to write the existing guard ... else:

guard let a = b else {
    return
}
f(a)

guard unless let a = b {
    return
}
f(a)

This isn't so useful an addition but exists to be symmetrical with guard if:

guard if let a = b {
    f(a)
    return
}

(*) Yes, I admit this is indeed only a "somewhat natural" extension to the existing guard statement and doesn't quite have its grammatical elegance. However, I propose that this is superior to adding another keyword just for "inverted-semantics guard".

And unless and else unless can be used similarly to if and else if:

unless str.isEmpty { // instead of: if !str.isEmpty
    apply(str)
}

With unless let allowing assigned vars to be used in all the else conditions and code blocks that follow:

if let a = b {
    f(a)
} else unless let c = d {
    print("banana")
} else if let e = g(c) {    // c usable here
    thing1(c, e)            // c usable here
} else {
    thing2(c)               // c usable here
}
// c not usable here though

There was a proposal last year on exactly this subject, Control Flow Negation Statements.

5 Likes

I'm not sure that April Fools jokes merit citing as prior art. :wink:

11 Likes

I try to avoid this in a couple of ways:

  1. I generally prefer using switch over if let/guard with the earlier branches handling the error paths and the final branch handling the happy path.

  2. Refactoring out the error-handling logic into a closure. This often makes it easier to spot inconsistencies in error-handling in different parts of the code.

    let handleXError = { error in
      log("ERROR: \(error)")
      completion(nil, error)
    }
    if let error = error {
      handleXError()
      return;
    }
    

    This pattern makes it easy to visually scan the error-handling if lets (they all look like handleABCError() followed by return) and double-check the returns.

Not saying this necessarily solves your problem, but something worth to try out/experimenting with if you feel like it might work for you. :slight_smile:

As I told Harlan, it's the fact that some of these are genuinely useful that makes the joke work.

8 Likes

Why not create a Result directly (you can use a custom initializer that takes data?, error?) and pass that to completion instead of trying to handle the error in the data task completion handler? I've found that pretty much eliminates this pattern and the need for nguard. The only issue is when both data and error are nil, and the initializer needs to add a "missing info" error of its own, which is minor.

8 Likes

In a somewhat jokingly note, another solution to this is to have typed throws and guard catch supported by the language.

So you’d have

guard let value = try result.get() catch {
  // handle error
  return
}
// use value
3 Likes

First time poster and I find this compelling as a way to ensure my own handling of any optional error isn't.... bad?

A fairly common example from working with UIKit:

@objc
func image(image: UIImage, didFinishSavingWithError error: NSError?, contextInfo: UnsafeMutablePointer<Int>) {
    if let error = error {
        // do stuff with error
        return // no compiler enforcement of this
    }
    // do stuff where error is still optional
}

Two problems with this:

  • If I forget to return, it isn’t at all obvious. Yet there is never an if let error that I don't want to exit from
  • I shouldn't be messing with the optional error after error handling, yet I can.

My syntax nomination for the proposal is guard let then, as a counterpart to guard let else. The wording makes clearer to me that the let constant will be used within the guard scope, not outside it.

@objc
func image(image: UIImage, didFinishSavingWithError error: NSError?, contextInfo: UnsafeMutablePointer<Int>) {
    guard let error = error then {
        // do stuff
        return // guard must exit, compiler enforced
    }
    // do stuff where error is definitely nil
}

This naturally segregates all of the error handling code (at least for this error) from the success path, which if let error allows, but doesn’t enforce. It also guarantees that I didn’t make the silly mistake of not returning.

11 Likes

So this is admittedly wonky, but map is another option:

guard error.map({print($0)}) == nil else {
    print("Guarded")
    return
}

Or with your original example:

class Network {
    let session: URLSession = .shared
    func callServer(request: URLRequest, completion: @escaping (Result<Data,Error>) -> Void) {
        let task = session.dataTask(with: request) {  (data, response, error) in
            guard error.map({
                completion(.failure($0))
                print("ERROR: \($0)")
                return Void.self
            }) == nil else {
            return
            }
        // The rest of your function
        }
    }
}

There are usually plenty of ways to work around the issue (I usually use switch) but it does seem natural for something like this to exist. Personally I would prefer to keep guard with it's current semantics. It can already be a bit confusing to reason about. Adding a case where a let could be available in the typical scope or inside else seems problematic.

How about escape?

escape if let error = error {
// return is required
    return
}

Instead of that weird double-guard, I would suggest force-unwrapping the variable just once:

        guard error == nil else {
            let error = error!
            log("ERROR: \(error)")
            completion(.failure(error))
            return
        }

In my view, there is no problem in using force-unwrap right after you’ve made sure that the variable is not nil, where a reader of the code can see that the unwrapping is fully justified and will not lead to a crash.
I even tend to find this approach more legible than if let or guard let, thanks to the unwrapping being explicit rather than implicit.

3 Likes

I agree with @Erica_Sadun's suggestion. Unless there's a pressing example of 'loose' API design where we cannot convert/abstract to a more declarative type, an inverted guard is not really needed. I would suggest to work with the tools we have available to make our API design better.
Doing this work just once (as you have, inside the Network class) is an OK compromise, considering that Apple is very unlikely to 'fix' that completion handler :man_shrugging:

For the OP's use case, it's useful to look at examples where this has been covered already:

Ole Begemann points out that the 'correct' type* would be:

Result<(Data, URLResponse), (Error, URLResponse?)>

Similarly, Apple's URLSession.DataTaskPublisher.Output is (data: Data, response: URLResponse) and .Failure is a custom URLError type that conforms to Error. The fact that Apple has built it this way, and on top of dataTask(...), is a pretty strong hint of the preferred type.

And so, making a converter should be pretty simple 🤞
func result(from data: Data?,
            response: URLResponse?,
            error: Error?) -> Result<(Data, URLResponse), DataTaskError> {
  if let error = error {
    return .failure(DataTaskError(error: error, response: response)
  } else if let data = data, let response = response {
    return .success((data, response))
  } else {
    assertionFailure("Unrecognised data task result")
    return .failure(DataTaskError(error: .genericErrorOrOther, response: response)
  }
}

struct DataTaskError: Error {
    var error: Error
    var response: URLResponse?
}

*of course, tuples cannot conform to Error, so I would either omit response or wrap these in an Error conforming struct, just as Apple have done

2 Likes

Seems like Foundation needs to start adopting its own Result type (until async/await) to solve that for all completion handlers.
Especially for URLSession.

1 Like

Is there a use case where it would not be a good solution to simply have regular old guard give unwrapped variables inside the else clause if they just failed an "== nil" test?

guard error == nil else {
    // use error here, as if already unwrapped
    return
}

Syntactically, I sort of like @justrowingby's 'guard let...then' for indicating you want the inverted semantics, over the 'nguard', but I don't really like the idea of guard switching semantics at all.

(@tevamerlin's simple let inside the guard is a good solution as well.)

Of course it would be great if a bunch of these APIs would make use of Result now that have it.

The automatic unwrap I suggest wouldn't work if you had multiple nil comparisons:

guard thingOne == nil, thingTwo == nil else {
    // no special behavior since we don't know which thing is not nil (or if both are)
    return
}

guard thingOne == nil else {
    // thingOne is unwrapped 
    return
}

That's an approach taken by kotlin, and it doesn't really work with anything that isn't a constant.

It changes the behavior of existing code:

func f(_ str: String)  { print("a") }
func f(_ str: String?) { print("b") }
func main() {
    let error: String? = "Hello"
    guard error == nil else {
        // use error here, as if already unwrapped
        f(error) // a or b?
        return
    }
}
main()

It would prevent access to the original variable

    var error: String? = "Hello"
    guard error == nil else {
        // use error here, as if already unwrapped
        error = nil // would this be an error? If so, how would you set error to nil in this scope if you wanted?
        return
    }

You couldn't use it with computed variables and non-local properties

class Foo { var error: String? = "" }
var globalFoo: Foo? = nil
func f(_ str: String) { print(str) }
func main() {
    let foo = Foo()
    globalFoo = foo
    guard foo.error == nil else {
        // use error here, as if already unwrapped
        globalFoo?.error = nil
        f(foo.error) // compile-time error? Runtime error?
        return
    }
}
main()

We've had discussions in the past (sorry, can't immediately find the threads) about re-defining variable types based on conditions, and it seems like that could work well here. Some examples:

var error: Error? = ...
guard error != nil else {
    // the compiler has proven that error is non-nil at this point
    // therefore the compiler changes the type of error from "Error?" to "Error"
    throw error
}

Or with an if statement:

var error: Error? = ...
if error == nil { return }

// error is known to be non-nil at this point, 
// therefore "error" has been adjusted to be of type "Error"
throw error
1 Like

You simply write an extension on URLSession that returns a result type.. it takes all of 5 lines of code.

Another idea besides what @Erica_Sadun has already suggested is to simply check for the success case first. If error is non-nil, but the server still returns a 200-status and all the data you need, there is no reason to error out anyways as you do in the code you start the thread with. You can guard on the success case and return there instead.

@Ilias_Karim or you simply accept the language as it is and not participate in forum discussions.
"all it takes" is almost never a good answer.

1 Like

that would actually solve a lot more problems than just this specific issue.
would love for this feature to be supported.