Merge pull request #27154 from nkcsgexi/diagnose-adding-desig-init-open-class

ABI/API checker: diagnose adding new designated initializers to open classes

Author: nkcsgexi

include/swift/AST/DiagnosticsModuleDiffer.def

@@ -98,6 +98,8 @@ ERROR(new_decl_without_intro,none,"%0 is a new API without @available attribute"
 
 ERROR(objc_name_change,none,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))
 
+ERROR(desig_init_added,none,"%0 has been added as a designated initializer to an open class", (StringRef))
+
 #ifndef DIAG_NO_UNDEF
 # if defined(DIAG)
 #  undef DIAG

include/swift/IDE/DigesterEnums.def

@@ -152,6 +152,7 @@ KEY_STRING(IntrotvOS, intro_tvOS)
 KEY_STRING(IntrowatchOS, intro_watchOS)
 KEY_STRING(Introswift, intro_swift)
 KEY_STRING(ObjCName, objc_name)
+KEY_STRING(InitKind, init_kind)
 
 KEY_STRING_ARR(SuperclassNames, superclassNames)
 KEY_STRING_ARR(ToolArgs, tool_arguments)

test/api-digester/Inputs/cake_baseline/cake.swift

@@ -201,3 +201,12 @@ public class SwiftObjcClass {
   @objc(OldObjCFool:OldObjCA:OldObjCB:)
   public func foo(a:Int, b:Int, c: Int) {}
 }
+
+@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
+open class AddingNewDesignatedInit {
+  public init() {}
+  public convenience init(foo: Int) {
+    self.init()
+    print(foo)
+  }
+}

test/api-digester/Inputs/cake_current/cake.swift

@@ -208,3 +208,13 @@ public class SwiftObjcClass {
   @objc(NewObjCFool:NewObjCA:NewObjCB:)
   public func foo(a:Int, b:Int, c: Int) {}
 }
+
+@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
+open class AddingNewDesignatedInit {
+  public init(_ b: Bool) {}
+  public init() {}
+  public convenience init(foo: Int) {
+    self.init()
+    print(foo)
+  }
+}

test/api-digester/Outputs/Cake-abi.txt

@@ -115,3 +115,4 @@ cake: Var RequiementChanges.addedVar has been added as a protocol requirement
 cake: Class C4 has changed its super class from APINotesTest.OldType to APINotesTest.NewType
 cake: Class SubGenericClass has changed its super class from cake.GenericClass<cake.P1> to cake.GenericClass<cake.P2>
 cake: Class SuperClassRemoval has removed its super class cake.C3
+cake: Constructor AddingNewDesignatedInit.init(_:) has been added as a designated initializer to an open class

test/api-digester/Outputs/Cake.txt

@@ -64,6 +64,7 @@ cake: Accessor ClassWithOpenMember.property.Get() is no longer open for subclass
 cake: Class C4 has changed its super class from APINotesTest.OldType to APINotesTest.NewType
 cake: Class SubGenericClass has changed its super class from cake.GenericClass<cake.P1> to cake.GenericClass<cake.P2>
 cake: Class SuperClassRemoval has removed its super class cake.C3
+cake: Constructor AddingNewDesignatedInit.init(_:) has been added as a designated initializer to an open class
 cake: Func ClassWithOpenMember.bar() is no longer open for subclassing
 cake: Func ClassWithOpenMember.foo() is no longer open for subclassing
 cake: Var ClassWithOpenMember.property is no longer open for subclassing

test/api-digester/Outputs/cake-abi.json

@@ -564,7 +564,8 @@
           "implicit": true,
           "declAttributes": [
             "Inlinable"
-          ]
+          ],
+          "init_kind": "Designated"
         },
         {
           "kind": "Var",
@@ -1823,5 +1824,5 @@
       ]
     }
   ],
-  "json_format_version": 5
+  "json_format_version": 6
 }

test/api-digester/Outputs/cake.json

@@ -584,7 +584,8 @@
           "implicit": true,
           "declAttributes": [
             "Inlinable"
-          ]
+          ],
+          "init_kind": "Designated"
         },
         {
           "kind": "Var",
@@ -1674,5 +1675,5 @@
       ]
     }
   ],
-  "json_format_version": 5
+  "json_format_version": 6
 }

test/api-digester/Outputs/clang-module-dump.txt

@@ -105,7 +105,8 @@
             "Override",
             "ObjC",
             "Dynamic"
-          ]
+          ],
+          "init_kind": "Designated"
         }
       ],
       "declKind": "Class",
@@ -183,5 +184,5 @@
       ]
     }
   ],
-  "json_format_version": 5
+  "json_format_version": 6
 }

test/api-digester/Outputs/empty-baseline.json

@@ -2,5 +2,5 @@
   "kind": "Root",
   "name": "TopLevel",
   "printedName": "TopLevel",
-  "json_format_version": 5
+  "json_format_version": 6
 }

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

@@ -132,7 +132,7 @@ SDKNodeDeclFunction::SDKNodeDeclFunction(SDKNodeInitInfo Info):
   FuncSelfKind(Info.FuncSelfKind) {}
 
 SDKNodeDeclConstructor::SDKNodeDeclConstructor(SDKNodeInitInfo Info):
-  SDKNodeDeclAbstractFunc(Info, SDKNodeKind::DeclConstructor) {}
+  SDKNodeDeclAbstractFunc(Info, SDKNodeKind::DeclConstructor), InitKind(Info.InitKind) {}
 
 SDKNodeDeclAccessor::SDKNodeDeclAccessor(SDKNodeInitInfo Info):
   SDKNodeDeclAbstractFunc(Info, SDKNodeKind::DeclAccessor),
