Merge pull request #15751 from huonw/cond-conf-fixits

Fix & improve the fixits for conditional conformances.

Author: huonw

include/swift/AST/DiagnosticsSema.def

@@ -1453,6 +1453,9 @@ NOTE(note_explicitly_state_conditional_conformance_relaxed,none,
      "did you mean to explicitly state the conformance with relaxed bounds?", ())
 NOTE(note_explicitly_state_conditional_conformance_same,none,
      "did you mean to explicitly state the conformance with the same bounds?", ())
+NOTE(note_explicitly_state_conditional_conformance_noneditor,none,
+     "did you mean to explicitly state the conformance like '%0where ...'?",
+     (StringRef))
 ERROR(protocol_has_missing_requirements,none,
       "type %0 cannot conform to protocol %1 because it has requirements that "
       "cannot be satisfied", (Type, Type))

lib/Sema/TypeCheckProtocol.cpp

@@ -1300,8 +1300,9 @@ static void diagnoseConformanceImpliedByConditionalConformance(
   // want to encourage it.
 
   auto ext = cast<ExtensionDecl>(implyingConf->getDeclContext());
+  auto &ctxt = ext->getASTContext();
 
-  auto &SM = ext->getASTContext().SourceMgr;
+  auto &SM = ctxt.SourceMgr;
   StringRef extraIndent;
   StringRef indent = Lexer::getIndentationForLine(SM, loc, &extraIndent);
 
@@ -1313,13 +1314,27 @@ static void diagnoseConformanceImpliedByConditionalConformance(
     llvm::raw_svector_ostream prefixStream(prefix);
     llvm::raw_svector_ostream suffixStream(suffix);
 
-    prefixStream << "extension " << T << ": " << protoType << " ";
+    ValueDecl *decl = T->getAnyNominal();
+    if (!decl)
+      decl = T->getAnyGeneric();
+
+    prefixStream << "extension " << decl->getFullName() << ": " << protoType << " ";
     suffixStream << " {\n"
                  << indent << extraIndent << "<#witnesses#>\n"
                  << indent << "}\n\n"
                  << indent;
   }
 
+  if (!ctxt.LangOpts.DiagnosticsEditorMode) {
+    // The fixits below are too complicated for the command line: the suggested
+    // code ends up not being displayed, and the text by itself doesn't help. So
+    // instead we skip all that and just have some text.
+    Diags.diagnose(loc,
+                   diag::note_explicitly_state_conditional_conformance_noneditor,
+                   prefix.str());
+    return;
+  }
+
   // First, we do the fixit for "matching" requirements (i.e. X: P where T: P).
   bool matchingIsValid = true;
   llvm::SmallString<128> matchingFixit = prefix;

test/Generics/conditional_conformances.swift

@@ -279,16 +279,14 @@ struct InheritImplicitOne<T> {}
 // incorrect/insufficiently general).
 extension InheritImplicitOne: P5 where T: P1 {}
 // expected-error@-1{{conditional conformance of type 'InheritImplicitOne<T>' to protocol 'P5' does not imply conformance to inherited protocol 'P2'}}
-// expected-note@-2{{did you mean to explicitly state the conformance with the same bounds?}}
-// expected-note@-3{{did you mean to explicitly state the conformance with different bounds?}}
+// expected-note@-2{{did you mean to explicitly state the conformance like 'extension InheritImplicitOne: P2 where ...'?}}
 
 struct InheritImplicitTwo<T> {}
 // Even if we relax the rule about implication, this double-up should still be
 // an error, because either conformance could imply InheritImplicitTwo: P2.
 extension InheritImplicitTwo: P5 where T: P1 {}
 // expected-error@-1{{conditional conformance of type 'InheritImplicitTwo<T>' to protocol 'P5' does not imply conformance to inherited protocol 'P2'}}
-// expected-note@-2{{did you mean to explicitly state the conformance with the same bounds?}}
-// expected-note@-3{{did you mean to explicitly state the conformance with different bounds?}}
+// expected-note@-2{{did you mean to explicitly state the conformance like 'extension InheritImplicitTwo: P2 where ...'?}}
 extension InheritImplicitTwo: P6 where T: P1 {}
 
 // However, if there's a non-conditional conformance that implies something, we

test/Generics/conditional_conformances_fixit.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all
+// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all -diagnostics-editor-mode
 // RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
 
 protocol P1 {}

test/Generics/conditional_conformances_fixit.swift.result

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all
+// RUN: %target-typecheck-verify-swift -emit-fixits-path %t.remap -fixit-all -diagnostics-editor-mode
 // RUN: c-arcmt-test %t.remap | arcmt-test -verify-transformed-files %s.result
 
 protocol P1 {}
@@ -7,11 +7,11 @@ protocol P2: P1 {}
 
 struct S1<T> {}
 
-extension S1<T>: P1 where T: P1 {
+extension S1: P1 where T: P1 {
     <#witnesses#>
 }
 
-extension S1<T>: P1 where <#requirements#> {
+extension S1: P1 where <#requirements#> {
     <#witnesses#>
 }
 
@@ -25,15 +25,15 @@ protocol P3 {
 }
 struct S2<T, U, V: P3> {}
 
-extension S2<T, U, V>: P1 where T: P1, U: P1, V.X: P1 {
+extension S2: P1 where T: P1, U: P1, V.X: P1 {
     <#witnesses#>
 }
 
-extension S2<T, U, V>: P1 where T: P2, U: P2, V.X: P2 {
+extension S2: P1 where T: P2, U: P2, V.X: P2 {
     <#witnesses#>
 }
 
-extension S2<T, U, V>: P1 where <#requirements#> {
+extension S2: P1 where <#requirements#> {
     <#witnesses#>
 }
 
@@ -46,11 +46,11 @@ extension S2: P2 where T: P2, U: P2, V.X: P2 {}
 
 struct S3<T, U, V: P3> {}
 
-extension S3<T, U, V>: P1 where T: P2, U: P2, V.X == Int {
+extension S3: P1 where T: P2, U: P2, V.X == Int {
     <#witnesses#>
 }
 
-extension S3<T, U, V>: P1 where <#requirements#> {
+extension S3: P1 where <#requirements#> {
     <#witnesses#>
 }
 
@@ -62,11 +62,11 @@ extension S3: P2 where T: P2, U: P2, V.X == Int {}
 
 struct S4<T, U, V: P3> {}
 
-extension S4<T, U, V>: P1 where T: P2, U: P3, V.X: P2 {
+extension S4: P1 where T: P2, U: P3, V.X: P2 {
     <#witnesses#>
 }
 
-extension S4<T, U, V>: P1 where <#requirements#> {
+extension S4: P1 where <#requirements#> {
     <#witnesses#>
 }