Revert "[Fixit] Add a fixit for converting non-trailing closures to trailing closures (#3317)"

This patch needs some polish to fix more false positives found by @rintaro and @lattner

Author: nkcsgexi

include/swift/AST/DiagnosticsSema.def

@@ -2082,10 +2082,6 @@ ERROR(string_literal_broken_proto,none,
 ERROR(bool_type_broken,none,
       "could not find a Bool type defined for 'is'", ())
 
-NOTE(convert_to_trailing_closure,none,
-"use trailing closure to simplify arguments", ())
-
-
 // Array literals
 ERROR(array_protocol_broken,none,
       "ArrayLiteralConvertible protocol definition is broken", ())

lib/Sema/MiscDiagnostics.cpp

@@ -1984,141 +1984,6 @@ static void diagAvailability(TypeChecker &TC, const Expr *E,
   const_cast<Expr*>(E)->walk(walker);
 }
 
-namespace {
-
-static ClosureExpr* getLastArgAsClosure(Expr* Arg) {
-  if (auto Paren = dyn_cast<ParenExpr>(Arg)) {
-
-    // If the argument is paren expression, get the sub expression as closure.
-    if (!Paren->hasTrailingClosure())
-      return dyn_cast_or_null<ClosureExpr>(Paren->getSubExpr());
-  } else if (auto Tuple = dyn_cast<TupleExpr>(Arg)) {
-
-    // If the argument is tuple expression, get the last expression in the tuple
-    // as closure.
-    if (!Tuple->hasTrailingClosure() && Tuple->getNumElements() > 0)
-      return dyn_cast<ClosureExpr>(Tuple->getElement(Tuple->getNumElements() - 1));
-  }
-  return nullptr;
-}
-
-static bool areIdentifiersSame(Identifier Left, Identifier Right) {
-
-  // If both are anonymous, returns true.
-  if (Left.empty() && Right.empty())
-    return true;
-
-  // If one is anonymous and the other is not, returns false.
-  if (Left.empty() || Right.empty())
-    return false;
-
-  // If both are not anonymous, compare the content.
-  return Left.str() == Right.str();
-}
-
-static bool willCauseAmbiguity(TypeChecker &TC, FuncDecl *SelectedFD) {
-  auto SelectedArgNames = SelectedFD->getEffectiveFullName().getArgumentNames();
-  auto SelectedArgNameExcludingClosure = SelectedArgNames.slice(0,
-    SelectedArgNames.size() - 1);
-
-  // Consider all decls from the same context with the simple
-  // identifier with SelectedFD.
-  for (ValueDecl *VD : TC.lookupUnqualified(SelectedFD->getDeclContext(),
-                                            SelectedFD->getName(), SourceLoc())) {
-
-    // Make sure the overload is not the selected function.
-    if (VD == SelectedFD)
-      continue;
-
-    // Make sure the overload is a function.
-    FuncDecl *FD = dyn_cast<FuncDecl>(VD);
-    if (!FD)
-      continue;
-    auto ArgNames = FD->getEffectiveFullName().getArgumentNames();
-
-    // If the overload requires more arguments than the selected; there are no
-    // chance of conflicts.
-    if (ArgNames.size() < SelectedArgNames.size())
-      continue;
-
-    // Assuming there is a conflict.
-    bool Conflict = true;
-    for (unsigned I = 0; I < SelectedArgNameExcludingClosure.size(); I ++) {
-      if (!areIdentifiersSame(ArgNames[I], SelectedArgNameExcludingClosure[I])) {
-        // Different argument names disprove our assumption.
-        Conflict = false;
-      }
-    }
-    if (Conflict)
-      return true;
-  }
-  return false;
-}
-
-static FuncDecl* digForFuncDecl(Expr *Fn) {
-  auto FD = dyn_cast_or_null<FuncDecl>(Fn->getReferencedDecl().getDecl());
-  if (FD)
-    return FD;
-  if (auto DE = dyn_cast<DotSyntaxCallExpr>(Fn)) {
-    return digForFuncDecl(DE->getFn());
-  }
-  return nullptr;
-}
-
-static void
-applyConvertToTrailingClosureFixit(TypeChecker &TC, SourceManager &SM,
-                                   ClosureExpr *Closure, Expr *Arg) {
-  if (auto *Paren = dyn_cast<ParenExpr>(Arg)) {
-    SourceRange LRemove(Paren->getLParenLoc(),
-                        Paren->getLParenLoc().getAdvancedLoc(1));
-    SourceRange RRemove(Paren->getRParenLoc(),
-                        Paren->getRParenLoc().getAdvancedLoc(1));
-
-    // Remove the left and right paren.
-    TC.diagnose(Closure->getStartLoc(), diag::convert_to_trailing_closure).
-      fixItRemove(LRemove).fixItRemove(RRemove);
-  } else if (auto *Tuple = dyn_cast<TupleExpr>(Arg)) {
-    if (Tuple->getNumElements() > 1) {
-      SourceLoc ReplaceStart = Lexer::getLocForEndOfToken(SM,
-        Tuple->getElement(Tuple->getNumElements() - 2)->getEndLoc());
-      SourceRange Replace(ReplaceStart, Closure->getStartLoc());
-      SourceRange ToRemove(Tuple->getRParenLoc(),
-                           Tuple->getRParenLoc().getAdvancedLoc(1));
-
-      // Closing the tuple before the closure; and remove the right paren after the
-      // closure.
-      TC.diagnose(Closure->getStartLoc(), diag::convert_to_trailing_closure).
-        fixItReplace(Replace, ") ").fixItRemove(ToRemove);
-    } else {
-      SourceRange LRemove(Tuple->getLParenLoc(), Closure->getStartLoc());
-      SourceRange RRemove(Lexer::getLocForEndOfToken(SM,Closure->getEndLoc()),
-                          Lexer::getLocForEndOfToken(SM, Tuple->getEndLoc()));
-      TC.diagnose(Closure->getStartLoc(), diag::convert_to_trailing_closure).
-        fixItRemove(LRemove).fixItRemove(RRemove);
-    }
-  }
-}
-}
-
-static void diagConvertToTrailingClosure(TypeChecker &TC, const Expr *E,
-                                      DeclContext *DC) {
-  auto* AE = dyn_cast<ApplyExpr>(E);
-  if (!AE)
-    return;
-
-  auto* Closure = getLastArgAsClosure(AE->getArg());
-  if (!Closure)
-    return;
-  if (auto* FD = digForFuncDecl(AE->getFn())) {
-    // Check the condition whether converting will cause ambiguity.
-    if (!willCauseAmbiguity(TC, FD))
-      // Without ambiguity, we can proceed to fix.
-      applyConvertToTrailingClosureFixit(TC, DC->getASTContext().SourceMgr,
-                                         Closure, AE->getArg());
-  }
-}
-
-
 //===----------------------------------------------------------------------===//
 // Per func/init diagnostics
 //===----------------------------------------------------------------------===//
@@ -3345,7 +3210,6 @@ void swift::performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,
   diagRecursivePropertyAccess(TC, E, DC);
   diagnoseImplicitSelfUseInClosure(TC, E, DC);
   diagAvailability(TC, E, const_cast<DeclContext*>(DC));
-  diagConvertToTrailingClosure(TC, E, const_cast<DeclContext*>(DC));
   if (TC.Context.LangOpts.EnableObjCInterop)
     diagDeprecatedObjCSelectors(TC, DC, E);
 }

