Forward scan matching for multiple trailing closures

Using Swift
1

I've just put up a PR to Vapor fixing warnings caused by the new forward scan matching change for trailing closures: Fix Swift 5.3 trailing closure warnings by tanner0101 · Pull Request #2484 · vapor/vapor · GitHub. On top of being quite invasive, these changes result in arguably less readable code. I think this is going to affect Vapor users negatively.

I'm wondering if there's anyway I can update Vapor to keep the old behavior?

To explain the issue, we have a test method that accepts two trailing closures: beforeRequest and afterResponse for setting / testing for the request and response objects.

// simplified method signature
func test(
    _ method: HTTPMethod, 
    _ path: String, beforeRequest,
    beforeRequest: (inout HTTPRequest) -> () = { _ in },
    afterResponse: (HTTPResponse) -> () = { _ in }
)

It's very common to leave the request untouched and only run checks on the response, so the backward matching for trailing closures was quite helpful here. Typical usage looks like:

// old syntax. no longer compiles without warnings.
app.test(.GET, "hello") { res in 
    XCTAssertEqual(res.status, .ok)
}

However, the new trailing closure matching rules break this use case. Now we must specify the afterResponse label explicitly:

// new syntax. more verbose with no benefit.
app.test(.GET, "hello", afterResponse: { res in 
    XCTAssertEqual(res.status, .ok)
})

Trailing closure syntax can now be used with beforeRequest, however it requires explicit types to be used. This is both more verbose and less helpful since setting beforeRequest is not common.

// syntactically messy and also not particularly helpful to use
// trailing closure syntax since it's not a common use case
try app.test(.GET, "test") { (req: inout XCTHTTPRequest) in
    print(req)
}

Cases where both the request and response closures are provided (less common usage) remain unaffected.

// luckily this still works fine
try app.test(.GET, "/check", beforeRequest: { req in
    try req.content.encode(["foo": "bar"], as: .json)
}) { res in
    XCTAssertEqual(res.body.string, "...")
}

Note that the solution for View.sheet(isPresented:onDismiss:content:) mentioned in the pitch doesn't help here since afterResponse should be an optional parameter. It is possible to omit both the beforeRequest and afterResponse closures if you simply want to run the test and not perform any checks.

// just runs the request.
try app.test(.GET, "foo")
2

I'm wondering if there's anyway I can update Vapor to keep the old behavior?

Good question! You should be able to add overloads to Vapor to achieve the same behavior for clients without a bunch of those warnings. For example, for this signature:

func test(
    _ method: HTTPMethod, 
    _ path: String,
    beforeRequest: (inout HTTPRequest) -> () = { _ in },
    afterResponse: (HTTPResponse) -> () = { _ in }
)

Since it's common for callers to only supply the afterResponse closure, you can add an overload that requires only the afterResponse closure with no default argument, e.g.:

func test(
    _ method: HTTPMethod, 
    _ path: String,
    afterResponse: (HTTPResponse) -> ()
)

I tested this out locally and it works for this contrived case

Edit: Full source code 
struct HTTPMethod {}
struct HTTPRequest {}
enum HTTPResponse { case ok }

func test(
    _ method: HTTPMethod,
    _ path: String,
    beforeRequest: (inout HTTPRequest) -> () = { _ in },
    afterResponse: (HTTPResponse) -> ()  = { _ in }
) {}

func test(
    _ method: HTTPMethod,
    _ path: String,
    afterResponse: (HTTPResponse) -> ()
) {}

test(HTTPMethod(), "") { response in
  assert(response == .ok)
}
1 Like
3

:o that fixed it. Thank you!!

1 Like