Fix #317099: [MusicXML import] replace assertions by proper error rep…

…orting

The MusicXML parser contains several assertions checking start and end elements to verify the parser is still in sync.
For end users they have no effect (resulting in incomplete import without any error), for a debug build they crash
MuseScore when triggered.

These assertions have been replaced by a dialog providing meaningful information (expected element name and type versus actual)
to the end user and the developer.

Other Q_ASSERTS have been replaced by ASSERT_IF_FAILED macros.

src/importexport/musicxml/internal/musicxml/exportxml.cpp

@@ -120,6 +120,7 @@
 #include "engraving/iengravingconfiguration.h"
 
 #include "config.h"
+#include "log.h"
 
 using namespace mu::iex::musicxml;
 
@@ -5956,7 +5957,9 @@ static MeasureBase* lastMeasureBase(const System* const system)
     MeasureBase* mb = nullptr;
     if (system) {
         const auto& measures = system->measures();
-        Q_ASSERT(!(measures.empty()));
+        IF_ASSERT_FAILED(!(measures.empty())) {
+            return nullptr;
+        }
         mb = measures.back();
     }
     return mb;
@@ -5971,7 +5974,9 @@ static bool hasPageBreak(const System* const system)
     const MeasureBase* mb = nullptr;
     if (system) {
         const auto& measures = system->measures();
-        Q_ASSERT(!(measures.empty()));
+        IF_ASSERT_FAILED(!(measures.empty())) {
+            return false;
+        }
         mb = measures.back();
     }
 
@@ -6864,7 +6869,10 @@ void ExportMusicXml::writeMeasureStaves(const Measure* m,
     _tboxesBelowWritten = false;
 
     for (int staffIdx = startStaff; staffIdx < endStaff; ++staffIdx) {
-        Q_ASSERT(m == origM); // some staves may need to make m point somewhere else, so just in case, ensure start in same place
+        // some staves may need to make m point somewhere else, so just in case, ensure start in same place
+        IF_ASSERT_FAILED(m == origM) {
+            return;
+        }
         moveToTick(m->tick());
 
         int partRelStaffNo = (nstaves > 1 ? staffIdx - startStaff + 1 : 0); // xml staff number, counting from 1 for this instrument

src/importexport/musicxml/internal/musicxml/importmxml.cpp

@@ -20,12 +20,38 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+#include <QMessageBox>
+
 #include "importmxml.h"
 #include "importmxmllogger.h"
 #include "importmxmlpass1.h"
 #include "importmxmlpass2.h"
 
 namespace Ms {
+//---------------------------------------------------------
+//   musicXMLImportErrorDialog
+//---------------------------------------------------------
+
+/**
+ Show a dialog displaying the MusicXML import error(s).
+ */
+
+static int musicXMLImportErrorDialog(QString text, QString detailedText)
+{
+    QMessageBox errorDialog;
+    errorDialog.setIcon(QMessageBox::Question);
+    errorDialog.setText(text);
+    errorDialog.setInformativeText(QObject::tr("Do you want to try to load this file anyway?"));
+    errorDialog.setDetailedText(detailedText);
+    errorDialog.setStandardButtons(QMessageBox::Yes | QMessageBox::No);
+    errorDialog.setDefaultButton(QMessageBox::No);
+    return errorDialog.exec();
+}
+
+//---------------------------------------------------------
+//   importMusicXMLfromBuffer
+//---------------------------------------------------------
+
 Score::FileError importMusicXMLfromBuffer(Score* score, const QString& /*name*/, QIODevice* dev)
 {
     //qDebug("importMusicXMLfromBuffer(score %p, name '%s', dev %p)",
@@ -40,13 +66,26 @@ Score::FileError importMusicXMLfromBuffer(Score* score, const QString& /*name*/,
     dev->seek(0);
     MusicXMLParserPass1 pass1(score, &logger);
     Score::FileError res = pass1.parse(dev);
-    if (res != Score::FileError::FILE_NO_ERROR) {
-        return res;
-    }
+    const auto pass1_errors = pass1.errors();
 
     // pass 2
-    dev->seek(0);
     MusicXMLParserPass2 pass2(score, pass1, &logger);
-    return pass2.parse(dev);
+    if (res == Score::FileError::FILE_NO_ERROR) {
+        dev->seek(0);
+        res = pass2.parse(dev);
+    }
+
+    // report result
+    const auto pass2_errors = pass2.errors();
+    if (!(pass1_errors.isEmpty() && pass2_errors.isEmpty())) {
+        if (!MScore::noGui) {
+            const QString text { QObject::tr("Error(s) found, import may be incomplete.") };
+            if (musicXMLImportErrorDialog(text, pass1.errors() + pass2.errors()) != QMessageBox::Yes) {
+                res = Score::FileError::FILE_USER_ABORT;
+            }
+        }
+    }
+
+    return res;
 }
 } // namespace Ms

src/importexport/musicxml/internal/musicxml/importmxmlnoteduration.cpp

@@ -180,7 +180,6 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons
 
 void mxmlNoteDuration::duration(QXmlStreamReader& e)
 {
-    Q_ASSERT(e.isStartElement() && e.name() == "duration");
     _logger->logDebugTrace("MusicXMLParserPass1::duration", &e);
 
     _dura.set(0, 0);          // invalid unless set correctly
@@ -235,7 +234,6 @@ bool mxmlNoteDuration::readProperties(QXmlStreamReader& e)
 
 void mxmlNoteDuration::timeModification(QXmlStreamReader& e)
 {
-    Q_ASSERT(e.isStartElement() && e.name() == "time-modification");
     _logger->logDebugTrace("MusicXMLParserPass1::timeModification", &e);
 
     int intActual = 0;

src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp

@@ -38,8 +38,6 @@ namespace Ms {
 
 static Accidental* accidental(QXmlStreamReader& e, Score* score)
 {
-    Q_ASSERT(e.isStartElement() && e.name() == "accidental");
-
     bool cautionary = e.attributes().value("cautionary") == "yes";
     bool editorial = e.attributes().value("editorial") == "yes";
     bool parentheses = e.attributes().value("parentheses") == "yes";
@@ -70,9 +68,6 @@ static Accidental* accidental(QXmlStreamReader& e, Score* score)
 
 void mxmlNotePitch::displayStepOctave(QXmlStreamReader& e)
 {
-    Q_ASSERT(e.isStartElement()
-             && (e.name() == "rest" || e.name() == "unpitched"));
-
     while (e.readNextStartElement()) {
         if (e.name() == "display-step") {
             const auto step = e.readElementText();
@@ -108,8 +103,6 @@ void mxmlNotePitch::displayStepOctave(QXmlStreamReader& e)
 
 void mxmlNotePitch::pitch(QXmlStreamReader& e)
 {
-    Q_ASSERT(e.isStartElement() && e.name() == "pitch");
-
     // defaults
     _step = -1;
     _alter = 0;