Merge pull request #33337 from HassanElDesouky/localization-report-deleted-IDs

[Localization] Make the serialization tool print the removed diagnostics

Author: xedin

include/swift/AST/LocalizationFormat.h

@@ -18,6 +18,7 @@
 #define SWIFT_LOCALIZATIONFORMAT_H
 
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitstreamReader.h"
@@ -38,6 +39,7 @@ namespace swift {
 enum class DiagID : uint32_t;
 
 namespace diag {
+
 using namespace llvm::support;
 
 class LocalizationWriterInfo {
@@ -152,6 +154,8 @@ class YAMLLocalizationProducer final : public LocalizationProducer {
   std::vector<std::string> diagnostics;
 
 public:
+  /// The diagnostics IDs that are no longer available in `.def`
+  std::vector<std::string> unknownIDs;
   explicit YAMLLocalizationProducer(llvm::StringRef filePath);
   llvm::StringRef getMessageOr(swift::DiagID id,
                                llvm::StringRef defaultMessage) const override;
@@ -185,12 +189,23 @@ class LocalizationInput : public llvm::yaml::Input {
   template <typename T, typename Context>
   friend typename std::enable_if<llvm::yaml::has_SequenceTraits<T>::value,
                                  void>::type
-  readYAML(llvm::yaml::IO &io, T &Seq, bool, Context &Ctx);
+  readYAML(llvm::yaml::IO &io, T &Seq, T &unknownIDs, bool, Context &Ctx);
 
   template <typename T>
   friend typename std::enable_if<llvm::yaml::has_SequenceTraits<T>::value,
                                  LocalizationInput &>::type
   operator>>(LocalizationInput &yin, T &diagnostics);
+
+public:
+  /// A vector that keeps track of the diagnostics IDs that are available in
+  /// YAML and not available in `.def` files.
+  std::vector<std::string> unknownIDs;
+  
+  /// A diagnostic ID might be present in YAML and not in `.def` file, if that's
+  /// the case the `id` won't have a `DiagID` value.
+  /// If the `id` is available in `.def` file this method will return the `id`'s
+  /// value, otherwise this method won't return a value.
+  static llvm::Optional<uint32_t> readID(llvm::yaml::IO &io);
 };
 
 } // namespace diag

lib/AST/LocalizationFormat.cpp

@@ -15,6 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/AST/LocalizationFormat.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitstreamReader.h"
@@ -28,16 +29,13 @@
 #include <type_traits>
 
 namespace {
+
 enum LocalDiagID : uint32_t {
 #define DIAG(KIND, ID, Options, Text, Signature) ID,
 #include "swift/AST/DiagnosticsAll.def"
   NumDiags
 };
 
-struct DiagnosticNode {
-  uint32_t id;
-  std::string msg;
-};
 } // namespace
 
 namespace llvm {
@@ -55,15 +53,6 @@ template <> struct ScalarEnumerationTraits<LocalDiagID> {
   }
 };
 
-template <> struct MappingTraits<DiagnosticNode> {
-  static void mapping(IO &io, DiagnosticNode &node) {
-    LocalDiagID diagID;
-    io.mapRequired("id", diagID);
-    io.mapRequired("msg", node.msg);
-    node.id = static_cast<uint32_t>(diagID);
-  }
-};
-
 } // namespace yaml
 } // namespace llvm
 
@@ -121,6 +110,7 @@ YAMLLocalizationProducer::YAMLLocalizationProducer(llvm::StringRef filePath) {
   llvm::MemoryBuffer *document = FileBufOrErr->get();
   diag::LocalizationInput yin(document->getBuffer());
   yin >> diagnostics;
+  unknownIDs = std::move(yin.unknownIDs);
 }
 
 llvm::StringRef
@@ -143,31 +133,45 @@ void YAMLLocalizationProducer::forEachAvailable(
   }
 }
 
+llvm::Optional<uint32_t> LocalizationInput::readID(llvm::yaml::IO &io) {
+  LocalDiagID diagID;
+  io.mapRequired("id", diagID);
+  if (diagID == LocalDiagID::NumDiags)
+    return llvm::None;
+  return static_cast<uint32_t>(diagID);
+}
+
 template <typename T, typename Context>
 typename std::enable_if<llvm::yaml::has_SequenceTraits<T>::value, void>::type
