Suggest Replacement Types for Invalid Placeholders

Now that placeholder types are preserved, we can open them wherever they appear in positions where they are used as contextual types. Use the checks we already run in the primaries to ban placeholders in top-level positions. Only now, use the inferred type of any associated expressions or statements to present the user with a contextual return type.

For parameters in functions and enum cases, we use any default argument expressions to get the contextual type.

For functions, we use a traversal similar to the opaque result type finder to find the type of any return statements in the program and present those as options.

Author: CodaFi

include/swift/AST/DiagnosticsSema.def

@@ -3703,6 +3703,12 @@ NOTE(descriptive_generic_type_declared_here,none,
 
 ERROR(placeholder_type_not_allowed,none,
       "type placeholder not allowed here", ())
+ERROR(placeholder_type_not_allowed_in_return_type,none,
+      "type placeholder may not appear in function return type", ())
+ERROR(placeholder_type_not_allowed_in_parameter,none,
+      "type placeholder may not appear in top-level parameter", ())
+NOTE(replace_placeholder_with_inferred_type,none,
+     "replace the placeholder with the correct type %0", (Type))
 
 WARNING(use_of_void_pointer,none,
 "Unsafe%0Pointer<Void> has been replaced by Unsafe%0RawPointer", (StringRef))

lib/Sema/MiscDiagnostics.cpp

@@ -2712,6 +2712,65 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
   }
 };
 
+class ReturnTypePlaceholderReplacer : public ASTWalker {
+  FuncDecl *Implementation;
+  BraceStmt *Body;
+  SmallVector<Type, 4> Candidates;
+
+  bool HasInvalidReturn = false;
+
+public:
+  ReturnTypePlaceholderReplacer(FuncDecl *Implementation, BraceStmt *Body)
+      : Implementation(Implementation), Body(Body) {}
+
+  void check() {
+    auto *resultRepr = Implementation->getResultTypeRepr();
+    if (!resultRepr) {
+      return;
+    }
+
+    Implementation->getASTContext()
+        .Diags
+        .diagnose(resultRepr->getLoc(),
+                  diag::placeholder_type_not_allowed_in_return_type)
+        .highlight(resultRepr->getSourceRange());
+
+    Body->walk(*this);
+
+    // If given function has any invalid returns in the body
+    // let's not try to validate the types, since it wouldn't
+    // be accurate.
+    if (HasInvalidReturn)
+      return;
+
+    auto writtenType = Implementation->getResultInterfaceType();
+    llvm::SmallPtrSet<TypeBase *, 8> seenTypes;
+    for (auto candidate : Candidates) {
+      if (!seenTypes.insert(candidate.getPointer()).second) {
+        continue;
+      }
+      TypeChecker::notePlaceholderReplacementTypes(writtenType, candidate);
+    }
+  }
+
+  std::pair<bool, Expr *> walkToExprPre(Expr *E) override { return {true, E}; }
+
+  std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
+    if (auto *RS = dyn_cast<ReturnStmt>(S)) {
+      if (RS->hasResult()) {
+        auto resultTy = RS->getResult()->getType();
+        HasInvalidReturn |= resultTy.isNull() || resultTy->hasError();
+        Candidates.push_back(resultTy);
+      }
+    }
+
+    return {true, S};
+  }
+
+  // Don't descend into nested decls.
+  bool walkToDeclPre(Decl *D) override { return false; }
+};
+
 } // end anonymous namespace
 
 // After we have scanned the entire region, diagnose variables that could be
@@ -3277,6 +3336,13 @@ void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
         OpaqueUnderlyingTypeChecker(AFD, opaqueResultTy, body).check();
       }
     }
+  } else if (auto *FD = dyn_cast<FuncDecl>(AFD)) {
+    auto resultIFaceTy = FD->getResultInterfaceType();
+    // If the result has a placeholder, we need to try to use the contextual
+    // type inferred in the body to replace it.
+    if (resultIFaceTy && resultIFaceTy->hasPlaceholder()) {
+      ReturnTypePlaceholderReplacer(FD, body).check();
+    }
   }
 }
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -18,19 +18,19 @@
 
 #include "CodeSynthesis.h"
 #include "DerivedConformances.h"
