In the previous `fatalError` message `Expected non-nil result 'result' for nil error`, it wasn’t exactly clear wht the `error` referred to. In some cases, when the error was ignored, there wasn’t even an `error` variable in the refactored code.
The new `Expected non-nil result '...' in the non-error case` is a little more generic and also fits if an error is ignored.
The 'success param/argument' is a terminology we use internally in the refactoring and not one we want to present to the user. Just name it 'result' instead.
Since we aren’t checking that all result arguments are `nil` if there was an `error` in all the other refactorings, also remove the assertions in the async wrapper.
Previously, when a completion handler call had arguments for both the success and error parameters, we were always interpreting it as an error call. In case the error argument was an `Optional`, this could cause us to generate code that didn't compile, because we `throw`ed the `Error?` or passed it to `continuation.resume(throwing)`, both of which didn't work.
We now generate an `if let` statement that checks if the error is `nil`. If it is, the error is thrown. Otherwise, we interpret the call as a success call and return the result.
- Example 1 (convert to continuation)
-- Base Code
```swift
func test(completionHandler: (Int?, Error?) -> Void) {
withoutAsyncAlternativeThrowing { (theValue, theError) in
completionHandler(theValue, theError)
}
}
```
-- Old Refactoring Result
```swift
func test() async throws -> Int {
return try await withCheckedThrowingContinuation { continuation in
withoutAsyncAlternativeThrowing { (theValue, theError) in
continuation.resume(throwing: theError) // error: Argument type 'Error?' does not conform to expected type 'Error'
}
}
}
-- New Refactoring Result
```swift
func testThrowingContinuationRelayingErrorAndResult() async throws -> Int {
return try await withCheckedThrowingContinuation { continuation in
withoutAsyncAlternativeThrowing { (theValue, theError) in
if let error = theError {
continuation.resume(throwing: error)
} else {
guard let theValue = theValue else {
fatalError("Expected non-nil success argument 'theValue' for nil error")
}
continuation.resume(returning: theValue)
}
}
}
}
```
- Example 2 (convert to async/await)
-- Base Code
```swift
func test(completion: (String?, Error?) -> Void) {
simpleErr() { (res, err) in
completion(res, err)
}
}
```
-- Old Refactoring Result
```swift
func test() async throws -> String {
let res = try await simpleErr()
throw <#err#>
}
```
-- New Refactoring Result
```swift
func test() async throws -> String {
let res = try await simpleErr()
return res
}
```
The `std::vector` keeps its elements alive. Currently, we might be accessing uninitalized memory if the call of `callArgs(CE)` (which owns its elements in case there is a single element) goes out of scope before `Args`.
Two mostly-cosmetic change to the "Add Async Wrapper" refactoring
- Rename the continuation from `cont` to `continuation` and error from `err` to `error` to better match Swifts naming guidelines
- Add assertions that all result parameters are `nil` if an error is passed to the completion handler.
Resolves rdar://80172152
If we are requested to convert a function to async, but the call in the function’s body that eventually calls the completion handler doesn’t have an async alternative, we are currently copying the call as-is, replacing any calls to the completion handler by placeholders.
For example,
```swift
func testDispatch(completionHandler: @escaping (Int) -> Void) {
DispatchQueue.global.async {
completionHandler(longSyncFunc())
}
}
```
becomes
```swift
func testDispatch() async -> Int {
DispatchQueue.global.async {
<#completionHandler#>(longSyncFunc())
}
}
```
and
```swift
func testUrlSession(completionHandler: @escaping (Data) -> Void) {
let task = URLSession.shared.dataTask(with: request) { data, response, error in
completion(data!)
}
task.resume()
}
```
becomes
```swift
func testUrlSession() async -> Data {
let task = URLSession.shared.dataTask(with: request) { data, response, error in
<#completion#>(data!)
}
task.resume()
}
```
Both of these are better modelled using continuations. Thus, if we find an expression that contains a call to the completion handler and can’t be hoisted to an await statement, we are wrapping the rest of the current scope in a `withChecked(Throwing)Continuation`, producing the following results:
```swift
func testDispatch() async -> Int {
return await withCheckedContinuation { (continuation: CheckedContinuation<Int, Never>) in
DispatchQueue.global.async {
continuation.resume(returning: syncComputation())
}
}
}
```
and
```swift
func testDataTask() async -> Int?
return await withCheckedContinuation { (continuation: CheckedContinuation<Data, Never>) in
let task = URLSession.shared.dataTask { data, response, error in
continuation.resume(returning: data!)
}
task.resume()
}
}
```
I think both are much closer to what the developer is actually expecting.
Resolves rdar://79304583
Previously, in the following case we were failing to add a `return` keyword inside `withImplicitReturn`.
```
func withImplicitReturn(completionHandler: (String) -> Void) {
simple {
completionHandler($0)
}
}
```
This is because the call of `completionHandler($0)` is wrapped by an implicit `ReturnStmt` and thus we assumed that there was already a `return` keyword present.
Fix this issue by checking if the wrapping `ReturnStmt` is implicit and if it is, add the `return` keyword.
Fixes rdar://80009760
If a completion handler specifies internal parameter labels, we can use those to label the elements of the tuple returned by the async alternative.
For example
```swift
func foo(completion: (_ first: String, _ second: String) -> Void) { }
```
gets refactored to
```swift
func foo() async -> (first: String, second: String) { }
```
Resolves rdar://77268040
Previously we only supported `case` patterns that bound with a `let` inside the associated value like `case .success(let value)`. With this change, we also support `case let .success(value)`.
Fixes rdar://79279846 [SR-14772]
If the completion handler parameter has a default
argument, mark the resulting async function as
`@discardableResult`, as the caller may not care
about the result.
rdar://79190170
If we're going to be classifying nodes in either
an `else if` block or after a `guard` statement
with a known condition path, be more lenient with
unhandled conditions, as we already know what path
they should be classified along.
rdar://78564388
If we're in a case stmt body, any DeclRefExprs
to vars bound by a pattern will actually be to an
implicit var decl that's declared for the body. We
therefore need to walk to its "parent" to get to
the var referenced by the pattern, which will have
the correct entries in the naming and placeholder
maps.
Keep track of patterns that bind multiple vars and
print them out when converting an async call. If
the parameter being bound isn't referenced elsewhere,
we'll print the pattern inline as e.g:
```
let ((x, y), z) = await foo()
```
Otherwise, if the parameter is referenced elsewhere
in the block we'll print the pattern out of line,
such as:
```
let (res, z) = await foo()
let (x, y) = res
```
In addition, make sure to print var bindings out
of line if there's also a let binding, e.g:
```
let x = await foo()
var y = x
```
This ensures any mutations to y doesn't affect x.
If there's only a single var binding, we'll print
it inline.
rdar://77802560
I don't believe this case currently can come up,
but leave it explicitly unhandled for now so we
don't perform an erroneous transform if it ever
comes up in the future.
We were trying to retrieve the name of all function calls in the body using `getBaseIdentifier`. But calls to `init` don’t have a base identifier, just a `DeclBaseName` (which is special). Work with the `DeclBaseName` internally to prevent the crash.
Fixes rdar://78024731 [SR-14637]
Add an additional case to `CallbackCondition` to
support boolean checks, and add the necessary
classification logic to determine whether a bool
flag check is for a success or failure path.
The logic currently first checks to see if the
async alternative has a convention that specifies
which parameter the flag is, and whether it's a
success or failure flag. If so, it classifies the
flag check accordingly. If there's no async
convention specified, we use a heuristic to see if
the error parameter is force unwrapped in the
failure block. If so, we treat that as the error
path. If there's no force unwrap of the error, we
leave the condition unhandled.
rdar://74063899
- Expand ConditionType to include `.success` and
`.failure` patterns.
- Introduce ConditionPath to represent whether a
classified condition represents a success or
failure path.
- Add methods to CallbackClassifier that deal with
deciding whether a given condition is a success or
failure path.
- Use these methods to simplify `classifyConditional`.
Adapt the `ThrowingEntityAnalyzer` to pick up any
`await` keywords and add an `async` to the extracted
function if necessary along with an `await` for its
call.
rdar://72199949
This allows an async alternative function to be
created that forwards onto the user's completion
handler function through the use of
`withCheckedContinuation`/`withCheckedThrowingContinuation`.
rdar://77802486
Factor out an `addForwardingCallTo` method that
allows the printing of a call to the old completion
handler function, with a given replacement for the
completion handler arg. `addCallToAsyncMethod` then
calls this with an empty replacement for the handler,
removing it from the call.
Abstract away the TupleExpr gunk and expose
`getLHS` and `getRHS` accessors. This is in
preparation for completely expunging the use
of TupleExpr as an argument list.
When adding an async alternative, add the @completionHandlerAsync
attribute to the sync function. Check for this attribute in addition to
the name check, ie. convert a call if the callee has either
@completionHandlerAsync or a name that is completion-handler-like name.
The addition of the attribute is currently gated behind the experimental
concurrency flag.
Resolves rdar://77486504
Previously we were unconditionally dropping a
return statement if it was the last node, which
could cause us to inadvertently drop the result
expression as well. Instead, only drop bare
'return' statements.
rdar://77789360
It's possible the user has already written an
explicit return for the call to the completion
handler. In that case, avoid adding another return.
rdar://77789360
- Add a missing return to the break statement
placeholder handling.
- Only turn the `return` token into a placeholder,
as we still want to apply the transformation to
the sub expression.
This stops us from crashing by attempting to walk
into the return sub-expression.
rdar://77789360
Previously we would drop comments between nodes in
a BraceStmt, as we printed each node out individually.
To remedy this, always make sure we scan backwards
to find any preceding comments attached to a node,
and also keep track of any SourceLocs which we
don't print, but may have comments attached which
we want to preserve.
rdar://77401810
When converting a call or function, rename declarations such that
redeclaration errors and shadowing are avoided. In some cases this will
be overly conservative, but since any renamed variable can be fixed with
edit all in scope, this is preferred over causing redeclaration errors
or possible shadowing.
Resolves rdar://73973517
When a function’s completion handler is being passed to another function and the function that the completion handler is being declared in is converted to async, replace the call to the completion handler by a `return` statement.
For example:
```swift
func foo(completion: (String) -> Void) {
bar(completion)
}
```
becomes
```swift
func foo() async -> String {
return await bar()
}
```
Previously, we were calling the completion handler, which no longer exists in the newly created `async` function.
