Merge pull request #38602 from hamishknight/errors-as-warnings-5.5

[5.5] [Async Refactoring] Add parens around custom error cast

Author: akyrtzi

lib/IDE/Refactoring.cpp

@@ -7612,17 +7612,32 @@ class AsyncConverter : private SourceEntityWalker {
     OS << tok::r_paren;
   }
 
-  /// If the error type of \p HandlerDesc is more specialized than \c Error,
-  /// adds an 'as! CustomError' cast to the more specialized error type to the
-  /// output stream.
-  void
-  addCastToCustomErrorTypeIfNecessary(const AsyncHandlerDesc &HandlerDesc) {
-    const ASTContext &Ctx = HandlerDesc.getHandler()->getASTContext();
+  /// Adds a forwarded error argument to a completion handler call. If the error
+  /// type of \p HandlerDesc is more specialized than \c Error, an
+  /// 'as! CustomError' cast to the more specialized error type will be added to
+  /// the output stream.
+  void addForwardedErrorArgument(StringRef ErrorName,
+                                 const AsyncHandlerDesc &HandlerDesc) {
+    // If the error type is already Error, we can pass it as-is.
     auto ErrorType = *HandlerDesc.getErrorType();
-    if (ErrorType->getCanonicalType() != Ctx.getExceptionType()) {
-      OS << " " << tok::kw_as << tok::exclaim_postfix << " ";
-      ErrorType->lookThroughSingleOptionalType()->print(OS);
+    if (ErrorType->getCanonicalType() == getASTContext().getExceptionType()) {
+      OS << ErrorName;
+      return;
     }
+
+    // Otherwise we need to add a force cast to the destination custom error
+    // type. If this is for an Error? parameter, we'll need to add parens around
+    // the cast to silence a compiler warning about force casting never
+    // producing nil.
+    auto RequiresParens = HandlerDesc.getErrorParam().hasValue();
+    if (RequiresParens)
+      OS << tok::l_paren;
+
+    OS << ErrorName << " " << tok::kw_as << tok::exclaim_postfix << " ";
+    ErrorType->lookThroughSingleOptionalType()->print(OS);
+
+    if (RequiresParens)
+      OS << tok::r_paren;
   }
 
   /// If \p T has a natural default value like \c nil for \c Optional or \c ()
@@ -7653,8 +7668,7 @@ class AsyncConverter : private SourceEntityWalker {
     if (HandlerDesc.HasError && Index == HandlerDesc.params().size() - 1) {
       // The error parameter is the last argument of the completion handler.
       if (ResultName.empty()) {
-        OS << "error";
-        addCastToCustomErrorTypeIfNecessary(HandlerDesc);
+        addForwardedErrorArgument("error", HandlerDesc);
       } else {
         addDefaultValueOrPlaceholder(HandlerDesc.params()[Index].getPlainType());
       }
@@ -7723,8 +7737,8 @@ class AsyncConverter : private SourceEntityWalker {
         OS << tok::period_prefix << "success" << tok::l_paren << ResultName
            << tok::r_paren;
       } else {
-        OS << tok::period_prefix << "failure" << tok::l_paren << "error";
-        addCastToCustomErrorTypeIfNecessary(HandlerDesc);
+        OS << tok::period_prefix << "failure" << tok::l_paren;
+        addForwardedErrorArgument("error", HandlerDesc);
         OS << tok::r_paren;
       }
       break;

test/refactoring/ConvertAsync/basic.swift

@@ -133,7 +133,7 @@ func customError(completion: (String?, CustomError?) -> Void) { }
 // ASYNC-CUSTOMERROR-NEXT: let result = try await customError()
 // ASYNC-CUSTOMERROR-NEXT: completion(result, nil)
 // ASYNC-CUSTOMERROR-NEXT: } catch {
-// ASYNC-CUSTOMERROR-NEXT: completion(nil, error as! CustomError)
+// ASYNC-CUSTOMERROR-NEXT: completion(nil, (error as! CustomError))
 // ASYNC-CUSTOMERROR-NEXT: }
 // ASYNC-CUSTOMERROR-NEXT: }
 // ASYNC-CUSTOMERROR-NEXT: }
@@ -308,7 +308,7 @@ func genericError<E>(completion: (String?, E?) -> Void) where E: Error { }
 // GENERIC-ERROR-NEXT: let result: String = try await genericError()
 // GENERIC-ERROR-NEXT: completion(result, nil)
 // GENERIC-ERROR-NEXT: } catch {
-// GENERIC-ERROR-NEXT: completion(nil, error as! E)
+// GENERIC-ERROR-NEXT: completion(nil, (error as! E))
 // GENERIC-ERROR-NEXT: }
 // GENERIC-ERROR-NEXT: }
 // GENERIC-ERROR-NEXT: }

test/refactoring/ConvertAsync/variable_as_callback.swift

@@ -317,7 +317,7 @@ func testGenericErrorVariableCompletionHandler<MyGenericError>(completionHandler
 // GENERIC-ERROR-NEXT:   let result: String = try await genericError()
 // GENERIC-ERROR-NEXT:   completionHandler(result, nil)
 // GENERIC-ERROR-NEXT: } catch {
-// GENERIC-ERROR-NEXT:   completionHandler(nil, error as! MyGenericError)
+// GENERIC-ERROR-NEXT:   completionHandler(nil, (error as! MyGenericError))
 // GENERIC-ERROR-NEXT: }
 
 // RUN: %refactor-check-compiles -convert-to-async -dump-text -source-filename %s -pos=%(line+2):1 | %FileCheck -check-prefix=DEFAULT-ARGS-FUNC %s