Skip to content

Commit 921443e

Browse files
ahoppenahoppen
committed
[Refactoring] Support creation of async legacy bodies even if a parameter is not optional
If the parameter is not an `Optional`, add a placeholder instead that the user can fill with a sensible default value.
1 parent dd978cc commit 921443e

File tree

3 files changed

+58
-33
lines changed

3 files changed

+58
-33
lines changed

‎lib/IDE/Refactoring.cpp‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4981,28 +4981,7 @@ class AsyncConverter : private SourceEntityWalker {
49814981
"if the original function throws");
49824982
return false;
49834983
}
4984-
switch (TopHandler.Type) {
4985-
case HandlerType::INVALID:
4986-
return false;
4987-
case HandlerType::PARAMS: {
4988-
if (TopHandler.HasError) {
4989-
// The non-error parameters must be optional so that we can set them to
4990-
// nil in the error case.
4991-
// The error parameter must be optional so we can set it to nil in the
4992-
// success case.
4993-
// Otherwise we can't synthesize the values to return for these
4994-
// parameters.
4995-
return llvm::all_of(TopHandler.params(),
4996-
[](AnyFunctionType::Param Param) -> bool {
4997-
return Param.getPlainType()->isOptional();
4998-
});
4999-
} else {
5000-
return true;
5001-
}
5002-
}
5003-
case HandlerType::RESULT:
5004-
return true;
5005-
}
4984+
return TopHandler.isValid();
50064985
}
50074986

50084987

@@ -5675,6 +5654,21 @@ class AsyncConverter : private SourceEntityWalker {
56755654
}
56765655
}
56775656

