Merge pull request #9583 from bitjammer/rdar-32143617-void-tuple-input-type

Migrator/QoI: Function input type: Don't fix Void to (Void), but fix …

Author: bitjammer

include/swift/AST/DiagnosticsSema.def

@@ -3183,6 +3183,8 @@ ERROR(objc_convention_invalid,none,
       " with '@convention(%1)'", (Type, StringRef))
 ERROR(function_type_no_parens,none,
       "single argument function types require parentheses", ())
+WARNING(paren_void_probably_void,none,
+      "when calling this function in Swift 4 or later, you must pass a '()' tuple; did you mean for the input type to be '()'?", ())
 NOTE(not_objc_empty_protocol_composition,none,
      "'Any' is not considered '@objc'; use 'AnyObject' instead", ())
 NOTE(not_objc_protocol,none,

include/swift/Migrator/FixitFilter.h

@@ -90,6 +90,12 @@ struct FixitFilter {
       return false;
     }
 
+    // Trying to add '_ in' to a closure signature can be counterproductive when
+    // fixing function signatures like (Void) -> () to () -> ().
+    if (Info.ID == diag::closure_argument_list_missing.ID) {
+      return false;
+    }
+
     if (Kind == DiagnosticKind::Error)
       return true;
 
@@ -120,7 +126,8 @@ struct FixitFilter {
         Info.ID == diag::override_swift3_objc_inference.ID ||
         Info.ID == diag::objc_inference_swift3_objc_derived.ID ||
         Info.ID == diag::missing_several_cases.ID ||
-        Info.ID == diag::missing_particular_case.ID)
+        Info.ID == diag::missing_particular_case.ID ||
+        Info.ID == diag::paren_void_probably_void.ID)
       return true;
 
     return false;

lib/Sema/CSDiag.cpp

@@ -5143,9 +5143,20 @@ static bool diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI,
           tuple->hasTrailingClosure())
         TC.diagnose(loc, diag::extra_trailing_closure_in_call)
             .highlight(arg->getSourceRange());
-      else if (Parameters.empty())
-        TC.diagnose(loc, diag::extra_argument_to_nullary_call)
+      else if (Parameters.empty()) {
+        auto Paren = dyn_cast<ParenExpr>(ArgExpr);
+        Expr *SubExpr = nullptr;
+        if (Paren) {
+          SubExpr = Paren->getSubExpr();
+        }
+        if (SubExpr && SubExpr->getType() && SubExpr->getType()->isVoid()) {
+          TC.diagnose(loc, diag::extra_argument_to_nullary_call)
+            .fixItRemove(SubExpr->getSourceRange());
+        } else {
+          TC.diagnose(loc, diag::extra_argument_to_nullary_call)
             .highlight(ArgExpr->getSourceRange());
+        }
+      }
       else if (name.empty())
         TC.diagnose(loc, diag::extra_argument_positional)
             .highlight(arg->getSourceRange());

lib/Sema/TypeCheckType.cpp

