Merge pull request #41198 from AnthonyLatsis/se-309-fixit

[SE-309] CSDiag: Add a fix-it that replaces an existential parameter type with its generic counterpart

Author: AnthonyLatsis

include/swift/AST/DiagnosticsSema.def

@@ -140,8 +140,8 @@ ERROR(could_not_use_instance_member_on_type,none,
       "%select{| instance of nested}3 type %0",
       (Type, DeclNameRef, Type, bool))
 ERROR(could_not_use_member_on_existential,none,
-      "member %1 cannot be used on value of protocol type %0; use a generic"
-      " constraint instead",
+      "member %1 cannot be used on value of protocol type %0; consider using a"
+      " generic constraint instead",
       (Type, DeclNameRef))
 FIXIT(replace_with_type,"%0",(Type))
 FIXIT(insert_type_qualification,"%0.",(Type))

lib/Sema/CSDiagnostics.cpp

@@ -3933,19 +3933,145 @@ bool UnintendedExtraGenericParamMemberFailure::diagnoseAsError() {
 }
 
 bool InvalidMemberRefOnExistential::diagnoseAsError() {
-  auto anchor = getRawAnchor();
+  const auto Anchor = getRawAnchor();
 
-  DeclNameLoc nameLoc;
-  if (auto *UDE = getAsExpr<UnresolvedDotExpr>(anchor)) {
-    nameLoc = UDE->getNameLoc();
-  } else if (auto *UME = getAsExpr<UnresolvedMemberExpr>(anchor)) {
-    nameLoc = UME->getNameLoc();
+  DeclNameLoc NameLoc;
+  ParamDecl *PD = nullptr;
+  if (auto *UDE = getAsExpr<UnresolvedDotExpr>(Anchor)) {
+    NameLoc = UDE->getNameLoc();
+    if (auto *DRE = dyn_cast<DeclRefExpr>(UDE->getBase())) {
+      PD = dyn_cast<ParamDecl>(DRE->getDecl());
+    }
+  } else if (auto *UME = getAsExpr<UnresolvedMemberExpr>(Anchor)) {
+    NameLoc = UME->getNameLoc();
+  } else if (auto *SE = getAsExpr<SubscriptExpr>(Anchor)) {
+    if (auto *DRE = dyn_cast<DeclRefExpr>(SE->getBase())) {
+      PD = dyn_cast<ParamDecl>(DRE->getDecl());
+    }
   }
 
-  emitDiagnostic(diag::could_not_use_member_on_existential, getBaseType(),
-                 getName())
-      .highlight(nameLoc.getSourceRange())
-      .highlight(getSourceRange());
+  auto Diag = emitDiagnostic(diag::could_not_use_member_on_existential,
+                             getBaseType(), getName());
+  Diag.highlight(NameLoc.getSourceRange());
+  Diag.highlight(getSourceRange());
+
+  // If the base expression is a reference to a function or subscript
+  // parameter, offer a fixit that replaces the existential parameter type with
+  // its generic equivalent, e.g. func foo(p: any P) → func foo<T: P>(p: T).
+  // FIXME: Add an option to use 'some' vs. an explicit generic parameter.
+
+  if (!PD || !PD->getDeclContext()->getAsDecl())
+    return true;
+
+  // Code inside a subscript is bound against a duplicate set of implicit
+  // accessor parameters, which don't have a TypeRepr; dig out the corresponding
+  // explicit subscript parameter.
+  if (auto *const AD =
+          dyn_cast<AccessorDecl>(PD->getDeclContext()->getAsDecl())) {
+    auto *const SD = dyn_cast<SubscriptDecl>(AD->getStorage());
+    if (!SD)
+      return true;
+
+    const auto AccessorParams = AD->getParameters()->getArray();
+    const unsigned idx =
+        llvm::find(AccessorParams, PD) - AccessorParams.begin();
+
+    switch (AD->getAccessorKind()) {
+    case AccessorKind::Set:
+    case AccessorKind::WillSet:
+    case AccessorKind::DidSet:
+      // Ignore references to the 'newValue' or 'oldValue' parameters.
+      if (AccessorParams.front() == PD) {
+        return true;
+      }
+
+      PD = SD->getIndices()->get(idx - 1);
+      break;
+
+    case AccessorKind::Get:
+    case AccessorKind::Read:
+    case AccessorKind::Modify:
+    case AccessorKind::Address:
+    case AccessorKind::MutableAddress:
+      PD = SD->getIndices()->get(idx);
+      break;
+    }
+  }
+
+  // Bail out in the absence of a TypeRepr.
+  if (!PD->getTypeRepr())
+    return true;
+
+  // Give up on 'inout' parameters. The intent is far more vague in this case,
+  // and applying the fix-it would invalidate mutations.
+  if (PD->isInOut())
+    return true;
+
+  constexpr StringRef GPNamePlaceholder = "<#generic parameter name#>";
+  SourceRange TyReplacementRange;
+  SourceRange RemoveAnyRange;
+  SourceLoc GPDeclLoc;
+  std::string GPDeclStr;
+  {
+    llvm::raw_string_ostream OS(GPDeclStr);
+    auto *const GC = PD->getDeclContext()->getAsDecl()->getAsGenericContext();
+    if (GC->getParsedGenericParams()) {
+      GPDeclLoc = GC->getParsedGenericParams()->getRAngleLoc();
+      OS << ", ";
+    } else {
+      GPDeclLoc =
+          isa<AbstractFunctionDecl>(GC)
+              ? cast<AbstractFunctionDecl>(GC)->getParameters()->getLParenLoc()
+              : cast<SubscriptDecl>(GC)->getIndices()->getLParenLoc();
+      OS << "<";
+    }
+    OS << GPNamePlaceholder << ": ";
+
+    auto *TR = PD->getTypeRepr()->getWithoutParens();
+    if (auto *STR = dyn_cast<SpecifierTypeRepr>(TR)) {
+      TR = STR->getBase()->getWithoutParens();
+    }
+    if (auto *ETR = dyn_cast<ExistentialTypeRepr>(TR)) {
+      TR = ETR->getConstraint();
+      RemoveAnyRange = SourceRange(ETR->getAnyLoc(), TR->getStartLoc());
+      TR = TR->getWithoutParens();
+    }
+    if (auto *MTR = dyn_cast<MetatypeTypeRepr>(TR)) {
+      TR = MTR->getBase();
+
+      // (P & Q).Type -> T.Type
+      // (P).Type -> (T).Type
+      // ((P & Q)).Type -> ((T)).Type
+      if (auto *TTR = dyn_cast<TupleTypeRepr>(TR)) {
+        assert(TTR->isParenType());
+        if (!isa<CompositionTypeRepr>(TTR->getElementType(0))) {
+          TR = TR->getWithoutParens();
+        }
+      }
+    }
+    TyReplacementRange = TR->getSourceRange();
+
+    // Strip any remaining parentheses and print the conformance constraint.
+    TR->getWithoutParens()->print(OS);
+
+    if (!GC->getParsedGenericParams()) {
+      OS << ">";
+    }
+  }
+
+  // First, replace the constraint type with the generic parameter type
+  // placeholder.
+  Diag.fixItReplace(TyReplacementRange, GPNamePlaceholder);
+
+  // Remove 'any' if needed, using a character-based removal to pick up
+  // whitespaces between it and its constraint repr.
+  if (RemoveAnyRange.isValid()) {
+    Diag.fixItRemoveChars(RemoveAnyRange.Start, RemoveAnyRange.End);
+  }
+
+  // Finally, insert the generic parameter declaration.
+  Diag.fixItInsert(GPDeclLoc, GPDeclStr);
+
   return true;
 }
 

lib/Sema/TypeCheckDecl.cpp

@@ -1611,7 +1611,7 @@ bool TypeChecker::isAvailabilitySafeForConformance(
   return requirementInfo.isContainedIn(witnessInfo);
 }
 
-// Returns 'nullptr' if this is the setter's 'newValue' parameter;
+// Returns 'nullptr' if this is the 'newValue' or 'oldValue' parameter;
 // otherwise, returns the corresponding parameter of the subscript
 // declaration.
 static ParamDecl *getOriginalParamFromAccessor(AbstractStorageDecl *storage,
@@ -1623,11 +1623,9 @@ static ParamDecl *getOriginalParamFromAccessor(AbstractStorageDecl *storage,
   switch (accessor->getAccessorKind()) {
   case AccessorKind::DidSet:
   case AccessorKind::WillSet:
-      return nullptr;
-
   case AccessorKind::Set:
     if (param == accessorParams->get(0)) {
-      // This is the 'newValue' parameter.
+      // This is the 'newValue' or 'oldValue' parameter.
       return nullptr;
     }
 

test/Constraints/opened_existentials.swift

@@ -164,7 +164,7 @@ func testReturningOpaqueTypes(p: any P) {
   let q = p.getQ()
   let _: Int = q  // expected-error{{cannot convert value of type 'Q' to specified type 'Int'}}
 
-  p.getCollectionOf() // expected-error{{member 'getCollectionOf' cannot be used on value of protocol type 'P'; use a generic constraint instead}}
+  p.getCollectionOf() // expected-error{{member 'getCollectionOf' cannot be used on value of protocol type 'P'; consider using a generic constraint instead}}
 
   let q2 = getPQ(p)
   let _: Int = q2  // expected-error{{cannot convert value of type 'Q' to specified type 'Int'}}

test/Constraints/protocols.swift

@@ -370,8 +370,8 @@ func testClonableExistential(_ v: Clonable, _ vv: Clonable.Type) {
   let _: (Bool) -> Clonable? = id(vv.returnSelfIUOStatic as (Bool) -> Clonable?)
   let _: Clonable! = id(vv.returnSelfIUOStatic(true))
 
-  let _ = v.badClonerFn() // expected-error {{member 'badClonerFn' cannot be used on value of protocol type 'Clonable'; use a generic constraint instead}}
-  let _ = v.veryBadClonerFn() // expected-error {{member 'veryBadClonerFn' cannot be used on value of protocol type 'Clonable'; use a generic constraint instead}}
+  let _ = v.badClonerFn() // expected-error {{member 'badClonerFn' cannot be used on value of protocol type 'Clonable'; consider using a generic constraint instead}}
+  let _ = v.veryBadClonerFn() // expected-error {{member 'veryBadClonerFn' cannot be used on value of protocol type 'Clonable'; consider using a generic constraint instead}}
 
 }
 

test/Generics/function_defs.swift

@@ -38,7 +38,7 @@ func existential<T : EqualComparable, U : EqualComparable>(_ t1: T, t2: T, u: U)
   eqComp = u
   if t1.isEqual(eqComp) {} // expected-error{{cannot convert value of type 'EqualComparable' to expected argument type 'T'}}
   if eqComp.isEqual(t2) {}
-  // expected-error@-1 {{member 'isEqual' cannot be used on value of protocol type 'EqualComparable'; use a generic constraint instead}}
+  // expected-error@-1 {{member 'isEqual' cannot be used on value of protocol type 'EqualComparable'; consider using a generic constraint instead}}
 }
 
 protocol OtherEqualComparable {