[CSDiagnostics] Optional closure diagnostic improvement (#60321)

* [NFC] Add a getOptionalityDepth method to return the number of optionals a type has

* [NFC] Use getOptionalityDepth to remove existing code

* [AST] Add a new diagnostic note for suggesting optional chaining

* [CSDiagnostics] Tweak MissingOptionalUnwrapFailure to suggest using optional chaining on closure

* [Test] Add a couple of test cases for perform_optional_chain_on_closure

* [CSDiagnostics] Change DeclRefExpr casts to isa checks

* [AST] Update diagnostic phrasing

* [Sema] Use new diagnostic identifier and update comment

* [Test] Add a new test case for function type

* [Test] Update cfuncs_parse.swift tests to check for new diagnostic note

Author: theblixguy

include/swift/AST/DiagnosticsSema.def

@@ -1166,6 +1166,9 @@ NOTE(unwrap_iuo_initializer,none,
 NOTE(unwrap_with_guard,none,
      "short-circuit using 'guard' to exit this function early "
      "if the optional value contains 'nil'", ())
+NOTE(perform_optional_chain_on_function_type, none,
+     "use optional chaining to call this value of function type when optional "
+     "is non-'nil'", ())
 ERROR(optional_base_not_unwrapped,none,
       "value of optional type %0 must be unwrapped to refer to member %1 of "
       "wrapped base type %2", (Type, DeclNameRef, Type))

include/swift/AST/Types.h

@@ -1254,6 +1254,9 @@ class alignas(1 << TypeAlignInBits) TypeBase
   /// types.
   Type lookThroughAllOptionalTypes(SmallVectorImpl<Type> &optionals);
 
+  /// Return the number of optionals this type is wrapped with.
+  unsigned int getOptionalityDepth();
+
   /// Remove concurrency-related types and constraints from the given
   /// type
   ///

lib/AST/Type.cpp

@@ -881,6 +881,12 @@ Type TypeBase::lookThroughAllOptionalTypes(SmallVectorImpl<Type> &optionals){
   return type;
 }
 
+unsigned int TypeBase::getOptionalityDepth() {
+  SmallVector<Type> types;
+  lookThroughAllOptionalTypes(types);
+  return types.size();
+}
+
 Type TypeBase::stripConcurrency(bool recurse, bool dropGlobalActor) {
   // Look through optionals.
   if (Type optionalObject = getOptionalObjectType()) {

lib/SILGen/SILGenExpr.cpp

@@ -4168,26 +4168,16 @@ RValue RValueEmitter::visitRebindSelfInConstructorExpr(
   auto *otherCtor = E->getCalledConstructor(isChaining)->getDecl();
   assert(otherCtor);
 
-  auto getOptionalityDepth = [](Type ty) {
-    unsigned level = 0;
-    Type objTy = ty->getOptionalObjectType();
-    while (objTy) {
-      ++level;
-      objTy = objTy->getOptionalObjectType();
-    }
-
-    return level;
-  };
-
   // The optionality depth of the 'new self' value. This can be '2' if the ctor
   // we are delegating/chaining to is both throwing and failable, or more if
   // 'self' is optional.
-  auto srcOptionalityDepth = getOptionalityDepth(E->getSubExpr()->getType());
+  auto srcOptionalityDepth = E->getSubExpr()->getType()->getOptionalityDepth();
 
   // The optionality depth of the result type of the enclosing initializer in
   // this context.
-  const auto destOptionalityDepth = getOptionalityDepth(
-      ctorDecl->mapTypeIntoContext(ctorDecl->getResultInterfaceType()));
+  const auto destOptionalityDepth =
+      ctorDecl->mapTypeIntoContext(ctorDecl->getResultInterfaceType())
+          ->getOptionalityDepth();
 
   // The subexpression consumes the current 'self' binding.
   assert(SGF.SelfInitDelegationState == SILGenFunction::NormalSelf

lib/Sema/CSDiagnostics.cpp

@@ -1566,6 +1566,26 @@ bool MissingOptionalUnwrapFailure::diagnoseAsError() {
   emitDiagnosticAt(unwrappedExpr->getLoc(), diag::optional_not_unwrapped,
                    baseType, unwrappedType);
 
+  // If this is a function type, suggest using optional chaining to
+  // call it.
+  if (unwrappedType->lookThroughAllOptionalTypes()->is<FunctionType>()) {
+    bool isDeclRefExpr = false;
+    if (isa<DeclRefExpr>(unwrappedExpr)) {
+      isDeclRefExpr = true;
+    } else if (auto fve = dyn_cast<ForceValueExpr>(unwrappedExpr)) {
+      isDeclRefExpr = isa<DeclRefExpr>(fve->getSubExpr());
+    } else if (auto boe = dyn_cast<BindOptionalExpr>(unwrappedExpr)) {
+      isDeclRefExpr = isa<DeclRefExpr>(boe->getSubExpr());
+    }
+    if (isDeclRefExpr) {
+      auto depth = baseType->getOptionalityDepth();
+      auto diag = emitDiagnosticAt(unwrappedExpr->getLoc(),
+                                   diag::perform_optional_chain_on_function_type);
+      auto fixItString = std::string(depth, '?');
+      diag.fixItInsertAfter(unwrappedExpr->getEndLoc(), fixItString);
+    }
+  }
+
   // If the expression we're unwrapping is the only reference to a
   // local variable whose type isn't explicit in the source, then
   // offer unwrapping fixits on the initializer as well.

test/ClangImporter/cfuncs_parse.swift

@@ -24,26 +24,29 @@ func test_cfunc2(_ i: Int) {
 func test_cfunc3_a() {
   let b = cfunc3( { (a : Double, b : Double) -> Double in a + b } )
   _ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
-  // expected-note@-1{{coalesce}}
-  // expected-note@-2{{force-unwrap}}
+  // expected-note@-1{{use optional chaining to call this value of function type when optional is non-'nil'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   _ = b!(1.5, 2.5) as Double
   _ = b as Double// expected-error{{cannot convert value of type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') to type 'Double' in coercion}}
 }
 
 func test_cfunc3_b() {
   let b = cfunc3( { a, b in a + b } )
   _ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
-  // expected-note@-1{{coalesce}}
-  // expected-note@-2{{force-unwrap}}
+  // expected-note@-1{{use optional chaining to call this value of function type when optional is non-'nil'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   _ = b!(1.5, 2.5) as Double
   _ = b as Double// expected-error{{cannot convert value of type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') to type 'Double' in coercion}}
 }
 
 func test_cfunc3_c() {
   let b = cfunc3({ $0 + $1 })
   _ = b(1.5, 2.5) as Double // expected-error{{value of optional type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') must be unwrapped to a value of type 'double_bin_op_block' (aka '(Double, Double) -> Double')}}
-  // expected-note@-1{{coalesce}}
-  // expected-note@-2{{force-unwrap}}
+  // expected-note@-1{{use optional chaining to call this value of function type when optional is non-'nil'}}
+  // expected-note@-2{{coalesce}}
+  // expected-note@-3{{force-unwrap}}
   _ = b!(1.5, 2.5) as Double
   _ = b as Double// expected-error{{cannot convert value of type 'double_bin_op_block?' (aka 'Optional<(Double, Double) -> Double>') to type 'Double' in coercion}}
 }

test/Constraints/optional.swift

@@ -512,3 +512,46 @@ func rdar85166519() {
 
 // https://github.com/apple/swift/issues/58539
 if let x = nil {} // expected-error{{'nil' requires a contextual type}}
+
+// https://github.com/apple/swift/issues/56699
+let singleOptionalClosure: (() -> Void)? = nil
+let doubleOptionalClosure: (() -> Void)?? = nil
+singleOptionalClosure()
+// expected-error@-1 {{value of optional type}}
+// expected-note@-2 {{use optional chaining to call this value of function type when optional is non-'nil'}} {{22-22=?}}
+// expected-note@-3 {{coalesce}}
+// expected-note@-4 {{force-unwrap}}
+
+doubleOptionalClosure()
+// expected-error@-1 {{value of optional type}}
+// expected-note@-2 {{use optional chaining to call this value of function type when optional is non-'nil'}} {{22-22=??}}
+// expected-note@-3 {{coalesce}}
+// expected-note@-4 {{force-unwrap}}
+
+doubleOptionalClosure?()
+// expected-error@-1 {{value of optional type}}
+// expected-note@-2 {{use optional chaining to call this value of function type when optional is non-'nil'}} {{23-23=?}}
+// expected-note@-3 {{coalesce}}
+// expected-note@-4 {{force-unwrap}}
+
+doubleOptionalClosure!()
+// expected-error@-1 {{value of optional type}}
+// expected-note@-2 {{use optional chaining to call this value of function type when optional is non-'nil'}} {{23-23=?}}
+// expected-note@-3 {{coalesce}}
+// expected-note@-4 {{force-unwrap}}
+
+struct FunctionContainer {
+  func test() {}
+}
+
+func testFunctionContainerMethodCall(container: FunctionContainer?) {
+  let fn = container?.test
+   // expected-note@-1 {{short-circuit}}
+   // expected-note@-2 {{coalesce}}
+   // expected-note@-3 {{force-unwrap}}
+  fn()
+  // expected-error@-1 {{value of optional type}}
+  // expected-note@-2 {{use optional chaining to call this value of function type when optional is non-'nil'}} {{5-5=?}}
+  // expected-note@-3 {{coalesce}}
+  // expected-note@-4 {{force-unwrap}}
+}