feat: check ID uniqueness and ID references

CMakeLists.txt

@@ -68,6 +68,8 @@ set(MX_CORE_RUNTIME_SOURCES
     ${PRIVATE_DIR}/mx/core/Decimal.cpp
     ${PRIVATE_DIR}/mx/core/Decimal.h
     ${PRIVATE_DIR}/mx/core/Error.h
+    ${PRIVATE_DIR}/mx/core/IdIntegrity.cpp
+    ${PRIVATE_DIR}/mx/core/IdIntegrity.h
     ${PRIVATE_DIR}/mx/core/Lexical.cpp
     ${PRIVATE_DIR}/mx/core/Lexical.h
     ${PRIVATE_DIR}/mx/core/NameToken.cpp

src/include/mx/api/Diagnostics.h

@@ -30,7 +30,9 @@ enum class DiagnosticCode
     unmatchedSpanner,      // a spanner endpoint had no matching endpoint
     invalidValue,          // a value could not be read, so a default was used
     missingValueDefaulted, // a required value was missing, so a default was used
-    droppedData            // data could not be read or written, so it was left out
+    droppedData,           // data could not be read or written, so it was left out
+    duplicateId,           // two elements claimed one ID, which must be unique
+    danglingIdReference    // an ID reference did not name the ID it should have
 };
 
 // A non-fatal problem noticed while producing a score or MusicXML document.

src/include/mx/api/MusicXml.h

@@ -55,16 +55,28 @@ class MusicXml
     // are represented by an error result.
     Result<void> writeToFile(const std::string &filePath) const;
 
+    // Writes the document to a file and reports what had to be adjusted to
+    // keep the output valid, such as an id claimed by two elements.
+    Result<void> writeToFile(const std::string &filePath, Diagnostics &diagnostics) const;
+
     // Writes the document to a character stream.
     Result<void> writeToStream(std::ostream &stream) const;
 
-    // TODO: document ID validity loophole
-    //
+    // Writes the document to a character stream and reports what had to be
+    // adjusted to keep the output valid.
+    Result<void> writeToStream(std::ostream &stream, Diagnostics &diagnostics) const;
+
     // This is an escape hatch in case mx::api does not do what you need and
     // you want to edit the core DOM directly. You will need to include the
     // private mx::core headers in your header search paths to do so. Not
     // recommended, try opening an issue first!
     //
+    // An id you set here follows the same rules as one that was parsed. The
+    // core model can keep an id a legal name, but it cannot keep it unique,
+    // so that is checked when the document is written. A duplicate is
+    // renamed and reported. A reference that does not name an id, or that
+    // names the wrong kind of element, is reported and left as it is.
+    //
     // The reference is only good for as long as this MusicXml is alive and
     // you have not moved it away: do not keep it past a std::move of this
     // object into another MusicXml or into intoScore, which destroys the

src/private/mx/api/MusicXml.cpp

@@ -5,6 +5,7 @@
 #include "mx/api/MusicXml.h"
 #include "mx/core/Attribution.h"
 #include "mx/core/Error.h"
+#include "mx/core/IdIntegrity.h"
 #include "mx/core/ParseContext.h"
 #include "mx/core/generated/Document.h"
 #include "mx/impl/ScoreConversions.h"
@@ -86,19 +87,28 @@ DiagnosticCode mirrorToApiDiagnosticCode(core::DiagnosticCode code)
         return DiagnosticCode::valueAdjusted;
     case core::DiagnosticCode::missingValueDefaulted:
         return DiagnosticCode::missingValueDefaulted;
+    case core::DiagnosticCode::duplicateId:
+        return DiagnosticCode::duplicateId;
+    case core::DiagnosticCode::danglingIdReference:
+        return DiagnosticCode::danglingIdReference;
     }
     return DiagnosticCode::invalidValue;
 }
 
-// Every import repair leaves a usable document, so each one is a warning.
-core::ParseContext parseContextReportingTo(Diagnostics &diagnostics)
+// Every repair leaves a usable document, so each one is a warning.
+core::DiagnosticHandler handlerReportingTo(Diagnostics &diagnostics)
 {
-    return core::ParseContext{[&diagnostics](const core::Diagnostic &diagnostic) {
+    return [&diagnostics](const core::Diagnostic &diagnostic) {
         Location location;
         location.xmlPath = diagnostic.path;
         diagnostics.add(Diagnostic{Severity::warning, mirrorToApiDiagnosticCode(diagnostic.code), std::move(location),
                                    diagnostic.message});
-    }};
+    };
+}
+
+core::ParseContext parseContextReportingTo(Diagnostics &diagnostics)
+{
+    return core::ParseContext{handlerReportingTo(diagnostics)};
 }
 
 // Builds the error for a caught exception. Call it inside a catch block only:
@@ -209,6 +219,10 @@ Result<MusicXml> MusicXml::fromFile(const std::string &filePath, Diagnostics &di
             return mirrorToApiError(parsed.error());
         }
 
+        // After the parse, so that a repaired id is reported before any
+        // collision the repair caused.
+        core::checkIds(xdoc, handlerReportingTo(diagnostics));
+
         return MusicXml{core::Document{std::move(parsed).value()}, true};
     }
     catch (const std::bad_alloc &)
@@ -250,6 +264,10 @@ Result<MusicXml> MusicXml::fromStream(std::istream &stream, Diagnostics &diagnos
             return mirrorToApiError(parsed.error());
         }
 
+        // After the parse, so that a repaired id is reported before any
+        // collision the repair caused.
+        core::checkIds(xdoc, handlerReportingTo(diagnostics));
+
         return MusicXml{core::Document{std::move(parsed).value()}, true};
     }
     catch (const std::bad_alloc &)
@@ -267,6 +285,12 @@ Result<MusicXml> MusicXml::fromStream(std::istream &stream, Diagnostics &diagnos
 }
 
 Result<void> MusicXml::writeToFile(const std::string &filePath) const
+{
+    Diagnostics diagnostics;
+    return writeToFile(filePath, diagnostics);
+}
+
+Result<void> MusicXml::writeToFile(const std::string &filePath, Diagnostics &diagnostics) const
 {
     try
     {
@@ -280,6 +304,7 @@ Result<void> MusicXml::writeToFile(const std::string &filePath) const
         {
             core::serialize(toWrite, xdoc);
         }
+        core::repairIds(xdoc, handlerReportingTo(diagnostics));
         if (!xdoc.save_file(filePath.c_str(), "  "))
         {
             return ApiError{ResultCode::ioError, Location{}, "writeToFile: could not write '" + filePath + "'"};
@@ -301,6 +326,12 @@ Result<void> MusicXml::writeToFile(const std::string &filePath) const
 }
 
 Result<void> MusicXml::writeToStream(std::ostream &stream) const
+{
+    Diagnostics diagnostics;
+    return writeToStream(stream, diagnostics);
+}
+
+Result<void> MusicXml::writeToStream(std::ostream &stream, Diagnostics &diagnostics) const
 {
     try
     {
@@ -314,6 +345,7 @@ Result<void> MusicXml::writeToStream(std::ostream &stream) const
         {
             core::serialize(toWrite, xdoc);
         }
+        core::repairIds(xdoc, handlerReportingTo(diagnostics));
         xdoc.save(stream, "  ");
         return Result<void>{};
     }