Better YAML error output

Change-Id: I6558d78872a9982d6d60e00e80d88007fcd0394a
Reviewed-by: Dominik Holland <dominik.holland@qt.io>

6 files changed, 41 insertions, 16 deletions

diff --git a/src/common-lib/configcache.cpp b/src/common-lib/configcache.cpp
index c23bbccc..cfeb72a8 100644
--- a/src/common-lib/configcache.cpp
+++ b/src/common-lib/configcache.cpp
@@ -333,7 +333,7 @@ void AbstractConfigCache::parse(QStringList *warnings)
                 buffer.open(QIODevice::ReadOnly);
                 ce.content = loadFromSource(&buffer, ce.filePath);
             } catch (const Exception &e) {
-                throw Exception("Could not parse file '%1': %2.\n")
+                throw Exception("Could not parse file '%1': %2")
                         .arg(ce.filePath).arg(e.errorString());
             }
         };
diff --git a/src/common-lib/qtyaml.cpp b/src/common-lib/qtyaml.cpp
index 1a5aa041..47d973f7 100644
--- a/src/common-lib/qtyaml.cpp
+++ b/src/common-lib/qtyaml.cpp
@@ -497,7 +497,7 @@ void YamlParser::nextEvent()
 
     do {
         if (!yaml_parser_parse(&d->parser, &d->event))
-            throw YamlParserException(this, "cannot get next event");
+            throw YamlParserException(this, "invalid YAML syntax");
         if (d->event.type == YAML_ALIAS_EVENT)
             throw YamlParserException(this, "anchors and aliases are not supported");
     } while (d->event.type == YAML_NO_EVENT);
@@ -799,9 +799,31 @@ void YamlParser::parseFields(const std::vector<Field> &fields)
             allowedEvents.append(YAML_SEQUENCE_START_EVENT);
 
         if (!allowedEvents.contains(d->event.type)) { // ALIASES MISSING HERE!
-            //TODO: better output -- the integer here is confusing
-            throw YamlParserException(this, "Field '%1' expected to have one of these types [%2], but got %3")
-                            .arg(field->name).arg(allowedEvents).arg(d->event.type);
+            auto mapEventNames = [](const QVector<yaml_event_type_t> &events) -> QString {
+                static const QHash<yaml_event_type_t, const char *> eventNames = {
+                    { YAML_NO_EVENT,             "nothing" },
+                    { YAML_STREAM_START_EVENT,   "stream start" },
+                    { YAML_STREAM_END_EVENT,     "stream end" },
+                    { YAML_DOCUMENT_START_EVENT, "document start" },
+                    { YAML_DOCUMENT_END_EVENT,   "document end" },
+                    { YAML_ALIAS_EVENT,          "alias" },
+                    { YAML_SCALAR_EVENT,         "scalar" },
+                    { YAML_SEQUENCE_START_EVENT, "sequence start" },
+                    { YAML_SEQUENCE_END_EVENT,   "sequence end" },
+                    { YAML_MAPPING_START_EVENT,  "mapping start" },
+                    { YAML_MAPPING_END_EVENT,    "mapping end" }
+                };
+                QString names;
+                for (int i = 0; i < events.size(); ++i) {
+                    if (i)
+                        names.append(i == (events.size() - 1) ? qL1S(" or ") : qL1S(", "));
+                    names.append(qL1S(eventNames.value(events.at(i), "<unknown>")));
+                }
+                return names;
+            };
+
+            throw YamlParserException(this, "Field '%1' expected to be of type '%2', but got '%3'")
+                            .arg(field->name).arg(mapEventNames(allowedEvents)).arg(mapEventNames({ d->event.type }));
         }
 
         yaml_event_type_t typeBefore = d->event.type;
@@ -823,7 +845,7 @@ void YamlParser::parseFields(const std::vector<Field> &fields)
             fieldsMissing.append(qL1S(field.name));
     }
     if (!fieldsMissing.isEmpty())
-        throw YamlParserException(this, "Required field(s) '%1' are missing").arg(fieldsMissing);
+        throw YamlParserException(this, "Required fields are missing: %1").arg(fieldsMissing);
 }
 
 YamlParserException::YamlParserException(YamlParser *p, const char *errorString)
@@ -836,12 +858,15 @@ YamlParserException::YamlParserException(YamlParser *p, const char *errorString)
     int lpos = context.lastIndexOf(qL1C('\n'), int(mark.index ? mark.index - 1 : 0));
     int rpos = context.indexOf(qL1C('\n'), int(mark.index));
     context = context.mid(lpos + 1, rpos == -1 ? context.size() : rpos - lpos - 1);
+    int contextPos = int(mark.index) - (lpos + 1);
 
