Merge pull request #11045 from itaiferber/codable-class-fixes

Codable Class Fixes

Author: itaiferber

include/swift/AST/DiagnosticsSema.def

@@ -2072,6 +2072,20 @@ NOTE(codable_codingkeys_type_is_not_an_enum_here,none,
      "cannot automatically synthesize %0 because 'CodingKeys' is not an enum", (Type))
 NOTE(codable_codingkeys_type_does_not_conform_here,none,
      "cannot automatically synthesize %0 because 'CodingKeys' does not conform to CodingKey", (Type))
+NOTE(decodable_no_super_init_here,none,
+     "cannot automatically synthesize %0 because superclass does not have a callable %1", (DeclName, DeclName))
+NOTE(decodable_super_init_not_designated_here,none,
+     "cannot automatically synthesize %0 because implementation would need to call %1, which is not designated", (DeclName, DeclName))
+NOTE(decodable_inaccessible_super_init_here,none,
+     "cannot automatically synthesize %0 because implementation would need to call %1, which is inaccessible due to "
+     "'%select{private|fileprivate|internal|%error|%error}2' protection level",
+     (DeclName, DeclName, Accessibility))
+NOTE(decodable_super_init_is_failable_here,none,
+     "cannot automatically synthesize %0 because implementation would need to call %1, which is failable", (DeclName, DeclName))
+NOTE(decodable_suggest_overriding_init_here,none,
+     "did you mean to override 'init(from:)'?", ())
+NOTE(codable_suggest_overriding_init_here,none,
+     "did you mean to override 'init(from:)' and 'encode(to:)'?", ())
 
 // Dynamic Self
 ERROR(dynamic_self_non_method,none,

lib/Sema/DerivedConformanceCodable.cpp

@@ -407,8 +407,9 @@ static bool canSynthesizeCodingKey(TypeChecker &tc, Decl *parentDecl,
       return false;
   }
 
-  if (!enumDecl->getInherited().empty() &&
-      enumDecl->getInherited().front().isError())
+  auto inherited = enumDecl->getInherited();
+  if (!inherited.empty() && inherited.front().wasValidated() &&
+      inherited.front().isError())
     return false;
 
   // If it meets all of those requirements, we can synthesize CodingKey

lib/Sema/DerivedConformanceCodingKey.cpp

@@ -343,8 +343,9 @@ static bool canSynthesizeRawRepresentable(TypeChecker &tc, Decl *parentDecl,
   auto parentDC = cast<DeclContext>(parentDecl);
   rawType       = parentDC->mapTypeIntoContext(rawType);
 
-  if (!enumDecl->getInherited().empty() &&
-      enumDecl->getInherited().front().isError())
+  auto inherited = enumDecl->getInherited();
+  if (!inherited.empty() && inherited.front().wasValidated() &&
+      inherited.front().isError())
     return false;
 
   // The raw type must be Equatable, so that we have a suitable ~= for

lib/Sema/DerivedConformanceRawRepresentable.cpp

@@ -7944,6 +7944,67 @@ static void diagnoseClassWithoutInitializers(TypeChecker &tc,
   tc.diagnose(classDecl, diag::class_without_init,
               classDecl->getDeclaredType());
 
+  // HACK: We've got a special case to look out for and diagnose specifically to
+  // improve the experience of seeing this, and mitigate some confusion.
+  //
+  // For a class A which inherits from Decodable class B, class A may have
+  // additional members which prevent default initializer synthesis (and
+  // inheritance of other initializers). The user may have assumed that this
+  // case would synthesize Encodable/Decodable conformance for class A the same
+  // way it may have for class B, or other classes.
+  //
+  // It is helpful to suggest here that the user may have forgotten to override
+  // init(from:) (and encode(to:), if applicable) in a note, before we start
+  // listing the members that prevented initializer synthesis.
+  // TODO: Add a fixit along with this suggestion.
+  if (auto *superclassDecl = classDecl->getSuperclassDecl()) {
+    ASTContext &C = tc.Context;
+    auto *decodableProto = C.getProtocol(KnownProtocolKind::Decodable);
+    auto superclassType = superclassDecl->getDeclaredInterfaceType();
+    if (auto ref = tc.conformsToProtocol(superclassType, decodableProto,
+                                         superclassDecl,
+                                         ConformanceCheckOptions(),
+                                         SourceLoc())) {
+      // super conforms to Decodable, so we've failed to inherit init(from:).
+      // Let's suggest overriding it here.
+      //
+      // We're going to diagnose on the concrete init(from:) decl if it exists
+      // and isn't implicit; otherwise, on the subclass itself.
+      ValueDecl *diagDest = classDecl;
+      auto initFrom = DeclName(C, C.Id_init, C.Id_from);
+      auto result = tc.lookupMember(superclassDecl, superclassType, initFrom,
+                                    NameLookupFlags::ProtocolMembers |
+                                    NameLookupFlags::IgnoreAccessibility);
+
+      if (!result.empty() && !result.front().getValueDecl()->isImplicit())
+        diagDest = result.front().getValueDecl();
+
+      auto diagName = diag::decodable_suggest_overriding_init_here;
+
+      // This is also a bit of a hack, but the best place we've got at the
+      // moment to suggest this.
+      //
+      // If the superclass also conforms to Encodable, it's quite
+      // likely that the user forgot to override its encode(to:). In this case,
+      // we can produce a slightly different diagnostic to suggest doing so.
+      auto *encodableProto = C.getProtocol(KnownProtocolKind::Encodable);
+      if ((ref = tc.conformsToProtocol(superclassType, encodableProto,
+                                       superclassDecl,
+                                       ConformanceCheckOptions(),
+                                       SourceLoc()))) {
+        // We only want to produce this version of the diagnostic if the
+        // subclass doesn't directly implement encode(to:).
+        // The direct lookup here won't see an encode(to:) if it is inherited
+        // from the superclass.
+        auto encodeTo = DeclName(C, C.Id_encode, C.Id_to);
+        if (classDecl->lookupDirect(encodeTo).empty())
+          diagName = diag::codable_suggest_overriding_init_here;
+      }
+
+      tc.diagnose(diagDest, diagName);
+    }
+  }
+
   for (auto member : classDecl->getMembers()) {
     auto pbd = dyn_cast<PatternBindingDecl>(member);
     if (!pbd)
@@ -8131,14 +8192,49 @@ void TypeChecker::addImplicitConstructors(NominalTypeDecl *decl) {
   bool FoundMemberwiseInitializedProperty = false;
   bool SuppressDefaultInitializer = false;
   bool SuppressMemberwiseInitializer = false;
+  bool FoundSynthesizedInit = false;
   bool FoundDesignatedInit = false;
   decl->setAddedImplicitInitializers();
+
+  // Before we look for constructors, we need to make sure that all synthesized
+  // initializers are properly synthesized.
+  //
+  // NOTE: Lookups of synthesized initializers MUST come after
+  //       decl->setAddedImplicitInitializers() in case synthesis requires
+  //       protocol conformance checking, which might be recursive here.
+  // FIXME: Disable this code and prevent _any_ implicit constructors from doing
+  //        this. Investigate why this hasn't worked otherwise.
+  DeclName synthesizedInitializers[1] = {
+    // init(from:) is synthesized by derived conformance to Decodable.
+    DeclName(Context, DeclBaseName(Context.Id_init), Context.Id_from)
+  };
+
+  auto initializerIsSynthesized = [=](ConstructorDecl *initializer) {
+    if (!initializer->isImplicit())
+      return false;
+
+    for (auto &name : synthesizedInitializers)
+      if (initializer->getFullName() == name)
+        return true;
+
+    return false;
+  };
+
+  for (auto &name : synthesizedInitializers) {
+    synthesizeMemberForLookup(decl, name);
+  }
+
   SmallPtrSet<CanType, 4> initializerParamTypes;
   llvm::SmallPtrSet<ConstructorDecl *, 4> overriddenInits;
   for (auto member : decl->getMembers()) {
     if (auto ctor = dyn_cast<ConstructorDecl>(member)) {
-      if (ctor->isDesignatedInit())
+      // Synthesized initializers others than the default initializer should
+      // not prevent default initializer synthesis.
+      if (initializerIsSynthesized(ctor)) {
+        FoundSynthesizedInit = true;
+      } else if (ctor->isDesignatedInit()) {
         FoundDesignatedInit = true;
+      }
 
       if (!ctor->isInvalid())
         initializerParamTypes.insert(getInitializerParamType(ctor));
@@ -8233,7 +8329,8 @@ void TypeChecker::addImplicitConstructors(NominalTypeDecl *decl) {
 
     // We can't define these overrides if we have any uninitialized
     // stored properties.
-    if (SuppressDefaultInitializer && !FoundDesignatedInit) {
+    if (SuppressDefaultInitializer && !FoundDesignatedInit
+        && !FoundSynthesizedInit) {
       diagnoseClassWithoutInitializers(*this, classDecl);
       return;
     }
@@ -8325,7 +8422,9 @@ void TypeChecker::addImplicitConstructors(NominalTypeDecl *decl) {
 
     // ... unless there are uninitialized stored properties.
     if (SuppressDefaultInitializer) {
-      diagnoseClassWithoutInitializers(*this, classDecl);
+      if (!FoundSynthesizedInit)
+        diagnoseClassWithoutInitializers(*this, classDecl);
+
       return;
     }
 

lib/Sema/TypeCheckDecl.cpp

@@ -0,0 +1,49 @@
+// RUN: %target-typecheck-verify-swift -verify-ignore-unknown
+
+// A class with no initializers (which has non-initialized properties so a
+// default constructor can be synthesized) should produce an error.
+class NoInitializers { // expected-error {{class 'NoInitializers' has no initializers}}
+// expected-note@-1 {{did you mean 'deinit'?}}
+  var x: Double // expected-note {{stored property 'x' without initial value prevents synthesized initializers}}
+
+  func foo() {
+    // The class should not receive a default constructor.
+    let _ = NoInitializers.init() // expected-error {{type 'NoInitializers' has no member 'init'}}
+  }
+}
+
+// A similar class with Codable properties adopting Codable should get a
+// synthesized init(from:), and thus not warn.
+class CodableNoExplicitInitializers : Codable {
+  var x: Double
+
+  func foo() {
+    // The class should receive a synthesized init(from:) and encode(to:)
+    let _ = CodableNoExplicitInitializers.init(from:)
+    let _ = CodableNoExplicitInitializers.encode(to:)
+
+    // It should not, however, receive a default constructor.
+    let _ = CodableNoExplicitInitializers.init() // expected-error {{missing argument for parameter 'from' in call}}
+  }
+}
+
+// A class with all initialized properties should receive a default constructor.
+class DefaultConstructed {
+  var x: Double = .pi
+
+  func foo() {
+    let _ = DefaultConstructed.init()
+  }
+}
+
+// A class with all initialized, Codable properties adopting Codable should get
+// the default constructor, along with a synthesized init(from:).
+class CodableDefaultConstructed : Codable {
+  var x: Double = .pi
+
+  func foo() {
+    let _ = CodableDefaultConstructed.init()
+    let _ = CodableDefaultConstructed.init(from:)
+    let _ = CodableDefaultConstructed.encode(to:)
+  }
+}

test/decl/protocol/special/coding/class_codable_default_initializer.swift

@@ -8,12 +8,12 @@ class C1 : Codable {
   var c: Nested = Nested()
 
   // CHECK: error: type 'C1' does not conform to protocol 'Decodable'
-  // CHECK: note: cannot automatically synthesize 'Decodable' because 'C1.Nested' does not conform to 'Decodable'
   // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
+  // CHECK: note: cannot automatically synthesize 'Decodable' because 'C1.Nested' does not conform to 'Decodable'
 
   // CHECK: error: type 'C1' does not conform to protocol 'Encodable'
-  // CHECK: note: cannot automatically synthesize 'Encodable' because 'C1.Nested' does not conform to 'Encodable'
   // CHECK: note: protocol requires function 'encode(to:)' with type 'Encodable'
+  // CHECK: note: cannot automatically synthesize 'Encodable' because 'C1.Nested' does not conform to 'Encodable'
 }
 
 // Codable class with non-enum CodingKeys.
@@ -30,12 +30,12 @@ class C2 : Codable {
   }
 
   // CHECK: error: type 'C2' does not conform to protocol 'Decodable'
-  // CHECK: note: cannot automatically synthesize 'Decodable' because 'CodingKeys' is not an enum
   // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
+  // CHECK: note: cannot automatically synthesize 'Decodable' because 'CodingKeys' is not an enum
 
   // CHECK: error: type 'C2' does not conform to protocol 'Encodable'
-  // CHECK: note: cannot automatically synthesize 'Encodable' because 'CodingKeys' is not an enum
   // CHECK: note: protocol requires function 'encode(to:)' with type 'Encodable'
+  // CHECK: note: cannot automatically synthesize 'Encodable' because 'CodingKeys' is not an enum
 }
 
 // Codable class with CodingKeys not conforming to CodingKey.
@@ -51,12 +51,12 @@ class C3 : Codable {
   }
 
   // CHECK: error: type 'C3' does not conform to protocol 'Decodable'
-  // CHECK: note: cannot automatically synthesize 'Decodable' because 'CodingKeys' does not conform to CodingKey
   // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
+  // CHECK: note: cannot automatically synthesize 'Decodable' because 'CodingKeys' does not conform to CodingKey
 
   // CHECK: error: type 'C3' does not conform to protocol 'Encodable'
-  // CHECK: note: cannot automatically synthesize 'Encodable' because 'CodingKeys' does not conform to CodingKey
   // CHECK: note: protocol requires function 'encode(to:)' with type 'Encodable'
+  // CHECK: note: cannot automatically synthesize 'Encodable' because 'CodingKeys' does not conform to CodingKey
 }
 
 // Codable class with extraneous CodingKeys
@@ -75,16 +75,16 @@ class C4 : Codable {
   }
 
   // CHECK: error: type 'C4' does not conform to protocol 'Decodable'
+  // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
   // CHECK: note: CodingKey case 'a2' does not match any stored properties
   // CHECK: note: CodingKey case 'b2' does not match any stored properties
   // CHECK: note: CodingKey case 'c2' does not match any stored properties
-  // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
 
   // CHECK: error: type 'C4' does not conform to protocol 'Encodable'
+  // CHECK: note: protocol requires function 'encode(to:)' with type 'Encodable'
   // CHECK: note: CodingKey case 'a2' does not match any stored properties
   // CHECK: note: CodingKey case 'b2' does not match any stored properties
   // CHECK: note: CodingKey case 'c2' does not match any stored properties
-  // CHECK: note: protocol requires function 'encode(to:)' with type 'Encodable'
 }
 
 // Codable class with non-decoded property (which has no default value).
@@ -98,12 +98,12 @@ class C5 : Codable {
     case c
   }
 
-  // CHECK: error: class 'C5' has no initializers
-  // CHECK: note: stored property 'b' without initial value prevents synthesized initializers
-
   // CHECK: error: type 'C5' does not conform to protocol 'Decodable'
-  // CHECK: note: cannot automatically synthesize 'Decodable' because 'b' does not have a matching CodingKey and does not have a default value
   // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
+  // CHECK: note: cannot automatically synthesize 'Decodable' because 'b' does not have a matching CodingKey and does not have a default value
+
+  // CHECK: error: class 'C5' has no initializers
+  // CHECK: note: stored property 'b' without initial value prevents synthesized initializers
 }
 
 // Codable class with non-decoded property (which has no default value).
@@ -122,8 +122,8 @@ class C6 : Codable {
   }
 
   // CHECK: error: type 'C6' does not conform to protocol 'Decodable'
-  // CHECK: note: cannot automatically synthesize 'Decodable' because 'b' does not have a matching CodingKey and does not have a default value
   // CHECK: note: protocol requires initializer 'init(from:)' with type 'Decodable'
+  // CHECK: note: cannot automatically synthesize 'Decodable' because 'b' does not have a matching CodingKey and does not have a default value
 }
 
 // Classes cannot yet synthesize Encodable or Decodable in extensions.