Merge pull request #26970 from CodaFi/extension-winding

Fix a Hole in Extension Checking For Non-Nominal Types and Indirectly Nominal Types

Author: CodaFi

include/swift/AST/DiagnosticsSema.def

@@ -1625,6 +1625,11 @@ NOTE(objc_generic_extension_using_type_parameter_here,none,
      "generic parameter used here", ())
 NOTE(objc_generic_extension_using_type_parameter_try_objc,none,
      "add '@objc' to allow uses of 'self' within the function body", ())
+ERROR(invalid_nominal_extension,none,
+      "extension of type %0 must be declared as an extension of %1",
+      (Type, Type))
+NOTE(invalid_nominal_extension_rewrite,none,
+     "did you mean to extend %0 instead?", (Type))
 
 // Protocols
 ERROR(type_does_not_conform,none,

lib/Sema/TypeCheckAccess.cpp

@@ -1926,9 +1926,6 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
 
   void visitExtensionDecl(ExtensionDecl *ED) {
     auto extendedType = ED->getExtendedNominal();
-    // TODO: Sometimes we have an extension that is marked valid but has no
-    //       extended type. Assert, just in case we see it while testing, but
-    //       don't crash. rdar://50401284
     assert(extendedType && "valid extension with no extended type?");
     if (!extendedType || shouldSkipChecking(extendedType))
       return;

lib/Sema/TypeCheckDecl.cpp

@@ -3049,29 +3049,53 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   }
 
   void visitExtensionDecl(ExtensionDecl *ED) {
+    // Produce any diagnostics for the extended type.
+    auto extType = ED->getExtendedType();
+
+    auto nominal = ED->getExtendedNominal();
+    if (nominal == nullptr) {
+	  const bool wasAlreadyInvalid = ED->isInvalid();
+      ED->setInvalid();
+      if (extType && !extType->hasError() && extType->getAnyNominal()) {
+        // If we've got here, then we have some kind of extension of a prima
+        // fascie non-nominal type.  This can come up when we're projecting
+        // typealiases out of bound generic types.
+        //
+        // struct Array<T> { typealias Indices = Range<Int> }
+        // extension Array.Indices.Bound {}
+        //
+        // Offer to rewrite it to the underlying nominal type.
+        auto canExtType = extType->getCanonicalType();
+        ED->diagnose(diag::invalid_nominal_extension, extType, canExtType)
+          .highlight(ED->getExtendedTypeRepr()->getSourceRange());
+        ED->diagnose(diag::invalid_nominal_extension_rewrite, canExtType)
+          .fixItReplace(ED->getExtendedTypeRepr()->getSourceRange(),
+                        canExtType->getString());
+      } else if (!wasAlreadyInvalid) {
+		// If nothing else applies, fall back to a generic diagnostic.
+        ED->diagnose(diag::non_nominal_extension, extType);
+      }
+      return;
+    }
+
     TC.validateExtension(ED);
 
     checkInheritanceClause(ED);
 
-    if (auto nominal = ED->getExtendedNominal()) {
-      TC.validateDecl(nominal);
-
-      // Check the raw values of an enum, since we might synthesize
-      // RawRepresentable while checking conformances on this extension.
-      if (auto enumDecl = dyn_cast<EnumDecl>(nominal)) {
-        if (enumDecl->hasRawType())
-          checkEnumRawValues(TC, enumDecl);
-      }
+    // Check the raw values of an enum, since we might synthesize
+    // RawRepresentable while checking conformances on this extension.
+    if (auto enumDecl = dyn_cast<EnumDecl>(nominal)) {
+      if (enumDecl->hasRawType())
+        checkEnumRawValues(TC, enumDecl);
+    }
 
-      // Only generic and protocol types are permitted to have
-      // trailing where clauses.
-      if (auto trailingWhereClause = ED->getTrailingWhereClause()) {
-        if (!ED->getGenericParams() &&
-            !ED->isInvalid()) {
-          ED->diagnose(diag::extension_nongeneric_trailing_where,
-                       nominal->getFullName())
+    // Only generic and protocol types are permitted to have
+    // trailing where clauses.
+    if (auto trailingWhereClause = ED->getTrailingWhereClause()) {
+      if (!ED->getGenericParams() && !ED->isInvalid()) {
+        ED->diagnose(diag::extension_nongeneric_trailing_where,
+                     nominal->getFullName())
           .highlight(trailingWhereClause->getSourceRange());
-        }
       }
     }
 
@@ -4371,13 +4395,14 @@ static Type formExtensionInterfaceType(
 /// Check the generic parameters of an extension, recursively handling all of
 /// the parameter lists within the extension.
 static GenericEnvironment *
-checkExtensionGenericParams(TypeChecker &tc, ExtensionDecl *ext, Type type,
+checkExtensionGenericParams(TypeChecker &tc, ExtensionDecl *ext,
                             GenericParamList *genericParams) {
   assert(!ext->getGenericEnvironment());
 
   // Form the interface type of the extension.
   bool mustInferRequirements = false;
   SmallVector<std::pair<Type, Type>, 4> sameTypeReqs;
+  auto type = ext->getExtendedType();
   Type extInterfaceType =
     formExtensionInterfaceType(tc, ext, type, genericParams, sameTypeReqs,
                                mustInferRequirements);
@@ -4488,10 +4513,6 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
 
   DeclValidationRAII IBV(ext);
 
-  auto extendedType = evaluateOrDefault(Context.evaluator,
-                                        ExtendedTypeRequest{ext},
-                                        ErrorType::get(ext->getASTContext()));
-
   if (auto *nominal = ext->getExtendedNominal()) {
     // If this extension was not already bound, it means it is either in an
     // inactive conditional compilation block, or otherwise (incorrectly)
@@ -4504,8 +4525,7 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
     validateDecl(nominal);
 
     if (auto *genericParams = ext->getGenericParams()) {
-      GenericEnvironment *env =
-        checkExtensionGenericParams(*this, ext, extendedType, genericParams);
+      auto *env = checkExtensionGenericParams(*this, ext, genericParams);
       ext->setGenericEnvironment(env);
     }
   }

test/SourceKit/Indexing/index.swift.response

@@ -1222,13 +1222,6 @@
         }
       ]
     },
-    {
-      key.kind: source.lang.swift.decl.function.method.instance,
-      key.name: "meth()",
-      key.usr: <usr>,
-      key.line: 134,
-      key.column: 8
-    },
     {
       key.kind: source.lang.swift.decl.class,
       key.name: "CC4",

test/attr/dynamicReplacement.swift

@@ -50,14 +50,6 @@ extension K {
   func replacement_finalFunction() {}
 }
 
-extension undeclared { // expected-error{{use of undeclared type 'undeclared'}}
-  @_dynamicReplacement(for: property) // expected-error{{replaced accessor for 'property' could not be found}}
-  var replacement_property: Int { return 2 }
-
-  @_dynamicReplacement(for: func) // expected-error{{replaced function 'func' could not be found}}
-  func func2() -> Int { return 2 }
-}
-
 extension P {
   @_dynamicReplacement(for: v)
   var replacement_v : Int {

test/decl/ext/extensions.swift

@@ -153,3 +153,20 @@ extension DoesNotImposeClassReq_2 where Self : AnyObject {
     set { property = newValue } // Okay
   }
 }
+
+// Reject extension of nominal type via parameterized typealias
+
+struct Nest<Egg> { typealias Contents = Egg }
+struct Tree { 
+  typealias LimbContent = Nest<Int> 
+  typealias BoughPayload = Nest<Nest<Int>>
+}
+
+extension Tree.LimbContent.Contents {
+  // expected-error@-1 {{extension of type 'Tree.LimbContent.Contents' (aka 'Int') must be declared as an extension of 'Int'}}
+  // expected-note@-2 {{did you mean to extend 'Int' instead?}} {{11-36=Int}}
+}
+
+extension Tree.BoughPayload.Contents {
+ // expected-error@-1 {{constrained extension must be declared on the unspecialized generic type 'Nest'}}
+}

test/decl/nested/type_in_extension.swift

@@ -8,12 +8,12 @@ extension G {
   }
 }
 
-extension { // expected-error {{expected type name in extension declaration}}
-  struct S<T> {
+extension G {
+  struct S<T> { // expected-note {{generic type 'S' declared here}}
     func foo(t: T) {}
   }
 
-  class M : S {} // expected-error {{use of undeclared type 'S'}}
+  class M : S {} // expected-error {{reference to generic type 'G<T>.S' requires arguments in <...>}}
 
   protocol P { // expected-error {{protocol 'P' cannot be nested inside another declaration}}
     associatedtype A