Merge pull request swiftlang#34648 from CodaFi/a-fully-qualified-success

[DiagnosticsQoI] Fully Qualify the Parent Type When Diagnosing Missing Member Types

Author: CodaFi

include/swift/AST/DiagnosticEngine.h

@@ -74,7 +74,24 @@ namespace swift {
       typedef T type;
     };
   }
-    
+
+  /// A family of wrapper types for compiler data types that forces its
+  /// underlying data to be formatted with full qualification.
+  ///
+  /// So far, this is only useful for \c Type, hence the SFINAE'ing.
+  template <typename T, typename = void> struct FullyQualified {};
+
+  template <typename T>
+  struct FullyQualified<
+      T, typename std::enable_if<std::is_convertible<T, Type>::value>::type> {
+    Type t;
+
+  public:
+    FullyQualified(T t) : t(t){};
+
+    Type getType() const { return t; }
+  };
+
   /// Describes the kind of diagnostic argument we're storing.
   ///
   enum class DiagnosticArgumentKind {
@@ -86,6 +103,7 @@ namespace swift {
     ValueDecl,
     Type,
     TypeRepr,
+    FullyQualifiedType,
     PatternKind,
     SelfAccessKind,
     ReferenceOwnership,
@@ -116,6 +134,7 @@ namespace swift {
       ValueDecl *TheValueDecl;
       Type TypeVal;
       TypeRepr *TyR;
+      FullyQualified<Type> FullyQualifiedTypeVal;
       PatternKind PatternKindVal;
       SelfAccessKind SelfAccessKindVal;
       ReferenceOwnership ReferenceOwnershipVal;
@@ -172,6 +191,10 @@ namespace swift {
       : Kind(DiagnosticArgumentKind::TypeRepr), TyR(T) {
     }
 
+    DiagnosticArgument(FullyQualified<Type> FQT)
+        : Kind(DiagnosticArgumentKind::FullyQualifiedType),
+          FullyQualifiedTypeVal(FQT) {}
+
     DiagnosticArgument(const TypeLoc &TL) {
       if (TypeRepr *tyR = TL.getTypeRepr()) {
         Kind = DiagnosticArgumentKind::TypeRepr;
@@ -268,7 +291,12 @@ namespace swift {
       assert(Kind == DiagnosticArgumentKind::TypeRepr);
       return TyR;
     }
-    
+
+    FullyQualified<Type> getAsFullyQualifiedType() const {
+      assert(Kind == DiagnosticArgumentKind::FullyQualifiedType);
+      return FullyQualifiedTypeVal;
+    }
+
     PatternKind getAsPatternKind() const {
       assert(Kind == DiagnosticArgumentKind::PatternKind);
       return PatternKindVal;

include/swift/AST/DiagnosticsSema.def

@@ -802,7 +802,8 @@ NOTE(invalid_redecl_implicit_wrapper,none,
 ERROR(ambiguous_type_base,none,
       "%0 is ambiguous for type lookup in this context", (DeclNameRef))
 ERROR(invalid_member_type,none,
-      "%0 is not a member type of %1", (DeclNameRef, Type))
+      "%0 is not a member type of %1 %2",
+      (DeclNameRef, DescriptiveDeclKind, FullyQualified<Type>))
 ERROR(invalid_member_type_suggest,none,
       "%0 does not have a member type named %1; did you mean %2?",
       (Type, DeclNameRef, DeclName))

lib/AST/DiagnosticEngine.cpp

@@ -526,21 +526,30 @@ static void formatDiagnosticArgument(StringRef Modifier,
     Out << FormatOpts.ClosingQuotationMark;
     break;
 
+  case DiagnosticArgumentKind::FullyQualifiedType:
   case DiagnosticArgumentKind::Type: {
     assert(Modifier.empty() && "Improper modifier for Type argument");
 
     // Strip extraneous parentheses; they add no value.
-    auto type = Arg.getAsType()->getWithoutParens();
+    Type type;
+    bool needsQualification = false;
+
+    if (Arg.getKind() == DiagnosticArgumentKind::Type) {
+      type = Arg.getAsType()->getWithoutParens();
+      needsQualification = typeSpellingIsAmbiguous(type, Args);
+    } else {
+      assert(Arg.getKind() == DiagnosticArgumentKind::FullyQualifiedType);
+      type = Arg.getAsFullyQualifiedType().getType()->getWithoutParens();
+      needsQualification = true;
+    }
 
     // If a type has an unresolved type, print it with syntax sugar removed for
     // clarity. For example, print `Array<_>` instead of `[_]`.
     if (type->hasUnresolvedType()) {
       type = type->getWithoutSyntaxSugar();
     }
 
-    bool isAmbiguous = typeSpellingIsAmbiguous(type, Args);
-
-    if (isAmbiguous && isa<OpaqueTypeArchetypeType>(type.getPointer())) {
+    if (needsQualification && isa<OpaqueTypeArchetypeType>(type.getPointer())) {
       auto opaqueTypeDecl = type->castTo<OpaqueTypeArchetypeType>()->getDecl();
 
       llvm::SmallString<256> NamingDeclText;
@@ -562,7 +571,7 @@ static void formatDiagnosticArgument(StringRef Modifier,
 
     } else {
       auto printOptions = PrintOptions();
-      printOptions.FullyQualifiedTypes = isAmbiguous;
+      printOptions.FullyQualifiedTypes = needsQualification;
       std::string typeName = type->getString(printOptions);
 
       if (shouldShowAKA(type, typeName)) {

lib/Sema/TypeCheckType.cpp

@@ -153,6 +153,17 @@ Type TypeResolution::mapTypeIntoContext(Type type) const {
   llvm_unreachable("unhandled stage");
 }
 
+// FIXME: It would be nice to have a 'DescriptiveTypeKind' abstraction instead.
+static DescriptiveDeclKind describeDeclOfType(Type t) {
+  if (!t) {
+    return DescriptiveDeclKind::Type;
+  }
+  if (auto *NTD = t->getAnyNominal()) {
+    return NTD->getDescriptiveKind();
+  }
+  return DescriptiveDeclKind::Type;
+}
+
 Type TypeResolution::resolveDependentMemberType(
                                           Type baseTy, DeclContext *DC,
                                           SourceRange baseRange,
@@ -211,8 +222,9 @@ Type TypeResolution::resolveDependentMemberType(
     if (!singleType) {
       auto name = ref->getNameRef();
       auto nameLoc = ref->getNameLoc();
-      ctx.Diags.diagnose(nameLoc, diag::invalid_member_type, name, baseTy)
-        .highlight(baseRange);
+      const auto kind = describeDeclOfType(baseTy);
+      ctx.Diags.diagnose(nameLoc, diag::invalid_member_type, name, kind, baseTy)
+          .highlight(baseRange);
       corrections.noteAllCandidates();
 
       return ErrorType::get(ctx);
@@ -1160,8 +1172,10 @@ static Type diagnoseUnknownType(TypeResolution resolution,
 
   // Qualified lookup case.
   if (!parentType->mayHaveMembers()) {
-    diags.diagnose(comp->getNameLoc(), diag::invalid_member_type,
-                   comp->getNameRef(), parentType)
+    const auto kind = describeDeclOfType(parentType);
+    diags
+        .diagnose(comp->getNameLoc(), diag::invalid_member_type,
+                  comp->getNameRef(), kind, parentType)
         .highlight(parentRange);
     return ErrorType::get(ctx);
   }
@@ -1214,9 +1228,11 @@ static Type diagnoseUnknownType(TypeResolution resolution,
                      parentType)
           .highlight(parentRange);
     } else {
-      diags.diagnose(comp->getNameLoc(), diag::invalid_member_type,
-                     comp->getNameRef(), parentType)
-        .highlight(parentRange);
+      const auto kind = describeDeclOfType(parentType);
+      diags
+          .diagnose(comp->getNameLoc(), diag::invalid_member_type,
+                    comp->getNameRef(), kind, parentType)
+          .highlight(parentRange);
       // Note where the type was defined, this can help diagnose if the user
       // expected name lookup to find a module when there's a conflicting type.
       if (auto typeDecl = parentType->getNominalOrBoundGenericNominal()) {

test/AutoDiff/Sema/differentiable_attr_type_checking.swift

@@ -632,7 +632,7 @@ class Super: Differentiable {
   // TODO(TF-632): Fix "'TangentVector' is not a member type of 'Self'" diagnostic.
   // The underlying error should appear instead:
   // "covariant 'Self' can only appear at the top level of method result type".
-  // expected-error @+1 2 {{'TangentVector' is not a member type of 'Self'}}
+  // expected-error @+1 2 {{'TangentVector' is not a member type of type 'Self'}}
   func vjpDynamicSelfResult() -> (Self, (Self.TangentVector) -> Self.TangentVector) {
     return (self, { $0 })
   }

test/Constraints/invalid_archetype_constraint.swift

@@ -19,6 +19,6 @@ struct A2<T> : P {
   typealias Element = T
 }
 
-func toA<S: Empty, AT:P>(_ s: S) -> AT where AT.Element == S.Generator.Element { // expected-error{{'Generator' is not a member type of 'S'}}
+func toA<S: Empty, AT:P>(_ s: S) -> AT where AT.Element == S.Generator.Element { // expected-error{{'Generator' is not a member type of type 'S'}}
   return AT()
 }

test/Constraints/invalid_constraint_lookup.swift

@@ -25,6 +25,6 @@ protocol _Collection {
 protocol Collection : _Collection, Sequence {
   subscript(i: Index) -> Iterator.Element {get set }
 }
-func insertionSort<C: Mutable> (_ elements: inout C, i: C.Index) { // expected-error {{cannot find type 'Mutable' in scope}} expected-error {{'Index' is not a member type of 'C'}}
-  var x: C.Iterator.Element = elements[i] // expected-error {{'Iterator' is not a member type of 'C'}}
+func insertionSort<C: Mutable> (_ elements: inout C, i: C.Index) { // expected-error {{cannot find type 'Mutable' in scope}} expected-error {{'Index' is not a member type of type 'C'}}
+  var x: C.Iterator.Element = elements[i] // expected-error {{'Iterator' is not a member type of type 'C'}}
 }

test/Constraints/same_types.swift

@@ -141,7 +141,7 @@ func test8b<T: Barrable, U: Barrable>(_ t: T, u: U)
 }
 
 // rdar://problem/19137463
-func rdar19137463<T>(_ t: T) where T.a == T {} // expected-error{{'a' is not a member type of 'T'}}
+func rdar19137463<T>(_ t: T) where T.a == T {} // expected-error{{'a' is not a member type of type 'T'}}
 rdar19137463(1)
 
 

test/Generics/associated_type_typo.swift

@@ -79,4 +79,4 @@ class D {
   typealias BElement = Int // expected-note{{did you mean 'BElement'?}}
 }
 
-func typoSuperclass2<T : D>(_: T, _: T.Element) { } // expected-error{{'Element' is not a member type of 'T'}}
+func typoSuperclass2<T : D>(_: T, _: T.Element) { } // expected-error{{'Element' is not a member type of type 'T'}}

test/Generics/function_defs.swift

@@ -290,7 +290,7 @@ func sameTypeEq<T>(_: T) where T = T {} // expected-error{{use '==' for same-typ
 
 func badTypeConformance1<T>(_: T) where Int : EqualComparable {} // expected-error{{type 'Int' in conformance requirement does not refer to a generic parameter or associated type}}
 
-func badTypeConformance2<T>(_: T) where T.Blarg : EqualComparable { } // expected-error{{'Blarg' is not a member type of 'T'}}
+func badTypeConformance2<T>(_: T) where T.Blarg : EqualComparable { } // expected-error{{'Blarg' is not a member type of type 'T'}}
 
 func badTypeConformance3<T>(_: T) where (T) -> () : EqualComparable { }
 // expected-error@-1{{type '(T) -> ()' in conformance requirement does not refer to a generic parameter or associated type}}