Merge pull request #58545 from cbjeukendrup/fixit_deprec_self_init

Fix #57354: Fix-it for deprecated initializers removes the `.init` part from `self.init`

Author: tshortli

lib/AST/Availability.cpp

@@ -14,14 +14,14 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "swift/AST/Availability.h"
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/Attr.h"
 #include "swift/AST/Decl.h"
-#include "swift/AST/Types.h"
-#include "swift/AST/Availability.h"
 #include "swift/AST/PlatformKind.h"
 #include "swift/AST/TypeCheckRequests.h"
 #include "swift/AST/TypeWalker.h"
+#include "swift/AST/Types.h"
 #include <map>
 
 using namespace swift;
@@ -96,10 +96,12 @@ static void mergeWithInferredAvailability(const AvailableAttr *Attr,
 
 /// Create an implicit availability attribute for the given platform
 /// and with the inferred availability.
-static AvailableAttr *
-createAvailableAttr(PlatformKind Platform,
-                       const InferredAvailability &Inferred,
-                       ASTContext &Context) {
+static AvailableAttr *createAvailableAttr(PlatformKind Platform,
+                                          const InferredAvailability &Inferred,
+                                          StringRef Message, 
+                                          StringRef Rename,
+                                          ValueDecl *RenameDecl,
+                                          ASTContext &Context) {
 
   llvm::VersionTuple Introduced =
       Inferred.Introduced.value_or(llvm::VersionTuple());
@@ -108,21 +110,27 @@ createAvailableAttr(PlatformKind Platform,
   llvm::VersionTuple Obsoleted =
       Inferred.Obsoleted.value_or(llvm::VersionTuple());
 
-  return new (Context) AvailableAttr(
-      SourceLoc(), SourceRange(), Platform,
-      /*Message=*/StringRef(),
-      /*Rename=*/StringRef(), /*RenameDecl=*/nullptr,
-        Introduced, /*IntroducedRange=*/SourceRange(),
-        Deprecated, /*DeprecatedRange=*/SourceRange(),
-        Obsoleted, /*ObsoletedRange=*/SourceRange(),
-      Inferred.PlatformAgnostic, /*Implicit=*/true,
-      Inferred.IsSPI);
+  return new (Context)
+      AvailableAttr(SourceLoc(), SourceRange(), Platform,
+                    Message, Rename, RenameDecl,
+                    Introduced, /*IntroducedRange=*/SourceRange(),
+                    Deprecated, /*DeprecatedRange=*/SourceRange(),
+                    Obsoleted, /*ObsoletedRange=*/SourceRange(),
+                    Inferred.PlatformAgnostic, /*Implicit=*/true,
+                    Inferred.IsSPI);
 }
 
 void AvailabilityInference::applyInferredAvailableAttrs(
     Decl *ToDecl, ArrayRef<const Decl *> InferredFromDecls,
     ASTContext &Context) {
 
+  // Let the new AvailabilityAttr inherit the message and rename.
+  // The first encountered message / rename will win; this matches the 
+  // behaviour of diagnostics for 'non-inherited' AvailabilityAttrs.
+  StringRef Message;
+  StringRef Rename;
+  ValueDecl *RenameDecl = nullptr;
+
   // Iterate over the declarations and infer required availability on
   // a per-platform basis.
   std::map<PlatformKind, InferredAvailability> Inferred;
@@ -133,14 +141,24 @@ void AvailabilityInference::applyInferredAvailableAttrs(
         continue;
 
       mergeWithInferredAvailability(AvAttr, Inferred[AvAttr->Platform]);
+
+      if (Message.empty() && !AvAttr->Message.empty())
+        Message = AvAttr->Message;
+
+      if (Rename.empty() && !AvAttr->Rename.empty()) {
+        Rename = AvAttr->Rename;
+        RenameDecl = AvAttr->RenameDecl;
+      }
     }
   }
 
   // Create an availability attribute for each observed platform and add
   // to ToDecl.
   DeclAttributes &Attrs = ToDecl->getAttrs();
   for (auto &Pair : Inferred) {
-    auto *Attr = createAvailableAttr(Pair.first, Pair.second, Context);
+    auto *Attr = createAvailableAttr(Pair.first, Pair.second, Message,
+                                     Rename, RenameDecl, Context);
+
     Attrs.add(Attr);
   }
 }

lib/Sema/TypeCheckAvailability.cpp

@@ -2355,16 +2355,22 @@ static void fixItAvailableAttrRename(InFlightDiagnostic &diag,
   } else if (parsed.BaseName == "init" && isa_and_nonnull<CallExpr>(call)) {
     auto *CE = cast<CallExpr>(call);
 
-    // For initializers, replace with a "call" of the context type...but only
-    // if we know we're doing a call (rather than a first-class reference).
+    // If it is a call to an initializer (rather than a first-class reference):
+
     if (parsed.isMember()) {
+      // replace with a "call" to the type (instead of writing `.init`)
       diag.fixItReplace(CE->getFn()->getSourceRange(), parsed.ContextName);
     } else if (auto *dotCall = dyn_cast<DotSyntaxCallExpr>(CE->getFn())) {
-      SourceLoc removeLoc = dotCall->getDotLoc();
-      if (removeLoc.isInvalid())
-        return;
-
-      diag.fixItRemove(SourceRange(removeLoc, dotCall->getFn()->getEndLoc()));
+      // if it's a dot call, and the left side is a type (and not `self` or 
+      // `super`, for example), just remove the dot and the right side, again 
+      // in order to make it a "call" to the type
+      if (isa<TypeExpr>(dotCall->getBase())) {
+        SourceLoc removeLoc = dotCall->getDotLoc();
+        if (removeLoc.isInvalid())
+          return;
+
+        diag.fixItRemove(SourceRange(removeLoc, dotCall->getFn()->getEndLoc()));
+      }
     } else if (!isa<ConstructorRefCallExpr>(CE->getFn())) {
       return;
     }

test/attr/attr_availability.swift

@@ -615,6 +615,62 @@ func testFactoryMethods() {
   Int.factory2(other: 1) // expected-error {{'factory2(other:)' has been replaced by 'Int.init(other:)'}} {{3-15=Int}}
 }
 
+class DeprecatedInitBase {
+  @available(*, deprecated, renamed: "init(new:)")
+  init(old: Int) {}
+
+  init(new: Int) {}
+
+  convenience init(testSelf: Int) {
+    // https://github.com/apple/swift/issues/57354
+    // The fix-it should not remove `.init`
+    self.init(old: testSelf) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{15-18=new}}
+  }
+
+  init(testSuper: Int) {}
+
+  @available(*, deprecated, renamed: "init(new:)")
+  @available(*, deprecated, renamed: "init(new:)")
+  init(multipleEqualAvailabilityAttributes: Int) {}
+
+  @available(*, deprecated, renamed: "init(old:)")
+  @available(*, deprecated, renamed: "init(testSuper:)")
+  @available(*, deprecated, renamed: "init(new:)")
+  init(multipleUnequalAvailabilityAttributes: Int) {}
+}
+
+class DeprecatedInitSub1: DeprecatedInitBase {
+  override init(testSuper: Int) {
+    // https://github.com/apple/swift/issues/57354
+    // The fix-it should not remove `.init`
+    super.init(old: testSuper) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{16-19=new}}
+  }
+}
+
+class DeprecatedInitSub2: DeprecatedInitBase { }
+
+_ = DeprecatedInitBase(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{24-27=new}}
+_ = DeprecatedInitBase.init(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{29-32=new}}
+let _: DeprecatedInitBase = .init(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{35-38=new}}
+_ = DeprecatedInitSub2(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{24-27=new}}
+_ = DeprecatedInitSub2.init(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{29-32=new}}
+let _: DeprecatedInitSub2 = .init(old: 0) // expected-warning {{'init(old:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{35-38=new}}
+
+_ = DeprecatedInitBase(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{24-59=new}}
+_ = DeprecatedInitBase.init(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{29-64=new}}
+let _: DeprecatedInitBase = .init(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{35-70=new}}
+_ = DeprecatedInitSub2(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{24-59=new}}
+_ = DeprecatedInitSub2.init(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{29-64=new}}
+let _: DeprecatedInitSub2 = .init(multipleEqualAvailabilityAttributes: 0) // expected-warning {{'init(multipleEqualAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{35-70=new}}
+
+_ = DeprecatedInitBase(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{24-61=new}}
+_ = DeprecatedInitBase.init(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{29-66=new}}
+let _: DeprecatedInitBase = .init(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated: replaced by 'init(new:)'}} expected-note {{use 'init(new:)' instead}} {{35-72=new}}
+_ = DeprecatedInitSub2(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated}} expected-note {{use 'init(new:)' instead}} {{24-61=new}}
+_ = DeprecatedInitSub2.init(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated}} expected-note {{use 'init(new:)' instead}} {{29-66=new}}
+let _: DeprecatedInitSub2 = .init(multipleUnequalAvailabilityAttributes: 0) // expected-warning {{'init(multipleUnequalAvailabilityAttributes:)' is deprecated}} expected-note {{use 'init(new:)' instead}} {{35-72=new}}
+
+
 class Base {
   @available(*, unavailable)
   func bad() {} // expected-note {{here}}
@@ -1073,14 +1129,6 @@ struct UnavailableAccessors {
   }
 }
 
-class BaseDeprecatedInit {
-  @available(*, deprecated) init(bad: Int) { }
-}
-
-class SubInheritedDeprecatedInit: BaseDeprecatedInit { }
-
-_ = SubInheritedDeprecatedInit(bad: 0) // expected-warning {{'init(bad:)' is deprecated}}
-
 // https://github.com/apple/swift/issues/51149
 // Should produce no warnings.