-#include "TypeChecker.h"
+#include "MiscDiagnostics.h"
 #include "TypeCheckAccess.h"
+#include "TypeCheckAvailability.h"
 #include "TypeCheckConcurrency.h"
 #include "TypeCheckDecl.h"
-#include "TypeCheckAvailability.h"
 #include "TypeCheckObjC.h"
 #include "TypeCheckType.h"
-#include "MiscDiagnostics.h"
-#include "swift/AST/AccessNotes.h"
-#include "swift/AST/AccessScope.h"
+#include "TypeChecker.h"
 #include "swift/AST/ASTPrinter.h"
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/ASTWalker.h"
+#include "swift/AST/AccessNotes.h"
+#include "swift/AST/AccessScope.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/ForeignErrorConvention.h"
@@ -42,15 +42,15 @@
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SourceFile.h"
+#include "swift/AST/TypeCheckRequests.h"
+#include "swift/AST/TypeDifferenceVisitor.h"
 #include "swift/AST/TypeWalker.h"
+#include "swift/Basic/Defer.h"
 #include "swift/Basic/Statistic.h"
 #include "swift/Parse/Lexer.h"
 #include "swift/Parse/Parser.h"
 #include "swift/Serialization/SerializedModuleLoader.h"
 #include "swift/Strings.h"
-#include "swift/AST/NameLookupRequests.h"
-#include "swift/AST/TypeCheckRequests.h"
-#include "swift/Basic/Defer.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
@@ -967,11 +967,67 @@ static void checkInheritedDefaultValueRestrictions(ParamDecl *PD) {
   }
 }
 
+void TypeChecker::notePlaceholderReplacementTypes(Type writtenType,
+                                                  Type inferredType) {
+  assert(writtenType && inferredType &&
+         "Must provide both written and inferred types");
+  assert(writtenType->hasPlaceholder() && "Written type has no placeholder?");
+
+  class PlaceholderNotator
+      : public CanTypeDifferenceVisitor<PlaceholderNotator> {
+  public:
+    bool visitDifferentComponentTypes(CanType t1, CanType t2) {
+      // Never replace anything the user wrote with an error type.
+      if (t2->hasError() || t2->hasUnresolvedType()) {
+        return false;
+      }
+
+      auto *placeholder = t1->getAs<PlaceholderType>();
+      if (!placeholder) {
+        return false;
+      }
+
+      if (auto *origRepr =
+              placeholder->getOriginator().dyn_cast<PlaceholderTypeRepr *>()) {
+        t1->getASTContext()
+            .Diags
+            .diagnose(origRepr->getLoc(),
+                      diag::replace_placeholder_with_inferred_type, t2)
+            .fixItReplace(origRepr->getSourceRange(), t2.getString());
+      }
+      return false;
+    }
+
+    bool check(Type t1, Type t2) {
+      return !visit(t1->getCanonicalType(), t2->getCanonicalType());
+    };
+  };
+
+  PlaceholderNotator().check(writtenType, inferredType);
+}
+
 /// Check the default arguments that occur within this pattern.
 static void checkDefaultArguments(ParameterList *params) {
   // Force the default values in case they produce diagnostics.
-  for (auto *param : *params)
-    (void)param->getTypeCheckedDefaultExpr();
+  for (auto *param : *params) {
+    auto ifacety = param->getInterfaceType();
+    auto *expr = param->getTypeCheckedDefaultExpr();
+    if (!ifacety->hasPlaceholder()) {
+      continue;
+    }
+
+    // Placeholder types are banned for all parameter decls. We try to use the
+    // freshly-checked default expression's contextual type to suggest a
+    // reasonable type to insert.
+    if (expr && !expr->getType()->hasError()) {
+      param->diagnose(diag::placeholder_type_not_allowed_in_parameter)
+          .highlight(param->getSourceRange());
+      TypeChecker::notePlaceholderReplacementTypes(
+          ifacety, expr->getType()->mapTypeOutOfContext());
+    } else {
+      param->diagnose(diag::placeholder_type_not_allowed);
+    }
+  }
 }
 
 Expr *DefaultArgumentExprRequest::evaluate(Evaluator &evaluator,

lib/Sema/TypeChecker.h

@@ -1138,6 +1138,24 @@ bool diagnoseInvalidFunctionType(FunctionType *fnTy, SourceLoc loc,
                                  DeclContext *dc,
                                  Optional<TypeResolutionStage> stage);
 
+/// Walk the parallel structure of a type with user-provided placeholders and
+/// an inferred type produced by the type checker. Where placeholders can be
+/// found, suggest the corresponding inferred type.
+///
+/// For example,
+///
+/// \code
+///  func foo(_ x: [_] = [0])
+/// \endcode
+///
+/// Has a written type of `(ArraySlice (Placeholder))` and an inferred type of
+/// `(ArraySlice Int)`, so we walk to `Placeholder` and `Int` in each type and
+/// suggest replacing `_` with `Int`.
+///
+/// \param writtenType The interface type usually derived from a user-written
+/// type repr. \param inferredType The type inferred by the type checker.
+void notePlaceholderReplacementTypes(Type writtenType, Type inferredType);
+
 }; // namespace TypeChecker
 
 /// Returns the protocol requirement kind of the given declaration.

