Merge pull request #4936 from jrose-apple/swift-3-generic-param-fix-it

Provide a fix-it that inserts omitted generic parameters when they can't be inferred.

Author: tkremenek

include/swift/AST/DiagnosticsSema.def

@@ -850,6 +850,8 @@ ERROR(unbound_generic_parameter_cast,none,
       "generic parameter %0 could not be inferred in cast to %1", (Type, Type))
 NOTE(archetype_declared_in_type,none,
      "%0 declared as parameter to type %1", (Type, Type))
+NOTE(unbound_generic_parameter_explicit_fix,none,
+     "explicitly specify the generic arguments to fix this issue", ())
 
 
 ERROR(string_index_not_integer,none,

lib/FrontendTool/FrontendTool.cpp

@@ -648,7 +648,8 @@ class JSONFixitWriter : public DiagnosticConsumer {
         Info.ID == diag::deprecated_protocol_composition.ID ||
         Info.ID == diag::deprecated_protocol_composition_single.ID ||
         Info.ID == diag::deprecated_any_composition.ID ||
-        Info.ID == diag::deprecated_operator_body.ID)
+        Info.ID == diag::deprecated_operator_body.ID ||
+        Info.ID == diag::unbound_generic_parameter_explicit_fix.ID)
       return true;
 
     return false;

lib/Sema/CSDiag.cpp

@@ -6488,39 +6488,112 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
   diagnosis.diagnoseAmbiguity(expr);
 }
 
+static bool hasArchetype(const GenericTypeDecl *generic,
+                         const ArchetypeType *archetype) {
+  return std::any_of(generic->getInnermostGenericParamTypes().begin(),
+                     generic->getInnermostGenericParamTypes().end(),
+                     [archetype](const GenericTypeParamType *genericParamTy) {
+    return genericParamTy->getDecl()->getArchetype() == archetype;
+  });
+}
+
 static void noteArchetypeSource(const TypeLoc &loc, ArchetypeType *archetype,
-                                TypeChecker &tc) {
-  GenericTypeDecl *FoundDecl = nullptr;
-  
+                                ConstraintSystem &cs) {
+  const GenericTypeDecl *FoundDecl = nullptr;
+  const ComponentIdentTypeRepr *FoundGenericTypeBase = nullptr;
+
   // Walk the TypeRepr to find the type in question.
   if (auto typerepr = loc.getTypeRepr()) {
     struct FindGenericTypeDecl : public ASTWalker {
-      GenericTypeDecl *&FoundDecl;
-      FindGenericTypeDecl(GenericTypeDecl *&FoundDecl) : FoundDecl(FoundDecl) {
-      }
+      const GenericTypeDecl *FoundDecl = nullptr;
+      const ComponentIdentTypeRepr *FoundGenericTypeBase = nullptr;
+      const ArchetypeType *Archetype;
+
+      FindGenericTypeDecl(const ArchetypeType *Archetype)
+          : Archetype(Archetype) {}
 
       bool walkToTypeReprPre(TypeRepr *T) override {
         // If we already emitted the note, we're done.
         if (FoundDecl) return false;
 
-        if (auto ident = dyn_cast<ComponentIdentTypeRepr>(T))
-          FoundDecl = dyn_cast_or_null<GenericTypeDecl>(ident->getBoundDecl());
+        if (auto ident = dyn_cast<ComponentIdentTypeRepr>(T)) {
+          auto *generic =
+              dyn_cast_or_null<GenericTypeDecl>(ident->getBoundDecl());
+          if (hasArchetype(generic, Archetype)) {
+            FoundDecl = generic;
+            FoundGenericTypeBase = ident;
+            return false;
+          }
+        }
         // Keep walking.
         return true;
       }
-    } findGenericTypeDecl(FoundDecl);
-    
+    } findGenericTypeDecl(archetype);
+
     typerepr->walk(findGenericTypeDecl);
+    FoundDecl = findGenericTypeDecl.FoundDecl;
+    FoundGenericTypeBase = findGenericTypeDecl.FoundGenericTypeBase;
   }
