Commit Graph

26 Commits

Author SHA1 Message Date
Hamish Knight
9b493970aa [Aync Refactoring] Better handle comment trailing whitespace
Use `getCommentRange` to trim off the comment's
trailing whitespace, avoiding outputting
extraneous empty lines.

rdar://82072147
2021-09-06 17:51:39 +01:00
Ben Barham
9b8d9e22ef Merge pull request #38981 from apple/skip-underscore
[Refactoring] Do not try to unique '_'
2021-08-24 11:33:15 +10:00
Ben Barham
afaa9e13e4 Merge pull request #38968 from apple/double-async-bad
[Refactoring] Do not add "async" if function is already async
2021-08-21 07:32:39 +10:00
Ben Barham
6a79964ad4 [Refactoring] Do not try to unique '_'
The async refactorings should not try to unique '_' when it's used as
the parameter name. Also skip adding a let binding to the `catch` if the
error parameter is '_'.

Resolves rdar://82158389
2021-08-21 07:30:53 +10:00
Ben Barham
86c47e45f0 [Refactoring] Do not add "async" if function is already async
Convert Function to Async is available on an async function. It could be
useful to run this refactoring still, as it would attempt to convert any
completion-handler functions to their async alternatives. Keep allowing
this, but make sure not to re-add "async" to the function declaration.

Resolves rdar://82156720
2021-08-20 15:24:17 +10:00
Hamish Knight
f58c62c02e [test] Use %refactor-check-compiles in more places
Update a bunch of async refactoring tests to use
%refactor-check-compiles in more cases.
2021-08-18 13:21:06 +01:00
Ben Barham
9fc758adc9 [Test] Update convert_function.swift refactoring tests to also compile
Update the async refactoring tests in `convert_function.swift` to
compile the refactored code (where possible) to check for errors. This
would have prevented the issue with the refactored `Result<Void, ...`>
handler producing erroneous code.
2021-08-12 21:13:20 +10:00
Alex Hoppen
3bea2b728c [Async Refactoring] Handle parenthesis around params that no longer need to be unwrapped after refactoring
Throw another `getSemanticsProvidingExpr` in the async refactoring code for better results.
2021-07-28 18:13:02 +02:00
Ben Barham
fabb02100f [Test] Add @escaping to async refactoring tests
The async refactorings ignore whether a completion handler had
`@escaping` or not. In preparation of fixing this, fix up all functions
to have `@escaping` for their completion handler parameter.

Also some small miscellaneous fixes in order to reduce the number of
warnings output on test failures and also the addition of `REQUIRES:
concurrency` on all tests.
2021-07-24 09:53:17 +10:00
Alex Hoppen
6cbbb7cb1f [Async Refactoring] Split ambiguous (error + success) completion handler calls into success and error case
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
}
```
2021-07-07 11:54:11 +02:00
Alex Hoppen
ec8957972a [Async Refactoring] Get semantics providing expr to decide if call is to completion handler
Resolves rdar://78011350
2021-07-05 17:41:39 +02:00
Alex Hoppen
54fcc90841 [Async Refactoring] Wrap code in a continuation if conversion doesn't yield reasonable results
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
2021-07-02 15:48:41 +02:00
Alex Hoppen
f7c7599a8c [Async Refactoring] Add return keyword if wrapping ReturnStmt is implicit
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
2021-07-01 10:21:46 +02:00
Hamish Knight
e97c781ca1 [Async Refactoring] Add @discardableResult for defaulted completion
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
2021-06-11 11:39:16 +01:00
Hamish Knight
4197bc2325 [Async Refactoring] Fix crash on recursive AST walk
The async refactoring may perform recursive AST
walks in cases such as transforming the body of
a hoisted closure. Make sure this can be handled
by the logic tracking the ASTWalker in the
SourceEntityWalker, such that we don't crash when
later converting the call to a completion callback.

rdar://78693050
2021-05-31 21:47:31 +01:00
Hamish Knight
87703a0716 [test] Add test for async completion handler call in parens
We don't currently handle this, but arguably we
should be able to.
2021-05-14 11:17:59 +01:00
Hamish Knight
7ab5915750 [Refactoring] Don't drop returns with expressions
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
2021-05-14 11:17:59 +01:00
Hamish Knight
aa189f2fe9 [Refactoring] Avoid duplicating return statements
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
2021-05-14 11:17:59 +01:00
Hamish Knight
8cb319b640 [Refactoring] Preserve comments in async transform
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
2021-05-13 14:16:27 +01:00
Hamish Knight
2d750747c2 [Refactoring] [test] Use more strict matching on returns and breaks
Make sure we match against an exact return or
break in these cases, rather than a placeholder.
2021-05-04 14:30:24 +01:00
Alex Hoppen
b2378a401e [Refactoring] Convert completion handler when converting function to async
Convert function to async currently only adds "async" to the function and runs the convert call refactoring on the body.

This was intentional, but it turns out to be somewhat confusing. Instead, run the same refactoring as the add async alternative refactoring but just replace rather than add.

Resolves rdar://77103049
2021-04-30 12:53:22 +02:00
Hamish Knight
c1e4dc3d82 [Refactoring] Improve Void handling for async conversion
When converting a function with a completion handler
that has a Void success parameter, e.g
`(Void?, Error?) -> Void`, or more likely a
`Result<Void, Error>` parameter, make sure to omit
the `-> Void` from the resulting async function
conversion.

In addition, strip any Void bindings from an async
function call, and any explicit Void return values
from inside the async function.

Resolves rdar://75189289
2021-04-30 01:09:04 +01:00
Ben Barham
9cddf498de [Refactoring] Replace attributes when converting to an async function
The replacement range was `FunctionDecl::getRange`, which does not
include attributes. This would cause attributes to be duplicated when
converting a function to async. Override the start with the attribute
start instead so that they are replaced as well.

Resolves rdar://74063741
2021-02-12 07:43:57 +10:00
Ben Barham
783c177956 [Refactoring] Fix stack use after free in new async refactorings
Resolves rdar://73984220
2021-02-05 15:32:35 +10:00
Rintaro Ishizaki
f67b5d9917 [Tests] Disable refactoring/ConvertAsync in ASAN
ASAN detected stack-use-after-scope errors. Disable tests while fixing.

rdar://problem/73984220
2021-02-04 10:17:49 -08:00
Ben Barham
a9073b0922 [Refactoring] Add async refactorings
Adds three refactorings intended to help users migrate their existing
code to use the new async language features:
  1. Convert call to use async alternative
  2. Convert function to async
  3. Add async alternative function

A function is considered to have an async alternative if it has a void
return type and has a void returning closure as its last parameter. A
method to explicitly mark functions as having an async alternative may
be added to make this more accurate in the future (required for eg.
a warning about a call to the non-async version of a function in an
async context).

(1) converts a call to use the new `await` async language syntax. If the
async alternative throws, it will also add `try`. The closure itself is
hoisted out of the call, see the comments on
`AsyncConversionStringBuilder` for specifics.

(2) converts a whole function to `async`, using (1) to convert any calls
in the function to their async alternatives. (3) is similar to (2), but
instead *adds* a function and replaces calls to its
completion/handler/callback closure parameter with `return` or `throws`.

Resolves rdar://68254700
2021-02-03 15:54:46 +10:00