Merge pull request #30746 from ismetanin/fix-it-for-removing-escaping-from-optional-closure-parameter

Add new error diagnostic for escaping optional closures

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -2856,8 +2856,8 @@ ERROR(autoclosure_function_input_nonunit,none,
 ERROR(escaping_non_function_parameter,none,
       "@escaping attribute may only be used in function parameter position", ())
 
-NOTE(escaping_optional_type_argument, none,
-     "closure is already escaping in optional type argument", ())
+ERROR(escaping_optional_type_argument, none,
+      "closure is already escaping in optional type argument", ())
 
 // @_nonEphemeral attribute
 ERROR(non_ephemeral_non_pointer_type,none,

lib/Sema/TypeCheckType.cpp

@@ -2392,13 +2392,14 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
         auto loc = attrs.getLoc(TAK_escaping);
         auto attrRange = getTypeAttrRangeWithAt(Context, loc);
 
-        diagnoseInvalid(repr, loc, diag::escaping_non_function_parameter)
-            .fixItRemove(attrRange);
-
-        // Try to find a helpful note based on how the type is being used
+        // Try to find a better diagnostic based on how the type is being used
         if (options.is(TypeResolverContext::ImmediateOptionalTypeArgument)) {
           diagnoseInvalid(repr, repr->getLoc(),
-                          diag::escaping_optional_type_argument);
+                          diag::escaping_optional_type_argument)
+              .fixItRemove(attrRange);
+        } else {
+          diagnoseInvalid(repr, loc, diag::escaping_non_function_parameter)
+              .fixItRemove(attrRange);
         }
       }
 
@@ -2418,25 +2419,29 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
       attrs.clearAttribute(TAK_autoclosure);
     }
 
+    const auto diagnoseInvalidAttr = [&](TypeAttrKind kind) {
+      if (kind == TAK_escaping) {
+        Type optionalObjectType = ty->getOptionalObjectType();
+        if (optionalObjectType && optionalObjectType->is<AnyFunctionType>()) {
+          return diagnoseInvalid(repr, attrs.getLoc(kind),
+                                 diag::escaping_optional_type_argument);
+        }
+      }
+      return diagnoseInvalid(repr, attrs.getLoc(kind),
+                             diag::attribute_requires_function_type,
+                             TypeAttributes::getAttrName(kind));
+    };
+
     for (auto i : FunctionAttrs) {
       if (!attrs.has(i))
         continue;
 
-      auto diag = diagnoseInvalid(repr, attrs.getLoc(i),
-                                  diag::attribute_requires_function_type,
-                                  TypeAttributes::getAttrName(i));
-
-      // If we see @escaping among the attributes on this type, because it isn't
-      // a function type, we'll remove it.
+      auto diag = diagnoseInvalidAttr(i);
+      // If we see @escaping among the attributes on this type, because it
+      // isn't a function type, we'll remove it.
       if (i == TAK_escaping) {
-        diag.fixItRemove(getTypeAttrRangeWithAt(Context,
-                                                attrs.getLoc(TAK_escaping)));
-        // Specialize the diagnostic for Optionals.
-        if (ty->getOptionalObjectType()) {
-          diag.flush();
-          diagnoseInvalid(repr, repr->getLoc(),
-                          diag::escaping_optional_type_argument);
-        }
+        diag.fixItRemove(
+            getTypeAttrRangeWithAt(Context, attrs.getLoc(TAK_escaping)));
       }
       attrs.clearAttribute(i);
     }

test/attr/attr_escaping.swift

@@ -72,15 +72,11 @@ struct GenericStruct<T> {}
 
 func misuseEscaping(_ a: @escaping Int) {} // expected-error{{@escaping attribute only applies to function types}} {{26-36=}}
 func misuseEscaping(_ a: (@escaping Int)?) {} // expected-error{{@escaping attribute only applies to function types}} {{27-36=}}
-func misuseEscaping(opt a: @escaping ((Int) -> Int)?) {} // expected-error{{@escaping attribute only applies to function types}} {{28-38=}}
-  // expected-note@-1{{closure is already escaping in optional type argument}}
-
-func misuseEscaping(_ a: (@escaping (Int) -> Int)?) {} // expected-error{{@escaping attribute may only be used in function parameter position}} {{27-36=}}
-  // expected-note@-1{{closure is already escaping in optional type argument}}
-func misuseEscaping(nest a: (((@escaping (Int) -> Int))?)) {} // expected-error{{@escaping attribute may only be used in function parameter position}} {{32-41=}}
-  // expected-note@-1{{closure is already escaping in optional type argument}}
-func misuseEscaping(iuo a: (@escaping (Int) -> Int)!) {} // expected-error{{@escaping attribute may only be used in function parameter position}} {{29-38=}}
-  // expected-note@-1{{closure is already escaping in optional type argument}}
+func misuseEscaping(opt a: @escaping ((Int) -> Int)?) {} // expected-error{{closure is already escaping in optional type argument}} {{28-38=}}
+
+func misuseEscaping(_ a: (@escaping (Int) -> Int)?) {} // expected-error{{closure is already escaping in optional type argument}} {{27-36=}}
+func misuseEscaping(nest a: (((@escaping (Int) -> Int))?)) {} // expected-error{{closure is already escaping in optional type argument}} {{32-41=}}
+func misuseEscaping(iuo a: (@escaping (Int) -> Int)!) {} // expected-error{{closure is already escaping in optional type argument}} {{29-38=}}
 
 func misuseEscaping(_ a: Optional<@escaping (Int) -> Int>, _ b: Int) {} // expected-error{{@escaping attribute may only be used in function parameter position}} {{35-44=}}
 func misuseEscaping(_ a: (@escaping (Int) -> Int, Int)) {} // expected-error{{@escaping attribute may only be used in function parameter position}} {{27-36=}}