Merge pull request swiftlang#27712 from CodaFi/a-quick-fixit-while-hes-away

[NFC] Refactor Override Fixit Emission

Author: CodaFi

lib/Sema/MiscDiagnostics.cpp

@@ -1799,9 +1799,15 @@ void swift::diagnoseUnownedImmediateDeallocation(TypeChecker &TC,
   }
 }
 
-bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
-                                          ValueDecl *decl,
-                                          const ValueDecl *base) {
+namespace {
+enum NoteKind_t {
+  FixItReplace,
+  FixItInsert,
+};
+
+static bool fixItOverrideDeclarationTypesImpl(
+    ValueDecl *decl, const ValueDecl *base,
+    SmallVectorImpl<std::tuple<NoteKind_t, SourceRange, std::string>> &notes) {
   // For now, just rewrite cases where the base uses a value type and the
   // override uses a reference type, and the value type is bridged to the
   // reference type. This is a way to migrate code that makes use of types
@@ -1864,7 +1870,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
     options.SynthesizeSugarOnTypes = true;
 
     newOverrideTy->print(baseTypeStr, options);
-    diag.fixItReplace(typeRange, baseTypeStr.str());
+    notes.emplace_back(FixItReplace, typeRange, baseTypeStr.str().str());
     return true;
   };
 
@@ -1884,7 +1890,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
         overrideFnTy->getExtInfo().isNoEscape() &&
         // The overridden function type should be escaping.
         !baseFnTy->getExtInfo().isNoEscape()) {
-      diag.fixItInsert(typeRange.Start, "@escaping ");
+      notes.emplace_back(FixItInsert, typeRange, "@escaping ");
       return true;
     }
     return false;
@@ -1922,7 +1928,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
       for_each(*fn->getParameters(),
                *baseFn->getParameters(),
                [&](ParamDecl *param, const ParamDecl *baseParam) {
-        fixedAny |= fixItOverrideDeclarationTypes(diag, param, baseParam);
+        fixedAny |= fixItOverrideDeclarationTypesImpl(param, baseParam, notes);
       });
     }
     if (auto *method = dyn_cast<FuncDecl>(decl)) {
@@ -1948,7 +1954,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
       for_each(*subscript->getIndices(),
                *baseSubscript->getIndices(),
                [&](ParamDecl *param, const ParamDecl *baseParam) {
-        fixedAny |= fixItOverrideDeclarationTypes(diag, param, baseParam);
+        fixedAny |= fixItOverrideDeclarationTypesImpl(param, baseParam, notes);
       });
     }
 
@@ -1964,6 +1970,26 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
 
   llvm_unreachable("unknown overridable member");
 }
+};
+
+bool swift::computeFixitsForOverridenDeclaration(
+    ValueDecl *decl, const ValueDecl *base,
+    llvm::function_ref<Optional<InFlightDiagnostic>(bool)> diag) {
+  SmallVector<std::tuple<NoteKind_t, SourceRange, std::string>, 4> Notes;
+  bool hasNotes = ::fixItOverrideDeclarationTypesImpl(decl, base, Notes);
+
+  Optional<InFlightDiagnostic> diagnostic = diag(hasNotes);
+  if (!diagnostic) return hasNotes;
+
+  for (const auto &note : Notes) {
+    if (std::get<0>(note) == FixItReplace) {
+      diagnostic->fixItReplace(std::get<1>(note), std::get<2>(note));
+    } else {
+      diagnostic->fixItInsert(std::get<1>(note).Start, std::get<2>(note));
+    }
+  }
+  return hasNotes;
+}
 
 //===----------------------------------------------------------------------===//
 // Per func/init diagnostics

lib/Sema/MiscDiagnostics.h