-  
+
   // If we didn't find the type in the TypeRepr, fall back to the type in the
   // type checked expression.
-  if (!FoundDecl)
-    FoundDecl = loc.getType()->getAnyGeneric();
-  
-  if (FoundDecl)
-    tc.diagnose(FoundDecl, diag::archetype_declared_in_type, archetype,
-                FoundDecl->getDeclaredType());
+  if (!FoundDecl) {
+    if (const GenericTypeDecl *generic = loc.getType()->getAnyGeneric())
+      if (hasArchetype(generic, archetype))
+        FoundDecl = generic;
+  }
+
+  auto &tc = cs.getTypeChecker();
+  if (FoundDecl) {
+    tc.diagnose(FoundDecl, diag::archetype_declared_in_type,
+                archetype, FoundDecl->getDeclaredType());
+  }
+
+  if (FoundGenericTypeBase && !isa<GenericIdentTypeRepr>(FoundGenericTypeBase)){
+    assert(FoundDecl);
+
+    // If we can, prefer using any types already fixed by the constraint system.
+    // This lets us produce fixes like `Pair<Int, Any>` instead of defaulting to
+    // `Pair<Any, Any>`.
+    // Right now we only handle this when the type that's at fault is the
+    // top-level type passed to this function.
+    ArrayRef<Type> genericArgs;
+    if (auto *boundGenericTy = loc.getType()->getAs<BoundGenericType>()) {
+      if (boundGenericTy->getDecl() == FoundDecl)
+        genericArgs = boundGenericTy->getGenericArgs();
+    }
+
+    auto getPreferredType =
+        [&](const GenericTypeParamDecl *genericParam) -> Type {
+      // If we were able to get the generic arguments (i.e. the types used at
+      // FoundDecl's use site), we can prefer those...
+      if (genericArgs.empty())
+        return Type();
+
+      Type preferred = genericArgs[genericParam->getIndex()];
+      if (!preferred || preferred->is<ErrorType>())
+        return Type();
+
+      // ...but only if they were actually resolved by the constraint system
+      // despite the failure.
+      TypeVariableType *unused;
+      Type maybeFixedType = cs.getFixedTypeRecursive(preferred, unused,
+                                                     /*wantRValue*/true);
+      if (maybeFixedType->hasTypeVariable() ||
+          maybeFixedType->hasUnresolvedType()) {
+        return Type();
+      }
+      return maybeFixedType;
+    };
+
+    SmallString<64> genericParamBuf;
+    if (tc.getDefaultGenericArgumentsString(genericParamBuf, FoundDecl,
+                                            getPreferredType)) {
+      tc.diagnose(FoundGenericTypeBase->getLoc(),
+                  diag::unbound_generic_parameter_explicit_fix)
+        .fixItInsertAfter(FoundGenericTypeBase->getEndLoc(), genericParamBuf);
+    }
+  }
 }
 
 
@@ -6636,11 +6709,7 @@ void FailureDiagnosis::diagnoseUnboundArchetype(ArchetypeType *archetype,
       .highlight(ECE->getCastTypeLoc().getSourceRange());
 
     // Emit a note specifying where this came from, if we can find it.
-    noteArchetypeSource(ECE->getCastTypeLoc(), archetype, tc);
-    if (auto *ND = ECE->getCastTypeLoc().getType()
-          ->getNominalOrBoundGenericNominal())
-      tc.diagnose(ND, diag::archetype_declared_in_type, archetype,
-                  ND->getDeclaredType());
+    noteArchetypeSource(ECE->getCastTypeLoc(), archetype, *CS);
     return;
   }
 
