[Sema] TypeWrapper: verify that type wrapper has both read-only and writable subscripts

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -6571,6 +6571,17 @@ ERROR(type_wrapper_requires_subscript,none,
       " - subscript(storedKeyPath:)",
       (DeclName))
 
+ERROR(type_wrapper_requires_readonly_subscript,none,
+      "type wrapper type %0 does not contain a required ready-only subscript",
+      (DeclName))
+
+ERROR(type_wrapper_requires_writable_subscript,none,
+      "type wrapper type %0 does not contain a required writable subscript",
+      (DeclName))
+
+NOTE(add_type_wrapper_subscript_stub_note,none,
+     "do you want to add a stub?", ())
+
 ERROR(type_wrapper_failable_init,none,
       "type wrapper initializer %0 cannot be failable", (DeclName))
 

lib/Sema/TypeCheckAttr.cpp

@@ -3771,6 +3771,9 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
     llvm::SmallDenseMap<SubscriptDecl *, SmallVector<UnviabilityReason, 2>, 2>
         nonViableSubscripts;
 
+    bool hasReadOnly = false;
+    bool hasWritable = false;
+
     for (auto *decl : subscripts) {
       auto *subscript = cast<SubscriptDecl>(decl);
 
@@ -3782,6 +3785,10 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
               BGT->isReferenceWritableKeyPath())) {
           nonViableSubscripts[subscript].push_back(
               UnviabilityReason::InvalidType);
+        } else {
+          hasReadOnly |= BGT->isKeyPath();
+          hasWritable |=
+              BGT->isWritableKeyPath() || BGT->isReferenceWritableKeyPath();
         }
       } else {
         nonViableSubscripts[subscript].push_back(
@@ -3793,6 +3800,41 @@ void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) {
             UnviabilityReason::Inaccessible);
     }
 
+    if (!hasReadOnly) {
+      auto &DE = ctx.Diags;
+      DE.diagnoseWithNotes(
+          DE.diagnose(nominal->getLoc(),
+                      diag::type_wrapper_requires_readonly_subscript,
+                      nominal->getName()),
+          [&]() {
+            DE.diagnose(nominal->getLoc(),
+                        diag::add_type_wrapper_subscript_stub_note)
+                .fixItInsertAfter(
+                    nominal->getBraces().Start,
+                    "\nsubscript<Value>(storageKeyPath path: KeyPath<<#Base#>, "
+                    "Value>) -> Value { get { <#code#> } }");
+          });
+      attr->setInvalid();
+    }
+
+    if (!hasWritable) {
+      auto &DE = ctx.Diags;
+      DE.diagnoseWithNotes(
+          DE.diagnose(nominal->getLoc(),
+                      diag::type_wrapper_requires_writable_subscript,
+                      nominal->getName()),
+          [&]() {
+            DE.diagnose(nominal->getLoc(),
+                        diag::add_type_wrapper_subscript_stub_note)
+                .fixItInsertAfter(
+                    nominal->getBraces().Start,
+                    "\nsubscript<Value>(storageKeyPath path: "
+                    "WritableKeyPath<<#Base#>, "
+                    "Value>) -> Value { get { <#code#> } set { <#code#> } }");
+          });
+      attr->setInvalid();
+    }
+
     if (subscripts.size() - nonViableSubscripts.size() == 0) {
       for (const auto &entry : nonViableSubscripts) {
         auto *subscript = entry.first;

test/SILOptimizer/type_wrapper_in_user_defined_init_lowering.sil

@@ -9,6 +9,7 @@ import SwiftShims
 @typeWrapper class MyWrapper<S> {
   @_hasStorage var underlying: S { get set }
   init(memberwise s: S)
+  subscript<V>(storageKeyPath path: KeyPath<S, V>) -> V { get }
   subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> V { get set }
   deinit
 }

test/type/type_wrapper.swift

@@ -33,6 +33,9 @@ struct FailableInit<S> {
 // Okay because there is a valid `init(memberwise:)` overload.
 @typeWrapper
 struct FailableAndValidInit<S> {
+  // expected-error@-1 {{type wrapper type 'FailableAndValidInit' does not contain a required writable subscript}}
+  // expected-note@-2 {{do you want to add a stub?}} {{33-33=\nsubscript<Value>(storageKeyPath path: WritableKeyPath<<#Base#>, Value>) -> Value { get { <#code#> \} set { <#code#> \} \}}}
+
   init(memberwise: S) {
   }
 
@@ -80,6 +83,11 @@ struct InaccessibleOrInvalidSubscripts<S> {
     get { fatalError() }
   }
 
+  private subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> [V] {
+    // expected-error@-1 {{private subscript 'subscript(storageKeyPath:)' cannot have more restrictive access than its enclosing type wrapper type 'InaccessibleOrInvalidSubscripts' (which is internal)}}
+    get { fatalError() }
+  }
+
   subscript(storageKeyPath path: Int) -> Bool { // expected-error {{type wrapper subscript expects a key path parameter type (got: 'Int')}}
     get { true }
   }
@@ -91,6 +99,10 @@ struct NoopWrapper<S> {
 
   subscript<V>(storageKeyPath path: KeyPath<S, V>) -> V {
     get { fatalError() }
+  }
+
+  subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> V {
+    get { fatalError() }
     set { }
   }
 }
@@ -130,6 +142,10 @@ struct Parent {
 
     subscript<V>(storageKeyPath path: KeyPath<S, V>) -> V {
       get { fatalError() }
+    }
+
+    subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> V {
+      get { fatalError() }
       set { }
     }
   }
@@ -437,3 +453,30 @@ func testIgnoredAttr() {
     }
   }
 }
+
+func testMissingReadOnlyAndWritableSubscriptsAreDiagnosed() {
+  @typeWrapper
+  struct MissingReadOnly<S> {
+    // expected-error@-1 {{type wrapper type 'MissingReadOnly' does not contain a required ready-only subscript}}
+    // expected-note@-2 {{do you want to add a stub?}} {{30-30=\nsubscript<Value>(storageKeyPath path: KeyPath<<#Base#>, Value>) -> Value { get { <#code#> \} \}}}
+
+    init(memberwise: S) {}
+
+    subscript<V>(storageKeyPath path: WritableKeyPath<S, V>) -> V {
+      get { fatalError() }
+      set { }
+    }
+  }
+
+  @typeWrapper
+  struct MissingWritable<S> {
+    // expected-error@-1 {{type wrapper type 'MissingWritable' does not contain a required writable subscript}}
+    // expected-note@-2 {{do you want to add a stub?}} {{30-30=\nsubscript<Value>(storageKeyPath path: WritableKeyPath<<#Base#>, Value>) -> Value { get { <#code#> \} set { <#code#> \} \}}}
+
+    init(memberwise: S) {}
+
+    subscript<V>(storageKeyPath path: KeyPath<S, V>) -> V {
+      get { fatalError() }
+    }
+  }
+}