[Diag] QoI: Suggest 'weak' fix-it if unowned/unmanaged is optional (#16094)

Author: davezarzycki

include/swift/AST/DiagnosticsSema.def

@@ -3243,6 +3243,8 @@ ERROR(invalid_ownership_protocol_type,none,
       "%0 must not be applied to non-class-bound %1; "
       "consider adding a protocol conformance that has a class bound",
       (ReferenceOwnership, Type))
+ERROR(invalid_ownership_with_optional,none,
+      "%0 variable cannot have optional type", (ReferenceOwnership))
 ERROR(invalid_weak_ownership_not_optional,none,
       "'weak' variable should have optional type %0", (Type))
 ERROR(invalid_weak_let,none,

lib/Sema/TypeCheckAttr.cpp

@@ -2146,32 +2146,45 @@ void TypeChecker::checkReferenceOwnershipAttr(VarDecl *var,
   auto ownershipKind = attr->get();
 
   // A weak variable must have type R? or R! for some ownership-capable type R.
-  Type underlyingType = type;
+  auto underlyingType = type->getOptionalObjectType();
+  auto isOptional = bool(underlyingType);
+  auto allowOptional = false;
   switch (ownershipKind) {
   case ReferenceOwnership::Strong:
     llvm_unreachable("Cannot specify 'strong' in an ownership attribute");
   case ReferenceOwnership::Unowned:
   case ReferenceOwnership::Unmanaged:
     break;
   case ReferenceOwnership::Weak:
+    allowOptional = true;
     if (var->isLet()) {
       diagnose(var->getStartLoc(), diag::invalid_weak_let);
       attr->setInvalid();
     }
 
-    if (Type objType = type->getOptionalObjectType()) {
-      underlyingType = objType;
-    } else {
-      // @IBOutlet must be optional, but not necessarily weak. Let it diagnose.
-      if (!var->getAttrs().hasAttribute<IBOutletAttr>()) {
-        diagnose(var->getStartLoc(), diag::invalid_weak_ownership_not_optional,
-                 OptionalType::get(type));
-      }
-      attr->setInvalid();
+    if (isOptional)
+      break;
+
+    // @IBOutlet must be optional, but not necessarily weak. Let it diagnose.
+    if (!var->getAttrs().hasAttribute<IBOutletAttr>()) {
+      diagnose(var->getStartLoc(), diag::invalid_weak_ownership_not_optional,
+               OptionalType::get(type));
     }
+    attr->setInvalid();
     break;
   }
 
+
+  if (!allowOptional && isOptional) {
+    diagnose(var->getStartLoc(), diag::invalid_ownership_with_optional,
+             ownershipKind)
+      .fixItReplace(attr->getRange(), "weak");
+    attr->setInvalid();
+  }
+
+  if (!underlyingType)
+    underlyingType = type;
+
   if (!underlyingType->allowsOwnership()) {
     auto D = diag::invalid_ownership_type;
 

test/decl/var/properties.swift

@@ -1109,7 +1109,7 @@ class OwnershipBadSub : OwnershipBase {
   override weak var strongVar: AnyObject? { // expected-error {{cannot override 'strong' property with 'weak' property}}
     didSet {}
   }
-  override unowned var weakVar: AnyObject? { // expected-error {{'unowned' may only be applied to class and class-bound protocol types, not 'AnyObject?'}}
+  override unowned var weakVar: AnyObject? { // expected-error {{'unowned' variable cannot have optional type}}
     didSet {}
   }
   override weak var unownedVar: AnyObject { // expected-error {{'weak' variable should have optional type 'AnyObject?'}}