test/Sema/placeholder_type.swift

@@ -7,7 +7,8 @@ let dict2: [Character: _] = ["h": 0]
 
 let arr = [_](repeating: "hi", count: 3)
 
-func foo(_ arr: [_] = [0]) {} // expected-error {{type placeholder not allowed here}}
+func foo(_ arr: [_] = [0]) {} // expected-error {{type placeholder may not appear in top-level parameter}}
+// expected-note@-1 {{replace the placeholder with the correct type 'Int'}}
 
 let foo = _.foo // expected-error {{could not infer type for placeholder}}
 let zero: _ = .zero // expected-error {{cannot infer contextual base in reference to member 'zero'}}
@@ -74,27 +75,35 @@ where T: ExpressibleByIntegerLiteral, U: ExpressibleByIntegerLiteral {
 }
 
 extension Bar {
-    func frobnicate2() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
-        return Bar(t: 42, u: 42)
-    }
-    func frobnicate3() -> Bar {
-        return Bar<_, _>(t: 42, u: 42)
-    }
-    func frobnicate4() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
-        return Bar<_, _>(t: 42, u: 42)
-    }
-    func frobnicate5() -> Bar<_, U> { // expected-error {{type placeholder not allowed here}}
-        return Bar(t: 42, u: 42)
-    }
-    func frobnicate6() -> Bar {
-        return Bar<_, U>(t: 42, u: 42)
-    }
-    func frobnicate7() -> Bar<_, _> { // expected-error {{type placeholder not allowed here}}
-        return Bar<_, U>(t: 42, u: 42)
-    }
-    func frobnicate8() -> Bar<_, U> { // expected-error {{type placeholder not allowed here}}
-        return Bar<_, _>(t: 42, u: 42)
-    }
+  func frobnicate2() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
+    // expected-note@-1 {{replace the placeholder with the correct type 'T'}}
+    // expected-note@-2 {{replace the placeholder with the correct type 'U'}}
+    return Bar(t: 42, u: 42)
+  }
+  func frobnicate3() -> Bar {
+    return Bar<_, _>(t: 42, u: 42)
+  }
+  func frobnicate4() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
+    // expected-note@-1 {{replace the placeholder with the correct type 'Int'}}
+    // expected-note@-2 {{replace the placeholder with the correct type 'Int'}}
+    return Bar<_, _>(t: 42, u: 42)
+  }
+  func frobnicate5() -> Bar<_, U> { // expected-error {{type placeholder may not appear in function return type}}
+    // expected-note@-1 {{replace the placeholder with the correct type 'T'}}
+    return Bar(t: 42, u: 42)
+  }
+  func frobnicate6() -> Bar {
+    return Bar<_, U>(t: 42, u: 42)
+  }
+  func frobnicate7() -> Bar<_, _> { // expected-error {{type placeholder may not appear in function return type}}
+    // expected-note@-1 {{replace the placeholder with the correct type 'Int'}}
+    // expected-note@-2 {{replace the placeholder with the correct type 'U'}}
+    return Bar<_, U>(t: 42, u: 42)
+  }
+  func frobnicate8() -> Bar<_, U> { // expected-error {{type placeholder may not appear in function return type}}
+    // expected-note@-1 {{replace the placeholder with the correct type 'Int'}}
+    return Bar<_, _>(t: 42, u: 42)
+  }
 }
 
 // FIXME: We should probably have better diagnostics for these situations--the user probably meant to use implicit member syntax