5657+
/// If \p T has a natural default value like \c nil for \c Optional or \c ()
5658+
/// for \c Void, add that default value to the output. Otherwise, add a
5659+
/// placeholder that contains \p T's name as the hint.
5660+
void addDefaultValueOrPlaceholder(Type T) {
5661+
if (T->isOptional()) {
5662+
OS << tok::kw_nil;
5663+
} else if (T->isVoid()) {
5664+
OS << "()";
5665+
} else {
5666+
OS << "<#";
5667+
T.print(OS);
5668+
OS << "#>";
5669+
}
5670+
}
5671+
56785672
/// Adds the \c Index -th parameter to the completion handler described by \p
56795673
/// HanderDesc.
56805674
/// If \p HasResult is \c true, it is assumed that a variable named
@@ -5691,11 +5685,11 @@ class AsyncConverter : private SourceEntityWalker {
56915685
OS << "error";
56925686
addCastToCustomErrorTypeIfNecessary(HandlerDesc);
56935687
} else {
5694-
OS << tok::kw_nil;
5688+
addDefaultValueOrPlaceholder(HandlerDesc.params()[Index].getPlainType());
56955689
}
56965690
} else {
56975691
if (!HasResult) {
5698-
OS << tok::kw_nil;
5692+
addDefaultValueOrPlaceholder(HandlerDesc.params()[Index].getPlainType());
56995693
} else if (HandlerDesc
57005694
.getSuccessParamAsyncReturnType(
57015695
HandlerDesc.params()[Index].getPlainType())

‎test/refactoring/ConvertAsync/basic.swift‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,18 @@ func errorOnly(completion: (Error?) -> Void) { }
111111
// ASYNC-ERRORONLY-NEXT: }
112112
// ASYNC-ERRORONLY: func errorOnly() async throws { }
113113

114-
// RUN: %refactor-check-compiles -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=ASYNC-ERRORNONOPTIONALRESULT %s
114+
// RUN: %refactor -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=ASYNC-ERRORNONOPTIONALRESULT %s
115115
func errorNonOptionalResult(completion: (String, Error?) -> Void) { }
116-
// We cannot convert the deprecated non-async method to call the async method because we can't synthesize the non-optional completion param. Smoke check for some keywords that would indicate we rewrote the body.
117-
// ASYNC-ERRORNONOPTIONALRESULT-NOT: detach
118-
// ASYNC-ERRORNONOPTIONALRESULT-NOT: await
116+
// ASYNC-ERRORNONOPTIONALRESULT: {
117+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: async {
118+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: do {
119+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: let result = try await errorNonOptionalResult()
120+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: completion(result, nil)
121+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: } catch {
122+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: completion(<#String#>, error)
123+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: }
124+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: }
125+
// ASYNC-ERRORNONOPTIONALRESULT-NEXT: }
119126
// ASYNC-ERRORNONOPTIONALRESULT: func errorNonOptionalResult() async throws -> String { }
120127

121128
// RUN: %refactor-check-compiles -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=ASYNC-CUSTOMERROR %s
@@ -244,9 +251,18 @@ func mixed(_ completion: (String?, Int) -> Void) { }
244251
// MIXED-NEXT: }
245252
// MIXED: func mixed() async -> (String?, Int) { }
246253

247-
// RUN: %refactor-check-compiles -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=MIXED-OPTIONAL-ERROR %s
254+
// RUN: %refactor -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1
248255
func mixedOptionalError(_ completion: (String?, Int, Error?) -> Void) { }
249-
// MIXED-OPTIONAL-ERROR-NOT: async {
256+
// MIXED-OPTIONAL-ERROR: {
257+
// MIXED-OPTIONAL-ERROR-NEXT: async {
258+
// MIXED-OPTIONAL-ERROR-NEXT: do {
259+
// MIXED-OPTIONAL-ERROR-NEXT: let result = try await mixedOptionalError()
260+
// MIXED-OPTIONAL-ERROR-NEXT: completion(result.0, result.1, nil)
261+
// MIXED-OPTIONAL-ERROR-NEXT: } catch {
262+
// MIXED-OPTIONAL-ERROR-NEXT: completion(nil, <#Int#>, error)
263+
// MIXED-OPTIONAL-ERROR-NEXT: }
264+
// MIXED-OPTIONAL-ERROR-NEXT: }
265+
// MIXED-OPTIONAL-ERROR-NEXT: }
250266
// MIXED-OPTIONAL-ERROR: func mixedOptionalError() async throws -> (String, Int) { }
251267

252268
// RUN: %refactor-check-compiles -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix=MIXED-ERROR %s
@@ -450,9 +466,18 @@ func tooVoidProperAndErrorCompletion(completion: (Void?, String?, Error?) -> Voi
450466
// VOID-PROPER-AND-ERROR-HANDLER-NEXT: }
451467
// VOID-PROPER-AND-ERROR-HANDLER: func tooVoidProperAndErrorCompletion() async throws -> (Void, String) {}
452468

453-
// RUN: %refactor -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1 | %FileCheck -check-prefix VOID-AND-ERROR-HANDLER %s
469+
// RUN: %refactor-check-compiles -add-async-alternative -dump-text -source-filename %s -pos=%(line+1):1
454470
func voidAndErrorCompletion(completion: (Void, Error?) -> Void) {}
455-
// VOID-AND-ERROR-HANDLER-NOT: async {
471+
// VOID-AND-ERROR-HANDLER: {
472+
// VOID-AND-ERROR-HANDLER-NEXT: async {
473+
// VOID-AND-ERROR-HANDLER-NEXT: do {
474+
// VOID-AND-ERROR-HANDLER-NEXT: try await voidAndErrorCompletion()
475+
// VOID-AND-ERROR-HANDLER-NEXT: completion((), nil)
476+
// VOID-AND-ERROR-HANDLER-NEXT: } catch {
477+
// VOID-AND-ERROR-HANDLER-NEXT: completion((), error)
478+
// VOID-AND-ERROR-HANDLER-NEXT: }
479+
// VOID-AND-ERROR-HANDLER-NEXT: }
480+
// VOID-AND-ERROR-HANDLER-NEXT: }
456481
// VOID-AND-ERROR-HANDLER: func voidAndErrorCompletion() async throws {}
457482

458483
// 2. Check that the various ways to call a function (and the positions the

‎test/refactoring/ConvertAsync/variable_as_callback.swift‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,16 @@ func testErrorOnlyWithVariableCompletionHandler(completionHandler: (Error?) -> V
8989
// ERROR-ONLY-VARIABLE-COMPLETION-HANDLER-NEXT: completionHandler(error)
9090
// ERROR-ONLY-VARIABLE-COMPLETION-HANDLER-NEXT: }
9191

92-
// FIXME: %refactor -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+2):3
92+
// RUN: %refactor -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+2):3 | %FileCheck -check-prefix=ERROR-NON-OPTIONAL-RESULT %s
9393
func testErrorNonOptionalResultWithVariableCompletionHandler(completionHandler: (String, Error?) -> Void) {
9494
errorNonOptionalResult(completion: completionHandler)
9595
}
96+
// ERROR-NON-OPTIONAL-RESULT: do {
97+
// ERROR-NON-OPTIONAL-RESULT-NEXT: let result = try await errorNonOptionalResult()
98+
// ERROR-NON-OPTIONAL-RESULT-NEXT: completionHandler(result, nil)
99+
// ERROR-NON-OPTIONAL-RESULT-NEXT: } catch {
100+
// ERROR-NON-OPTIONAL-RESULT-NEXT: completionHandler(<#String#>, error)
101+
// ERROR-NON-OPTIONAL-RESULT-NEXT: }
96102

97103
// RUN: %refactor -convert-call-to-async-alternative -dump-text -source-filename %s -pos=%(line+2):3 | %FileCheck -check-prefix=ALIAS-VARIABLE-COMPLETION-HANDLER %s
98104
func testAliasWithVariableCompletionHandler(completionHandler: SomeCallback) {

0 commit comments

Comments
 (0)