@@ -1271,6 +1271,30 @@ static std::vector<DeclAttrKind> collectDeclAttributes(Decl *D) {
   return Results;
 }
 
+CtorInitializerKind SDKNodeDeclConstructor::getInitKind() const {
+#define CASE(KIND) if (InitKind == #KIND) return CtorInitializerKind::KIND;
+  CASE(Designated)
+  CASE(Convenience)
+  CASE(ConvenienceFactory)
+  CASE(Factory)
+#undef CASE
+  llvm_unreachable("unhandled init kind");
+}
+
+StringRef SDKContext::getInitKind(Decl *D) {
+  if (auto *CD = dyn_cast<ConstructorDecl>(D)) {
+    switch(CD->getInitKind()) {
+#define CASE(KIND) case CtorInitializerKind::KIND: return #KIND;
+    CASE(Designated)
+    CASE(Convenience)
+    CASE(ConvenienceFactory)
+    CASE(Factory)
+#undef CASE
+    }
+  }
+  return StringRef();
+}
+
 SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
       Ctx(Ctx), DKind(D->getKind()),
       Location(calculateLocation(Ctx, D)),
@@ -1285,6 +1309,7 @@ SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
       IntrowatchOS(Ctx.getPlatformIntroVersion(D, PlatformKind::watchOS)),
       Introswift(Ctx.getLanguageIntroVersion(D)),
       ObjCName(Ctx.getObjcName(D)),
+      InitKind(Ctx.getInitKind(D)),
       IsImplicit(D->isImplicit()),
       IsDeprecated(D->getAttrs().getDeprecated(D->getASTContext())),
       IsABIPlaceholder(isABIPlaceholderRecursive(D)),
@@ -1916,6 +1941,11 @@ void SDKNodeDeclFunction::jsonize(json::Output &out) {
   output(out, KeyKind::KK_funcSelfKind, FuncSelfKind);
 }
 
+void SDKNodeDeclConstructor::jsonize(json::Output &out) {
+  SDKNodeDeclAbstractFunc::jsonize(out);
+  output(out, KeyKind::KK_init_kind, InitKind);
+}
+
 void SDKNodeDeclType::jsonize(json::Output &out) {
   SDKNodeDecl::jsonize(out);
   output(out, KeyKind::KK_superclassUsr, SuperclassUsr);

tools/swift-api-digester/ModuleAnalyzerNodes.h

@@ -62,7 +62,7 @@ namespace api {
 ///
 /// When the json format changes in a way that requires version-specific handling, this number should be incremented.
 /// This ensures we could have backward compatibility so that version changes in the format won't stop the checker from working.
-const uint8_t DIGESTER_JSON_VERSION = 5; // Populate ObjC, Dynamic and Final to attribute list
+const uint8_t DIGESTER_JSON_VERSION = 6; // Add initkind for constructors
 const uint8_t DIGESTER_JSON_DEFAULT_VERSION = 0; // Use this version number for files before we have a version number in json.
 
 class SDKNode;
@@ -218,6 +218,7 @@ class SDKContext {
   StringRef getPlatformIntroVersion(Decl *D, PlatformKind Kind);
   StringRef getLanguageIntroVersion(Decl *D);
   StringRef getObjcName(Decl *D);
+  StringRef getInitKind(Decl *D);
   bool isEqual(const SDKNode &Left, const SDKNode &Right);
   bool checkingABI() const { return Opts.ABI; }
   AccessLevel getAccessLevel(const ValueDecl *VD) const;
@@ -677,9 +678,12 @@ class SDKNodeDeclFunction: public SDKNodeDeclAbstractFunc {
 };
 
 class SDKNodeDeclConstructor: public SDKNodeDeclAbstractFunc {
+  StringRef InitKind;
 public:
   SDKNodeDeclConstructor(SDKNodeInitInfo Info);
   static bool classof(const SDKNode *N);
+  CtorInitializerKind getInitKind() const;
+  void jsonize(json::Output &Out) override;
 };
 
 class SDKNodeDeclAccessor: public SDKNodeDeclAbstractFunc {

tools/swift-api-digester/ModuleDiagsConsumer.cpp

@@ -73,6 +73,7 @@ static StringRef getCategoryName(uint32_t ID) {
   case LocalDiagID::super_class_removed:
   case LocalDiagID::super_class_changed:
   case LocalDiagID::no_longer_open:
+  case LocalDiagID::desig_init_added:
     return "/* Class Inheritance Change */";
   default:
     return StringRef();

tools/swift-api-digester/swift-api-digester.cpp

@@ -1144,6 +1144,18 @@ class PrunePass : public MatchedNodeListener, public SDKTreeDiffPass {
           }
         }
       }
+      if (auto *CD = dyn_cast<SDKNodeDeclConstructor>(Right)) {
+        if (auto *TD = dyn_cast<SDKNodeDeclType>(Right->getParent())) {
+          if (TD->isOpen() && CD->getInitKind() == CtorInitializerKind::Designated) {
+            // If client's subclass provides an implementation of all of its superclass designated
+            // initializers, it automatically inherits all of the superclass convenience initializers.
+            // This means if a new designated init is added to the base class, the inherited
+            // convenience init may be missing and cause breakage.
+            CD->emitDiag(diag::desig_init_added);
+          }
+        }
+      }
+
       return;
     case NodeMatchReason::Removed:
       assert(!Right);