-    m_errorString.append(qSL(" at line %1, column %2 [.. %3 ..]").arg(mark.line + 1).arg(mark.column + 1).arg(context));
-    if (isProblem)
-        m_errorString.append(qSL(" (%1)").arg(QString::fromUtf8(p->d->parser.problem)));
+    m_errorString.append(qSL(":\nfile://%1:%2:%3: error").arg(p->sourcePath()).arg(mark.line + 1).arg(mark.column + 1));
     if (errorString)
         m_errorString.append(qSL(": %1").arg(qL1S(errorString)));
+    if (isProblem)
+        m_errorString.append(qSL(": %1").arg(QString::fromUtf8(p->d->parser.problem)));
+    if (!context.isEmpty())
+        m_errorString.append(qSL("\n %1\n %2^").arg(context, QString(contextPos, qL1C(' '))));
 }
 
 QT_END_NAMESPACE_AM
diff --git a/src/main-lib/configuration.cpp b/src/main-lib/configuration.cpp
index 362ec65b..26a58dd5 100644
--- a/src/main-lib/configuration.cpp
+++ b/src/main-lib/configuration.cpp
@@ -322,7 +322,7 @@ void Configuration::parseWithArguments(const QStringList &arguments, QStringList
             cache.parse(deploymentWarnings);
             m_data = cache.takeMergedResult();
         } catch (const Exception &e) {
-            showParserMessage(e.errorString(), ErrorMessage);
+            showParserMessage(e.errorString() + qL1C('\n'), ErrorMessage);
             exit(1);
         }
     }
diff --git a/tests/applicationinstaller/tst_applicationinstaller.cpp b/tests/applicationinstaller/tst_applicationinstaller.cpp
index 438b5929..89bd0224 100644
--- a/tests/applicationinstaller/tst_applicationinstaller.cpp
+++ b/tests/applicationinstaller/tst_applicationinstaller.cpp
@@ -410,7 +410,7 @@ void tst_PackageManager::packageInstallation_data()
             << false << false << false << false << nomd << "~package digest mismatch.*";
     QTest::newRow("invalid-info.yaml") \
             << "test-invalid-info.appkg" << ""
-            << false << false << false << false << nomd << "~.*YAML parse error.*did not find expected key.*";
+            << false << false << false << false << nomd << "~.*did not find expected key.*";
     QTest::newRow("invalid-info.yaml-id") \
             << "test-invalid-info-id.appkg" << ""
             << false << false << false << false << nomd << "~.*the identifier \\(:invalid\\) is not a valid package-id: must consist of printable ASCII characters only, except any of .*";
diff --git a/tests/error-checking.h b/tests/error-checking.h
index 19259b3b..09d3f61f 100644
--- a/tests/error-checking.h
+++ b/tests/error-checking.h
@@ -34,7 +34,7 @@
 // sadly this has to be a define for QVERIFY2() to work
 #define AM_CHECK_ERRORSTRING(_actual_errstr, _expected_errstr) do { \
     if (_expected_errstr.startsWith(QLatin1String("~"))) { \
-        QRegularExpression re(QStringLiteral("\\A") + _expected_errstr.mid(1) + QStringLiteral("\\z")); \
+        QRegularExpression re(_expected_errstr.mid(1)); \
         QVERIFY2(re.match(_actual_errstr).hasMatch(), \
                  qPrintable("\n    Got     : " + _actual_errstr.toLocal8Bit() + \
                             "\n    Expected: " + _expected_errstr.toLocal8Bit())); \
diff --git a/tests/packager-tool/tst_packager-tool.cpp b/tests/packager-tool/tst_packager-tool.cpp
index eaa31d30..c97a8317 100644
--- a/tests/packager-tool/tst_packager-tool.cpp
+++ b/tests/packager-tool/tst_packager-tool.cpp
@@ -279,10 +279,10 @@ void tst_PackagerTool::brokenMetadata_data()
     QTest::addColumn<QVariant>("yamlValue");
     QTest::addColumn<QString>("errorString");
 
-    QTest::newRow("missing-name")       << qSL("name")    << QVariant() << "~.*Required field\\(s\\) 'name' are missing.*";
-    QTest::newRow("missing-runtime")    << qSL("runtime") << QVariant() << "~.*Required field\\(s\\) 'runtime' are missing.*";
-    QTest::newRow("missing-identifier") << qSL("id")      << QVariant() << "~.*Required field\\(s\\) 'id' are missing.*";
-    QTest::newRow("missing-code")       << qSL("code")    << QVariant() << "~.*Required field\\(s\\) 'code' are missing.*";
+    QTest::newRow("missing-name")       << qSL("name")    << QVariant() << "~.*Required fields are missing: name.*";
+    QTest::newRow("missing-runtime")    << qSL("runtime") << QVariant() << "~.*Required fields are missing: runtime";
+    QTest::newRow("missing-identifier") << qSL("id")      << QVariant() << "~.*Required fields are missing: id";
+    QTest::newRow("missing-code")       << qSL("code")    << QVariant() << "~.*Required fields are missing: code";
 }
 
 void tst_PackagerTool::brokenMetadata()