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

nkcsgexi · web-flow · commit 5664045d01ce · 2019-09-12T17:04:43.000-07:00
ABI/API checker: diagnose adding new designated initializers to open classes
diff --git a/include/swift/AST/DiagnosticsModuleDiffer.def b/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
diff --git a/include/swift/IDE/DigesterEnums.def b/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)
diff --git a/test/api-digester/Inputs/cake_baseline/cake.swift b/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)
+  }
+}
diff --git a/test/api-digester/Inputs/cake_current/cake.swift b/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)
+  }
+}
diff --git a/test/api-digester/Outputs/Cake-abi.txt b/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
diff --git a/test/api-digester/Outputs/Cake.txt b/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
diff --git a/test/api-digester/Outputs/cake-abi.json b/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
 }
diff --git a/test/api-digester/Outputs/cake.json b/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
 }
diff --git a/test/api-digester/Outputs/clang-module-dump.txt b/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
 }
diff --git a/test/api-digester/Outputs/empty-baseline.json b/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
 }
diff --git a/tools/swift-api-digester/ModuleAnalyzerNodes.cpp b/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);
diff --git a/tools/swift-api-digester/ModuleAnalyzerNodes.h b/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 {
diff --git a/tools/swift-api-digester/ModuleDiagsConsumer.cpp b/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();
diff --git a/tools/swift-api-digester/swift-api-digester.cpp b/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);

Original file line number	Diff line number	Diff line change
`@@ -564,7 +564,8 @@`
`564`	`564`	`"implicit": true,`
`565`	`565`	`"declAttributes": [`
`566`	`566`	`"Inlinable"`
`567`		`- ]`
	`567`	`+ ],`
	`568`	`+ "init_kind": "Designated"`
`568`	`569`	`},`
`569`	`570`	`{`
`570`	`571`	`"kind": "Var",`
`@@ -1823,5 +1824,5 @@`
`1823`	`1824`	`]`
`1824`	`1825`	`}`
`1825`	`1826`	`],`
`1826`		`- "json_format_version": 5`
	`1827`	`+ "json_format_version": 6`
`1827`	`1828`	`}`
Original file line number	Diff line number	Diff line change
`@@ -584,7 +584,8 @@`
`584`	`584`	`"implicit": true,`
`585`	`585`	`"declAttributes": [`
`586`	`586`	`"Inlinable"`
`587`		`- ]`
	`587`	`+ ],`
	`588`	`+ "init_kind": "Designated"`
`588`	`589`	`},`
`589`	`590`	`{`
`590`	`591`	`"kind": "Var",`
`@@ -1674,5 +1675,5 @@`
`1674`	`1675`	`]`
`1675`	`1676`	`}`
`1676`	`1677`	`],`
`1677`		`- "json_format_version": 5`
	`1678`	`+ "json_format_version": 6`
`1678`	`1679`	`}`
Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,8 @@`
`105`	`105`	`"Override",`
`106`	`106`	`"ObjC",`
`107`	`107`	`"Dynamic"`
`108`		`- ]`
	`108`	`+ ],`
	`109`	`+ "init_kind": "Designated"`
`109`	`110`	`}`
`110`	`111`	`],`
`111`	`112`	`"declKind": "Class",`
`@@ -183,5 +184,5 @@`
`183`	`184`	`]`
`184`	`185`	`}`
`185`	`186`	`],`
`186`		`- "json_format_version": 5`
	`187`	`+ "json_format_version": 6`
`187`	`188`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@`
`2`	`2`	`"kind": "Root",`
`3`	`3`	`"name": "TopLevel",`
`4`	`4`	`"printedName": "TopLevel",`
`5`		`- "json_format_version": 5`
	`5`	`+ "json_format_version": 6`
`6`	`6`	`}`