@@ -2324,13 +2324,43 @@ Type TypeResolver::resolveASTFunctionType(FunctionTypeRepr *repr,
   if (!isa<TupleTypeRepr>(repr->getArgsTypeRepr()) &&
       !repr->isWarnedAbout()) {
     auto args = repr->getArgsTypeRepr();
-    TC.diagnose(args->getStartLoc(), diag::function_type_no_parens)
-      .highlight(args->getSourceRange())
-      .fixItInsert(args->getStartLoc(), "(")
-      .fixItInsertAfter(args->getEndLoc(), ")");
+
+    bool isVoid = false;
+    if (const auto Void = dyn_cast<SimpleIdentTypeRepr>(args)) {
+      if (Void->getIdentifier().str() == "Void") {
+        isVoid = true;
+      }
+    }
+    if (isVoid) {
+      TC.diagnose(args->getStartLoc(), diag::function_type_no_parens)
+        .fixItReplace(args->getStartLoc(), "()");
+    } else {
+      TC.diagnose(args->getStartLoc(), diag::function_type_no_parens)
+        .highlight(args->getSourceRange())
+        .fixItInsert(args->getStartLoc(), "(")
+        .fixItInsertAfter(args->getEndLoc(), ")");
+    }
 
     // Don't emit this warning three times when in generics.
     repr->setWarned();
+  } else if (isa<TupleTypeRepr>(repr->getArgsTypeRepr()) &&
+             !repr->isWarnedAbout()) {
+    // If someone wrote (Void) -> () in Swift 3, they probably meant
+    // () -> (), but (Void) -> () is (()) -> () so emit a warning
+    // asking if they meant () -> ().
+    auto args = repr->getArgsTypeRepr();
+    if (const auto Tuple = dyn_cast<TupleTypeRepr>(args)) {
+      if (Tuple->getNumElements() == 1) {
+        if (const auto Void =
+            dyn_cast<SimpleIdentTypeRepr>(Tuple->getElement(0))) {
+          if (Void->getIdentifier().str() == "Void") {
+            TC.diagnose(args->getStartLoc(), diag::paren_void_probably_void)
+              .fixItReplace(args->getSourceRange(), "()");
+            repr->setWarned();
+          }
+        }
+      }
+    }
   }
 
   // SIL uses polymorphic function types to resolve overloaded member functions.

test/Constraints/diagnostics.swift

@@ -84,12 +84,12 @@ for j in i.wibble(a, a) { // expected-error {{type 'A' does not conform to proto
 }
 
 // Generic as part of function/tuple types
-func f6<T:P2>(_ g: (Void) -> T) -> (c: Int, i: T) {
+func f6<T:P2>(_ g: (Void) -> T) -> (c: Int, i: T) { // expected-warning {{when calling this function in Swift 4 or later, you must pass a '()' tuple; did you mean for the input type to be '()'?}} {{20-26=()}}
   return (c: 0, i: g())
 }
 
 func f7() -> (c: Int, v: A) {
-  let g: (Void) -> A = { return A() }
+  let g: (Void) -> A = { return A() } // expected-warning {{when calling this function in Swift 4 or later, you must pass a '()' tuple; did you mean for the input type to be '()'?}} {{10-16=()}}
   return f6(g) // expected-error {{cannot convert return expression of type '(c: Int, i: A)' to return type '(c: Int, v: A)'}}
 }
 
@@ -966,4 +966,4 @@ class SR_4692_b {
 
   private func f(x: Int, y: Bool) {
   }
-}
+}

test/Migrator/fixit_void.swift

@@ -0,0 +1,17 @@
+// RUN: %target-swift-frontend -typecheck %s -swift-version 3
+// RUN: %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t.result -swift-version 3
+// RUN: diff -u %s.expected %t.result
+// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 4
+
+func takesNothing(_ f: () -> ()) {
+  f()
+  f(())
+}
+
+func takesVoidFunction(_ f: (Void) -> ()) {
+  f()
+  f(())
+}
+
+takesNothing { print("Hello") }
+takesVoidFunction { print("Hello") }

test/Migrator/fixit_void.swift.expected

@@ -0,0 +1,17 @@
+// RUN: %target-swift-frontend -typecheck %s -swift-version 3
+// RUN: %target-swift-frontend -typecheck -update-code -primary-file %s -emit-migrated-file-path %t.result -swift-version 3
+// RUN: diff -u %s.expected %t.result
+// RUN: %target-swift-frontend -typecheck %s.expected -swift-version 4
+
+func takesNothing(_ f: () -> ()) {
+  f()
+  f()
+}
+
+func takesVoidFunction(_ f: () -> ()) {
+  f()
+  f()
+}
+
+takesNothing { print("Hello") }
+takesVoidFunction { print("Hello") }

test/Parse/type_expr.swift

@@ -298,3 +298,5 @@ func complexSequence() {
   // expected-error @-3 {{expected member name or constructor call after type name}}
   // expected-note @-4 {{use '.self' to reference the type object}} {{11-11=(}} {{36-36=).self}}
 }
+
+func takesVoid(f: Void -> ()) {} // expected-error {{single argument function types require parentheses}} {{19-23=()}}

test/expr/closure/single_expr.swift

@@ -23,7 +23,7 @@ func testMap(_ array: [Int]) {
 // Nested single-expression closures -- <rdar://problem/20931915>
 class NestedSingleExpr {
   private var b: Bool = false
-  private func callClosure(_ callback: (Void) -> Void) {}
+  private func callClosure(_ callback: () -> Void) {}
 
   func call() {
     callClosure { [weak self] in