-readYAML(llvm::yaml::IO &io, T &Seq, bool, Context &Ctx) {
+readYAML(llvm::yaml::IO &io, T &Seq, T &unknownIDs, bool, Context &Ctx) {
   unsigned count = io.beginSequence();
-  if (count)
+  if (count) {
     Seq.resize(LocalDiagID::NumDiags);
+  }
+
   for (unsigned i = 0; i < count; ++i) {
     void *SaveInfo;
     if (io.preflightElement(i, SaveInfo)) {
-      DiagnosticNode current;
-      yamlize(io, current, true, Ctx);
-      io.postflightElement(SaveInfo);
+      io.beginMapping();
 
-      // A diagnostic ID might be present in YAML and not in `.def` file,
-      // if that's the case ScalarEnumerationTraits will assign the diagnostic ID
-      // to `LocalDiagID::NumDiags`. Since the diagnostic ID isn't available
-      // in `.def` it shouldn't be stored in the diagnostics array.
-      if (current.id != LocalDiagID::NumDiags) {
+      // If the current diagnostic ID is available in YAML and in `.def`, add it
+      // to the diagnostics array. Otherwise, re-parse the current diagnnostic
+      // id as a string and store it in `unknownIDs` array.
+      if (auto id = LocalizationInput::readID(io)) {
         // YAML file isn't guaranteed to have diagnostics in order of their
         // declaration in `.def` files, to accommodate that we need to leave
         // holes in diagnostic array for diagnostics which haven't yet been
-        // localized and for the ones that have `DiagnosticNode::id`
-        // indicates their position.
-        Seq[static_cast<unsigned>(current.id)] = std::move(current.msg);
+        // localized and for the ones that have `id` indicates their position.
+        io.mapRequired("msg", Seq[*id]);
+      } else {
+        std::string unknownID, message;
+        // Read "raw" id since it doesn't exist in `.def` file.
+        io.mapRequired("id", unknownID);
+        io.mapRequired("msg", message);
+        unknownIDs.push_back(unknownID);
       }
+      io.endMapping();
+      io.postflightElement(SaveInfo);
     }
   }
   io.endSequence();
@@ -181,7 +185,7 @@ operator>>(LocalizationInput &yin, T &diagnostics) {
   if (yin.setCurrentDocument()) {
     // If YAML file's format doesn't match the current format in
     // DiagnosticMessageFormat, will throw an error.
-    readYAML(yin, diagnostics, true, Ctx);
+    readYAML(yin, diagnostics, yin.unknownIDs, true, Ctx);
   }
   return yin;
 }

test/diagnostics/Localization/Inputs/en.yaml

@@ -22,12 +22,24 @@
 
 - id: "lex_unterminated_string"
   msg: "unterminated string literal"
-  
+
+- id: "not_available_in_def_2"
+  msg: "Shouldn't be produced"
+
 - id: "var_init_self_referential"
   msg: "variable used within its own initial value"
 
 - id: "cannot_find_in_scope"
   msg: "cannot %select{find|find operator}1 %0 in scope"
 
+- id: "not_available_in_def_3"
+  msg: "Shouldn't be produced"
+
 - id: "warning_invalid_locale_code"
   msg: "unsupported locale code; supported locale codes are '%0'"
+
+- id: "not_available_in_def_4"
+  msg: "Shouldn't be produced"
+
+- id: "not_available_in_def_5"
+  msg: "Shouldn't be produced"

test/diagnostics/Localization/fr_localization.swift

@@ -1,8 +1,9 @@
 // RUN: %empty-directory(%t)
 // RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/fr.yaml --output-directory=%t/
-// RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/en.yaml --output-directory=%t/
+// RUN: swift-serialize-diagnostics --input-file-path=%S/Inputs/en.yaml --output-directory=%t/ 2>&1 | %FileCheck %s
 // RUN: %target-typecheck-verify-swift -localization-path %t -locale fr
 
+// CHECK: These diagnostic IDs are no longer availiable: 'not_available_in_def, not_available_in_def_2, not_available_in_def_3, not_available_in_def_4, not_available_in_def_5'
 _ = "HI!
 // expected-error@-1{{chaîne non terminée littérale}}
 var self1 = self1 // expected-error {{variable utilisée dans sa propre valeur initiale}}

tools/swift-serialize-diagnostics/swift-serialize-diagnostics.cpp

@@ -16,6 +16,7 @@
 
 #include "swift/AST/LocalizationFormat.h"
 #include "swift/Basic/LLVMInitialize.h"
+#include "swift/Basic/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -82,5 +83,15 @@ int main(int argc, char *argv[]) {
     return EXIT_FAILURE;
   }
 
+  // Print out the diagnostics IDs that are available in YAML but not available
+  // in `.def`
+  if (!yaml.unknownIDs.empty()) {
+    llvm::errs() << "These diagnostic IDs are no longer availiable: '";
+    llvm::interleave(
+        yaml.unknownIDs, [&](std::string id) { llvm::errs() << id; },
+        [&] { llvm::errs() << ", "; });
+    llvm::errs() << "'\n";
+  }
+
   return EXIT_SUCCESS;
 }