@@ -6670,7 +6739,7 @@ void FailureDiagnosis::diagnoseUnboundArchetype(ArchetypeType *archetype,
 
   if (auto TE = dyn_cast<TypeExpr>(anchor)) {
     // Emit a note specifying where this came from, if we can find it.
-    noteArchetypeSource(TE->getTypeLoc(), archetype, tc);
+    noteArchetypeSource(TE->getTypeLoc(), archetype, *CS);
     return;
   }
 

lib/Sema/MiscDiagnostics.cpp

@@ -938,6 +938,69 @@ static void diagnoseImplicitSelfUseInClosure(TypeChecker &TC, const Expr *E,
   const_cast<Expr *>(E)->walk(DiagnoseWalker(TC, isAlreadyInClosure));
 }
 
+bool TypeChecker::getDefaultGenericArgumentsString(
+    SmallVectorImpl<char> &buf,
+    const swift::GenericTypeDecl *typeDecl,
+    llvm::function_ref<Type(const GenericTypeParamDecl *)> getPreferredType) {
+  llvm::raw_svector_ostream genericParamText{buf};
+  genericParamText << "<";
+
+  auto printGenericParamSummary =
+      [&](const GenericTypeParamType *genericParamTy) {
+    const GenericTypeParamDecl *genericParam = genericParamTy->getDecl();
+    if (Type result = getPreferredType(genericParam)) {
+      result.print(genericParamText);
+      return;
+    }
+
+    ArrayRef<ProtocolDecl *> protocols =
+        genericParam->getConformingProtocols(this);
+
+    if (Type superclass = genericParam->getSuperclass()) {
+      if (protocols.empty()) {
+        superclass.print(genericParamText);
+        return;
+      }
+
+      genericParamText << "<#" << genericParam->getName() << ": ";
+      superclass.print(genericParamText);
+      for (const ProtocolDecl *proto : protocols) {
+        if (proto->isSpecificProtocol(KnownProtocolKind::AnyObject))
+          continue;
+        genericParamText << " & " << proto->getName();
+      }
+      genericParamText << "#>";
+      return;
+    }
+
+    if (protocols.empty()) {
+      genericParamText << Context.Id_Any;
+      return;
+    }
+
+    if (protocols.size() == 1 &&
+        (protocols.front()->isObjC() ||
+         protocols.front()->isSpecificProtocol(KnownProtocolKind::AnyObject))) {
+      genericParamText << protocols.front()->getName();
+      return;
+    }
+
+    genericParamText << "<#" << genericParam->getName() << ": ";
+    interleave(protocols,
+               [&](const ProtocolDecl *proto) {
+                 genericParamText << proto->getName();
+               },
+               [&] { genericParamText << " & "; });
+    genericParamText << "#>";
+  };
+
+  interleave(typeDecl->getInnermostGenericParamTypes(),
+             printGenericParamSummary, [&]{ genericParamText << ", "; });
+
+  genericParamText << ">";
+  return true;
+}
+
 /// Diagnose an argument labeling issue, returning true if we successfully
 /// diagnosed the issue.
 bool swift::diagnoseArgumentLabelError(TypeChecker &TC, const Expr *expr,

lib/Sema/TypeCheckType.cpp

@@ -622,41 +622,9 @@ static void diagnoseUnboundGenericType(TypeChecker &tc, Type ty,SourceLoc loc) {
     InFlightDiagnostic diag = tc.diagnose(loc,
         diag::generic_type_requires_arguments, ty);
     if (auto *genericD = unbound->getDecl()) {
-
-      // Tries to infer the type arguments to pass.
-      // Currently it only works if all the generic arguments have a super type,
-      // or it requires a class, in which case it infers 'AnyObject'.
-      auto inferGenericArgs = [](GenericTypeDecl *genericD)->std::string {
-        GenericParamList *genParamList = genericD->getGenericParams();
-        if (!genParamList)
-          return std::string();
-        auto params= genParamList->getParams();
-        if (params.empty())
-          return std::string();
-        std::string argsToAdd = "<";
-        for (unsigned i = 0, e = params.size(); i != e; ++i) {
-          auto param = params[i];
-          auto archTy = param->getArchetype();
-          if (!archTy)
-            return std::string();
-          if (auto superTy = archTy->getSuperclass()) {
-            argsToAdd += superTy.getString();
-          } else if (archTy->requiresClass()) {
-            argsToAdd += "AnyObject";
-          } else {
-            return std::string(); // give up.
-          }
-          if (i < e-1)
-            argsToAdd += ", ";
-        }
-        argsToAdd += ">";
-        return argsToAdd;
-      };
-
-      std::string genericArgsToAdd = inferGenericArgs(genericD);
-      if (!genericArgsToAdd.empty()) {
+      SmallString<64> genericArgsToAdd;
+      if (tc.getDefaultGenericArgumentsString(genericArgsToAdd, genericD))
         diag.fixItInsertAfter(loc, genericArgsToAdd);
-      }
     }
   }
   tc.diagnose(unbound->getDecl()->getLoc(), diag::generic_type_declared_here,

lib/Sema/TypeChecker.h

@@ -1968,6 +1968,23 @@ class TypeChecker final : public LazyResolver {
                                               ConstructorDecl *ctorDecl,
                                               bool SuppressDiagnostics);
 
+  /// Builds a string representing a "default" generic argument list for
+  /// \p typeDecl. In general, this means taking the bound of each generic
+  /// parameter. The \p getPreferredType callback can be used to provide a
+  /// different type from the bound.
+  ///
+  /// It may not always be possible to find a single appropriate type for a
+  /// particular parameter (say, if it has two bounds). In this case, an
+  /// Xcode-style placeholder will be used instead.
+  ///
+  /// Returns true if the arguments list could be constructed, false if for
+  /// some reason it could not.
+  bool getDefaultGenericArgumentsString(
+      SmallVectorImpl<char> &buf,
+      const GenericTypeDecl *typeDecl,
+      llvm::function_ref<Type(const GenericTypeParamDecl *)> getPreferredType =
+          [](const GenericTypeParamDecl *) { return Type(); });
+
   /// Attempt to omit needless words from the name of the given declaration.
   Optional<DeclName> omitNeedlessWords(AbstractFunctionDecl *afd);
 

test/Constraints/bridging.swift

@@ -298,7 +298,7 @@ func rdar20029786(_ ns: NSString?) {
 
 // <rdar://problem/19813772> QoI: Using as! instead of as in this case produces really bad diagnostic
 func rdar19813772(_ nsma: NSMutableArray) {
-  var a1 = nsma as! Array // expected-error{{generic parameter 'Element' could not be inferred in cast to 'Array<_>'}}
+  var a1 = nsma as! Array // expected-error{{generic parameter 'Element' could not be inferred in cast to 'Array<_>'}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{26-26=<Any>}}
   // FIXME: The following diagnostic is misleading and should not happen: expected-warning@-1{{cast from 'NSMutableArray' to unrelated type 'Array<_>' always fails}}
   var a2 = nsma as! Array<AnyObject> // expected-warning{{forced cast from 'NSMutableArray' to 'Array<AnyObject>' always succeeds; did you mean to use 'as'?}} {{17-20=as}}
   var a3 = nsma as Array<AnyObject>

test/Constraints/construction.swift

@@ -57,7 +57,7 @@ acceptString("\(hello), \(world) #\(i)!")
 Optional<Int>(1) // expected-warning{{unused}}
 Optional(1) // expected-warning{{unused}}
 _ = .none as Optional<Int>
-Optional(.none) // expected-error{{generic parameter 'T' could not be inferred}}
+Optional(.none) // expected-error{{generic parameter 'T' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{9-9=<Any>}}
 
 // Interpolation
 _ = "\(hello), \(world) #\(i)!"