test/ClangModules/blocks_parse.swift

@@ -11,7 +11,7 @@ func useString(_ s: String) {}
 accepts_block { }
 someNSString.enumerateLines {(s:String?) in }
 someNSString.enumerateLines {s in }
-someNSString.enumerateLines({ useString($0) }) // expected-note {{use trailing closure to simplify arguments}}
+someNSString.enumerateLines({ useString($0) })
 
 accepts_block(/*not a block=*/()) // expected-error{{cannot convert value of type '()' to expected argument type 'my_block_t' (aka '() -> ()'}}
 

test/Misc/single_expr_closure_conversion.swift

@@ -45,7 +45,7 @@ class C {
 	var a = A()
 
 	func act() {
-		a.dispatch({() -> Void in  // expected-note {{use trailing closure to simplify arguments}}
+		a.dispatch({() -> Void in
                   self.prop // expected-warning {{expression of type 'Int' is unused}}
                 })
 	}

test/Sema/convert_trailing_closure.swift

@@ -34,8 +34,8 @@ func protocolConformance(ac1: @autoclosure () -> CustomStringConvertible,
   f2 = f1 // expected-error{{cannot assign value of type '(fp: FormattedPrintable) -> CustomStringConvertible' to type '(p: CustomStringConvertible) -> FormattedPrintable'}}
 
   accept_creates_Printable(ac1)
-  accept_creates_Printable({ ac2() }) // expected-note {{use trailing closure to simplify arguments}}
-  accept_creates_Printable({ ip1() }) // expected-note {{use trailing closure to simplify arguments}}
+  accept_creates_Printable({ ac2() })
+  accept_creates_Printable({ ip1() })
   accept_creates_FormattedPrintable(ac1) // expected-error{{cannot convert value of type '@autoclosure () -> CustomStringConvertible' to expected argument type '@noescape () -> FormattedPrintable'}}
   accept_creates_FormattedPrintable(ac2)
   accept_creates_FormattedPrintable(ip1) // expected-error{{cannot convert value of type '@autoclosure () -> IsPrintable1' to expected argument type '@noescape () -> FormattedPrintable'}}

test/TypeCoercion/subtyping.swift

@@ -709,40 +709,40 @@ func closure_UU_LU_ne(_ x: Int, _ y: @noescape () -> Int) {} // expected-note 2
 
 func testTrailingClosure() {
   closure_U_L { 0 } // expected-error {{'closure_U_L' has been renamed to 'after(fn:)'}} {{3-14=after}} {{none}}
-  closure_U_L() { 0 } // expected-error {{'closure_U_L' has been renamed to 'after(fn:)'}} {{3-14=after}} {{none}} 
-  closure_U_L({ 0 }) // expected-error {{'closure_U_L' has been renamed to 'after(fn:)'}} {{3-14=after}} {{15-15=fn: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_U_L() { 0 } // expected-error {{'closure_U_L' has been renamed to 'after(fn:)'}} {{3-14=after}} {{none}}
+  closure_U_L({ 0 }) // expected-error {{'closure_U_L' has been renamed to 'after(fn:)'}} {{3-14=after}} {{15-15=fn: }} {{none}}
 
   closure_L_L { 0 } // expected-error {{'closure_L_L(x:)' has been renamed to 'after(fn:)'}} {{3-14=after}} {{none}}
   closure_L_L() { 0 } // expected-error {{'closure_L_L(x:)' has been renamed to 'after(fn:)'}} {{3-14=after}} {{none}}
-  closure_L_L(x: { 0 }) // expected-error {{'closure_L_L(x:)' has been renamed to 'after(fn:)'}} {{3-14=after}} {{15-16=fn}} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_L_L(x: { 0 }) // expected-error {{'closure_L_L(x:)' has been renamed to 'after(fn:)'}} {{3-14=after}} {{15-16=fn}} {{none}}
 
   closure_L_U { 0 } // expected-error {{'closure_L_U(x:)' has been renamed to 'after(_:)'}} {{3-14=after}} {{none}}
   closure_L_U() { 0 } // expected-error {{'closure_L_U(x:)' has been renamed to 'after(_:)'}} {{3-14=after}} {{none}}
-  closure_L_U(x: { 0 }) // expected-error {{'closure_L_U(x:)' has been renamed to 'after(_:)'}} {{3-14=after}} {{15-18=}} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_L_U(x: { 0 }) // expected-error {{'closure_L_U(x:)' has been renamed to 'after(_:)'}} {{3-14=after}} {{15-18=}} {{none}}
 
   closure_UU_LL(0) { 0 } // expected-error {{'closure_UU_LL' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-17=arg: }} {{none}}
-  closure_UU_LL(0, { 0 }) // expected-error {{'closure_UU_LL' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-17=arg: }} {{20-20=fn: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_UU_LL(0, { 0 }) // expected-error {{'closure_UU_LL' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-17=arg: }} {{20-20=fn: }} {{none}}
 
   closure_LU_LL(x: 0) { 0 } // expected-error {{'closure_LU_LL(x:_:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{none}}
-  closure_LU_LL(x: 0, { 0 }) // expected-error {{'closure_LU_LL(x:_:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{23-23=fn: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_LU_LL(x: 0, { 0 }) // expected-error {{'closure_LU_LL(x:_:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{23-23=fn: }} {{none}}
 
   closure_LL_LL(x: 1) { 1 } // expected-error {{'closure_LL_LL(x:y:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{none}}
-  closure_LL_LL(x: 1, y: { 0 }) // expected-error {{'closure_LL_LL(x:y:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{23-24=fn}} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_LL_LL(x: 1, y: { 0 }) // expected-error {{'closure_LL_LL(x:y:)' has been renamed to 'after(arg:fn:)'}} {{3-16=after}} {{17-18=arg}} {{23-24=fn}} {{none}}
 
   closure_UU_LL_ne(1) { 1 } // expected-error {{'closure_UU_LL_ne' has been renamed to 'after(arg:fn:)'}} {{3-19=after}} {{20-20=arg: }} {{none}}
-  closure_UU_LL_ne(1, { 0 }) // expected-error {{'closure_UU_LL_ne' has been renamed to 'after(arg:fn:)'}} {{3-19=after}} {{20-20=arg: }} {{23-23=fn: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_UU_LL_ne(1, { 0 }) // expected-error {{'closure_UU_LL_ne' has been renamed to 'after(arg:fn:)'}} {{3-19=after}} {{20-20=arg: }} {{23-23=fn: }} {{none}}
 
   closure_UU_LU(0) { 0 } // expected-error {{'closure_UU_LU' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-17=arg: }} {{none}}
-  closure_UU_LU(0, { 0 }) // expected-error {{'closure_UU_LU' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-17=arg: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_UU_LU(0, { 0 }) // expected-error {{'closure_UU_LU' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-17=arg: }} {{none}}
 
   closure_LU_LU(x: 0) { 0 } // expected-error {{'closure_LU_LU(x:_:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{none}}
-  closure_LU_LU(x: 0, { 0 }) // expected-error {{'closure_LU_LU(x:_:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_LU_LU(x: 0, { 0 }) // expected-error {{'closure_LU_LU(x:_:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{none}}
 
   closure_LL_LU(x: 1) { 1 } // expected-error {{'closure_LL_LU(x:y:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{none}}
-  closure_LL_LU(x: 1, y: { 0 }) // expected-error {{'closure_LL_LU(x:y:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{23-26=}} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_LL_LU(x: 1, y: { 0 }) // expected-error {{'closure_LL_LU(x:y:)' has been renamed to 'after(arg:_:)'}} {{3-16=after}} {{17-18=arg}} {{23-26=}} {{none}}
 
   closure_UU_LU_ne(1) { 1 } // expected-error {{'closure_UU_LU_ne' has been renamed to 'after(arg:_:)'}} {{3-19=after}} {{20-20=arg: }} {{none}}
-  closure_UU_LU_ne(1, { 0 }) // expected-error {{'closure_UU_LU_ne' has been renamed to 'after(arg:_:)'}} {{3-19=after}} {{20-20=arg: }} {{none}} // expected-note {{use trailing closure to simplify arguments}}
+  closure_UU_LU_ne(1, { 0 }) // expected-error {{'closure_UU_LU_ne' has been renamed to 'after(arg:_:)'}} {{3-19=after}} {{20-20=arg: }} {{none}}
 }
 
 @available(*, unavailable, renamed: "after(x:y:)")

test/attr/attr_availability.swift

@@ -13,7 +13,7 @@ f2(1,2)
 f2(1,2,3)
 
 func f3(_ a: (String) -> Void) { }
-f3({ print($0) }) // expected-note {{use trailing closure to simplify arguments}}
+f3({ print($0) })
 
 
 func f4(_ a: Int..., b: Int) { }

test/decl/func/vararg.swift

@@ -207,7 +207,7 @@ func subscript_rvalue_materialize(_ i: inout Int) {
 
 func subscript_coerce(_ fn: ([UnicodeScalar], [UnicodeScalar]) -> Bool) {}
 func test_subscript_coerce() {
-  subscript_coerce({ $0[$0.count-1] < $1[$1.count-1] }) // expected-note {{use trailing closure to simplify arguments}}
+  subscript_coerce({ $0[$0.count-1] < $1[$1.count-1] })
 }
 
 struct no_index {

test/decl/subscript/subscripting.swift

@@ -831,7 +831,7 @@ struct WillSetDidSetDisambiguate2Attr {
 // No need to disambiguate -- this is clearly a function call.
 func willSet(_: () -> Int) {}
 struct WillSetDidSetDisambiguate3 {
-  var x: Int = takeTrailingClosure({ // expected-note {{use trailing closure to simplify arguments}}
+  var x: Int = takeTrailingClosure({
     willSet { 42 }
   })
 }