Add empty parens to var-to-function renames, take two. (swiftlang#4938)

jrose-apple · jrose-apple · commit aaac511403fd · 2016-09-23T14:59:00.000-07:00
This time, propagate the decl marked deprecated or unavailable through to the fix-it, so we can be sure it's a var. https://bugs.swift.org/browse/SR-1649 (cherry picked from commit d89c232)
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -1333,6 +1333,7 @@ bool swift::fixItOverrideDeclarationTypes(TypeChecker &TC,
 void swift::fixItAvailableAttrRename(TypeChecker &TC,
                                      InFlightDiagnostic &diag,
                                      SourceRange referenceRange,
+                                     const ValueDecl *renamedDecl,
                                      const AvailableAttr *attr,
                                      const ApplyExpr *call) {
   ParsedDeclName parsed = swift::parseDeclName(attr->Rename);
@@ -1487,6 +1488,12 @@ void swift::fixItAvailableAttrRename(TypeChecker &TC,
       baseReplace += '.';
     }
     baseReplace += parsed.BaseName;
+    if (parsed.IsFunctionName && parsed.ArgumentLabels.empty() &&
+        isa<VarDecl>(renamedDecl)) {
+      // If we're going from a var to a function with no arguments, emit an
+      // empty parameter list.
+      baseReplace += "()";
+    }
     diag.fixItReplace(referenceRange, baseReplace);
   }
 
@@ -1689,11 +1696,14 @@ describeRename(ASTContext &ctx, const AvailableAttr *attr, const ValueDecl *D,
   return ReplacementDeclKind::None;
 }
 
-void TypeChecker::diagnoseDeprecated(SourceRange ReferenceRange,
-                                     const DeclContext *ReferenceDC,
-                                     const AvailableAttr *Attr,
-                                     DeclName Name,
-                                     const ApplyExpr *Call) {
+void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
+                                       const DeclContext *ReferenceDC,
+                                       const ValueDecl *DeprecatedDecl,
+                                       const ApplyExpr *Call) {
+  const AvailableAttr *Attr = TypeChecker::getDeprecated(DeprecatedDecl);
+  if (!Attr)
+    return;
+
   // We match the behavior of clang to not report deprecation warnings
   // inside declarations that are themselves deprecated on all deployment
   // targets.
@@ -1712,6 +1722,7 @@ void TypeChecker::diagnoseDeprecated(SourceRange ReferenceRange,
     }
   }
 
+  DeclName Name = DeprecatedDecl->getFullName();
   StringRef Platform = Attr->prettyPlatformString();
   clang::VersionTuple DeprecatedVersion;
   if (Attr->Deprecated)
@@ -1750,7 +1761,8 @@ void TypeChecker::diagnoseDeprecated(SourceRange ReferenceRange,
     auto renameDiag = diagnose(ReferenceRange.Start,
                                diag::note_deprecated_rename,
                                newName);
-    fixItAvailableAttrRename(*this, renameDiag, ReferenceRange, Attr, Call);
+    fixItAvailableAttrRename(*this, renameDiag, ReferenceRange, DeprecatedDecl,
+                             Attr, Call);
   }
 }
 
@@ -1806,7 +1818,7 @@ bool TypeChecker::diagnoseExplicitUnavailability(const ValueDecl *D,
                                                  const ApplyExpr *call) {
   return diagnoseExplicitUnavailability(D, R, DC,
                                         [=](InFlightDiagnostic &diag) {
-    fixItAvailableAttrRename(*this, diag, R, AvailableAttr::isUnavailable(D),
+    fixItAvailableAttrRename(*this, diag, R, D, AvailableAttr::isUnavailable(D),
                              call);
   });
 }
@@ -2134,9 +2146,7 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
     return true;
 
   // Diagnose for deprecation
-  if (const AvailableAttr *Attr = TypeChecker::getDeprecated(D)) {
-    TC.diagnoseDeprecated(R, DC, Attr, D->getFullName(), call);
-  }
+  TC.diagnoseIfDeprecated(R, DC, D, call);
 
   // Diagnose for potential unavailability
   auto maybeUnavail = TC.checkDeclarationAvailability(D, R.Start, DC);
diff --git a/lib/Sema/MiscDiagnostics.h b/lib/Sema/MiscDiagnostics.h
@@ -73,6 +73,7 @@ bool diagnoseArgumentLabelError(TypeChecker &TC, const Expr *expr,
 void fixItAvailableAttrRename(TypeChecker &TC,
                               InFlightDiagnostic &diag,
                               SourceRange referenceRange,
+                              const ValueDecl *renamedDecl,
                               const AvailableAttr *attr,
                               const ApplyExpr *call);
 
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -1259,10 +1259,11 @@ static Type resolveIdentTypeComponent(
 }
 
 // FIXME: Merge this with diagAvailability in MiscDiagnostics.cpp.
-static bool checkTypeDeclAvailability(Decl *TypeDecl, IdentTypeRepr *IdType,
+static bool checkTypeDeclAvailability(const TypeDecl *TypeDecl,
+                                      IdentTypeRepr *IdType,
                                       SourceLoc Loc, DeclContext *DC,
                                       TypeChecker &TC,
-                                      bool AllowPotentiallyUnavailableProtocol) {
+                                      bool AllowPotentiallyUnavailableProtocol){
 
   if (auto CI = dyn_cast<ComponentIdentTypeRepr>(IdType)) {
     if (auto Attr = AvailableAttr::isUnavailable(TypeDecl)) {
@@ -1282,7 +1283,8 @@ static bool checkTypeDeclAvailability(Decl *TypeDecl, IdentTypeRepr *IdType,
                                   diag::availability_decl_unavailable_rename,
                                   CI->getIdentifier(), /*"replaced"*/false,
                                   /*special kind*/0, Attr->Rename);
-          fixItAvailableAttrRename(TC, diag, Loc, Attr, /*call*/nullptr);
+          fixItAvailableAttrRename(TC, diag, Loc, TypeDecl, Attr,
+                                   /*call*/nullptr);
         } else if (Attr->Message.empty()) {
           TC.diagnose(Loc,
                       inSwift ? diag::availability_decl_unavailable_in_swift
@@ -1308,11 +1310,9 @@ static bool checkTypeDeclAvailability(Decl *TypeDecl, IdentTypeRepr *IdType,
       return true;
     }
 
-    if (auto *Attr = TypeChecker::getDeprecated(TypeDecl)) {
-      TC.diagnoseDeprecated(CI->getSourceRange(), DC, Attr,
-                            CI->getIdentifier(), /*call, N/A*/nullptr);
-    }
-
+    TC.diagnoseIfDeprecated(CI->getSourceRange(), DC, TypeDecl,
+                            /*call, N/A*/nullptr);
+    
     if (AllowPotentiallyUnavailableProtocol && isa<ProtocolDecl>(TypeDecl))
       return false;
 
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -1921,11 +1921,10 @@ class TypeChecker final : public LazyResolver {
   /// Emits a diagnostic for a reference to a declaration that is deprecated.
   /// Callers can provide a lambda that adds additional information (such as a
   /// fixit hint) to the deprecation diagnostic, if it is emitted.
-  void diagnoseDeprecated(SourceRange SourceRange,
-                          const DeclContext *ReferenceDC,
-                          const AvailableAttr *Attr,
-                          DeclName Name,
-                          const ApplyExpr *Call);
+  void diagnoseIfDeprecated(SourceRange SourceRange,
+                            const DeclContext *ReferenceDC,
+                            const ValueDecl *DeprecatedDecl,
+                            const ApplyExpr *Call);
   /// @}
 
   /// If LangOptions::DebugForbidTypecheckPrefix is set and the given decl
diff --git a/test/1_stdlib/Renames.swift b/test/1_stdlib/Renames.swift
@@ -519,8 +519,8 @@ func _String<S, C>(x: String, s: S, c: C, i: String.Index)
   x.replaceRange(i..<i, with: x) // expected-error {{'replaceRange(_:with:)' has been renamed to 'replaceSubrange'}} {{5-17=replaceSubrange}} {{none}}
   _ = x.removeAtIndex(i) // expected-error {{'removeAtIndex' has been renamed to 'remove(at:)'}} {{9-22=remove}} {{23-23=at: }} {{none}}
   x.removeRange(i..<i) // expected-error {{'removeRange' has been renamed to 'removeSubrange'}} {{5-16=removeSubrange}} {{none}}
-  _ = x.lowercaseString // expected-error {{'lowercaseString' has been renamed to 'lowercased()'}} {{9-24=lowercased}} {{none}}
-  _ = x.uppercaseString // expected-error {{'uppercaseString' has been renamed to 'uppercased()'}} {{9-24=uppercased}} {{none}}
+  _ = x.lowercaseString // expected-error {{'lowercaseString' has been renamed to 'lowercased()'}} {{9-24=lowercased()}} {{none}}
+  _ = x.uppercaseString // expected-error {{'uppercaseString' has been renamed to 'uppercased()'}} {{9-24=uppercased()}} {{none}}
   // FIXME: SR-1649 <rdar://problem/26563343>; We should suggest to add '()'
 }
 func _String<S : Sequence>(s: S, sep: String) where S.Iterator.Element == String {
diff --git a/test/Sema/availability.swift b/test/Sema/availability.swift
@@ -130,3 +130,26 @@ func testPlatforms() {
   let _: UnavailableOnOSXAppExt = 0
   let _: UnavailableOnMacOSAppExt = 0
 }
+
+struct VarToFunc {
+  @available(*, unavailable, renamed: "function()")
+  var variable: Int // expected-note 2 {{explicitly marked unavailable here}}
+
+  @available(*, unavailable, renamed: "function()")
+  func oldFunction() -> Int { return 42 } // expected-note 2 {{explicitly marked unavailable here}}
+
+  func function() -> Int {
+    _ = variable // expected-error{{'variable' has been renamed to 'function()'}}{{9-17=function()}}
+    _ = oldFunction() //expected-error{{'oldFunction()' has been renamed to 'function()'}}{{9-20=function}}
+    _ = oldFunction // expected-error{{'oldFunction()' has been renamed to 'function()'}} {{9-20=function}}
+
+    return 42
+  }
+
+  mutating func testAssignment() {
+    // This is nonsense, but someone shouldn't be using 'renamed' for this
+    // anyway. Just make sure we don't crash or anything.
+    variable = 2 // expected-error {{'variable' has been renamed to 'function()'}} {{5-13=function()}}
+  }
+}
+