@@ -186,7 +195,7 @@ struct Just<Output>: Publisher {
 struct SetFailureType<Output, Failure>: Publisher {}
 
 extension Publisher {
-    func setFailureType<T>(to: T.Type) -> SetFailureType<Output, T> { // expected-note {{in call to function 'setFailureType(to:)'}}
+    func setFailureType<T>(to: T.Type) -> SetFailureType<Output, T> { // expected-note 2 {{in call to function 'setFailureType(to:)'}}
         return .init()
     }
 }
@@ -197,10 +206,10 @@ let _: SetFailureType<Int, (String) -> Double> = Just<Int>().setFailureType(to:
 let _: SetFailureType<Int, (String, Double)> = Just<Int>().setFailureType(to: (_, _).self)
 
 // TODO: Better error message here? Would be nice if we could point to the placeholder...
-let _: SetFailureType<Int, String> = Just<Int>().setFailureType(to: _.self).setFailureType(to: String.self) // expected-error {{placeholders are not allowed as top-level types}}
+let _: SetFailureType<Int, String> = Just<Int>().setFailureType(to: _.self).setFailureType(to: String.self) // expected-error {{generic parameter 'T' could not be inferred}}
 
-let _: (_) = 0 as Int // expected-error {{placeholders are not allowed as top-level types}}
-let _: Int = 0 as (_) // expected-error {{placeholders are not allowed as top-level types}}
+let _: (_) = 0 as Int
+let _: Int = 0 as (_)
 
 _ = (1...10)
     .map {
@@ -217,3 +226,31 @@ _ = (1...10)
     }
 
 let _: SetFailureType<Int, String> = Just<Int>().setFailureType(to: _.self).setFailureType(to: String.self) // expected-error {{generic parameter 'T' could not be inferred}}
+
+// N.B. The parallel structure of the annotation and inferred default
+// initializer types is all wrong. Plus, we do not trust
+// the contextual type with placeholders in it so the result is a generic
+// diagnostic.
+func mismatchedDefault<T>(_ x: [_] = [String: T]()) {} // expected-error {{type placeholder not allowed here}}
+
+func mismatchedReturnTypes() -> _ { // expected-error {{type placeholder may not appear in function return type}}
+  if true {
+    return "" // expected-note@-2 {{replace the placeholder with the correct type 'String'}}
+  } else {
+    return 0.5 // expected-note@-4 {{replace the placeholder with the correct type 'Double'}}
+  }
+}
+
+// FIXME: Opaque result types ought to be treated better than this. But it's
+// tricky to know which type is intended in a lot of cases.
+@available(iOS 13.0, OSX 10.15, tvOS 13.0, watchOS 6.0, *)
+func opaque() -> some _ { // expected-error {{type placeholder not allowed here}}
+  return Just<Int>().setFailureType(to: _.self)
+}
+
+enum EnumWithPlaceholders {
+  case topLevelPlaceholder(x: _) // expected-error {{type placeholder not allowed here}}
+  case placeholderWithDefault(x: _ = 5) // expected-error {{type placeholder may not appear in top-level parameter}}
+  // expected-note@-1 {{replace the placeholder with the correct type 'Int'}}
+}
+