@@ -90,10 +90,15 @@ void diagnoseUnownedImmediateDeallocation(TypeChecker &TC,
 /// \p base...but only if we're highly confident that we know what the user
 /// should have written.
 ///
+/// The \p diag closure allows the caller to control the diagnostic that is
+/// emitted. It is passed true if the diagnostic will be emitted with fixits
+/// attached, and false otherwise. If None is returned, no diagnostics are
+/// emitted.  Else the fixits are attached to the returned diagnostic.
+///
 /// \returns true iff any fix-its were attached to \p diag.
-bool fixItOverrideDeclarationTypes(InFlightDiagnostic &diag,
-                                   ValueDecl *decl,
-                                   const ValueDecl *base);
+bool computeFixitsForOverridenDeclaration(
+    ValueDecl *decl, const ValueDecl *base,
+    llvm::function_ref<Optional<InFlightDiagnostic>(bool)> diag);
 
 /// Emit fix-its to enclose trailing closure in argument parens.
 void fixItEncloseTrailingClosure(TypeChecker &TC,

lib/Sema/TypeCheckDeclOverride.cpp

@@ -422,44 +422,40 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base);
 static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
   auto &ctx = decl->getASTContext();
   auto &diags = ctx.Diags;
-  DiagnosticTransaction tentativeDiags(diags);
 
-  {
-    Type baseTy = base->getInterfaceType();
-    if (baseTy->hasError())
-      return false;
-
-    Optional<InFlightDiagnostic> activeDiag;
-    if (auto *baseInit = dyn_cast<ConstructorDecl>(base)) {
-      // Special-case initializers, whose "type" isn't useful besides the
-      // input arguments.
-      auto *fnType = baseTy->getAs<AnyFunctionType>();
-      baseTy = fnType->getResult();
-      Type argTy = FunctionType::composeInput(ctx,
-                                              baseTy->getAs<AnyFunctionType>()
-                                                    ->getParams(),
-                                              false);
-      auto diagKind = diag::override_type_mismatch_with_fixits_init;
-      unsigned numArgs = baseInit->getParameters()->size();
-      activeDiag.emplace(diags.diagnose(decl, diagKind,
-                                        /*plural*/std::min(numArgs, 2U),
-                                        argTy));
-    } else {
-      if (isa<AbstractFunctionDecl>(base))
-        baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
-
-      activeDiag.emplace(
-        diags.diagnose(decl,
-                       diag::override_type_mismatch_with_fixits,
-                       base->getDescriptiveKind(), baseTy));
-    }
+  Type baseTy = base->getInterfaceType();
+  if (baseTy->hasError())
+    return false;
 
-    if (fixItOverrideDeclarationTypes(*activeDiag, decl, base))
-      return true;
+  if (auto *baseInit = dyn_cast<ConstructorDecl>(base)) {
+    // Special-case initializers, whose "type" isn't useful besides the
+    // input arguments.
+    auto *fnType = baseTy->getAs<AnyFunctionType>();
+    baseTy = fnType->getResult();
+    Type argTy = FunctionType::composeInput(
+        ctx, baseTy->getAs<AnyFunctionType>()->getParams(), false);
+    auto diagKind = diag::override_type_mismatch_with_fixits_init;
+    unsigned numArgs = baseInit->getParameters()->size();
+    return computeFixitsForOverridenDeclaration(
+        decl, base, [&](bool HasNotes) -> Optional<InFlightDiagnostic> {
+          if (!HasNotes)
+            return None;
+          return diags.diagnose(decl, diagKind,
+                                /*plural*/ std::min(numArgs, 2U), argTy);
+        });
+  } else {
+    if (isa<AbstractFunctionDecl>(base))
+      baseTy = baseTy->getAs<AnyFunctionType>()->getResult();
+
+    return computeFixitsForOverridenDeclaration(
+        decl, base, [&](bool HasNotes) -> Optional<InFlightDiagnostic> {
+          if (!HasNotes)
+            return None;
+          return diags.diagnose(decl, diag::override_type_mismatch_with_fixits,
+                                base->getDescriptiveKind(), baseTy);
+        });
   }
 
-  // There weren't any fixes we knew how to make. Drop this diagnostic.
-  tentativeDiags.abort();
   return false;
 }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -2069,12 +2069,19 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
     break;
 
   case MatchKind::TypeConflict: {
-    auto diag = diags.diagnose(match.Witness, 
-                               diag::protocol_witness_type_conflict,
-                               getTypeForDisplay(module, match.Witness),
-                               withAssocTypes);
-    if (!isa<TypeDecl>(req))
-      fixItOverrideDeclarationTypes(diag, match.Witness, req);
+    if (!isa<TypeDecl>(req)) {
+      computeFixitsForOverridenDeclaration(match.Witness, req, [&](bool){
+        return diags.diagnose(match.Witness,
+                              diag::protocol_witness_type_conflict,
+                              getTypeForDisplay(module, match.Witness),
+                              withAssocTypes);
+      });
+    } else {
+      diags.diagnose(match.Witness,
+                     diag::protocol_witness_type_conflict,
+                     getTypeForDisplay(module, match.Witness),
+                     withAssocTypes);
+    }
     break;
   }