qtattributionsscanner: Exit with error for qt_attribution.json warnings

So far qtattributionsscanner did print warnings for erroneous or
incomplete qt_attribution.json files, but still generated output and
returned normally. Let's be more strict, and actually fail in these
cases.

This also required moving the resolution of referenced copyright
and license files from the generation to the scanning phase.

Do be more lenient with Chromium attribution files though, because
we don't really control them.

With this in place, make docs for Qt modules should fail if incorrect
qt_attribution.json files are found.

Pick-to: 6.4
Fixes: QTBUG-107106
Change-Id: I3d0192dfb966b4422410d0b1fb4ba4652a5303cf
Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>

5 files changed, 147 insertions, 74 deletions

diff --git a/src/qtattributionsscanner/main.cpp b/src/qtattributionsscanner/main.cpp
index 6b9c909a9..79cb67f55 100644
--- a/src/qtattributionsscanner/main.cpp
+++ b/src/qtattributionsscanner/main.cpp
@@ -99,9 +99,16 @@ int main(int argc, char *argv[])
         if (logLevel == VerboseLog)
             std::cerr << qPrintable(tr("Recursively scanning %1 for attribution files...").arg(
                                         QDir::toNativeSeparators(path))) << std::endl;
-        packages = Scanner::scanDirectory(path, formats, logLevel);
+        std::optional<QList<Package>> p = Scanner::scanDirectory(path, formats, logLevel);
+        if (!p)
+            return 1;
+        packages = *p;
     } else if (pathInfo.isFile()) {
-        packages = Scanner::readFile(path, logLevel);
+        std::optional<QList<Package>> p = Scanner::readFile(path, logLevel);
+        if (!p)
+            return 1;
+        packages = *p;
+
     } else {
         std::cerr << qPrintable(tr("%1 is not a valid file or directory.").arg(
                                     QDir::toNativeSeparators(path))) << std::endl << std::endl;
diff --git a/src/qtattributionsscanner/package.h b/src/qtattributionsscanner/package.h
index 7a4db14c2..d226930eb 100644
--- a/src/qtattributionsscanner/package.h
+++ b/src/qtattributionsscanner/package.h
@@ -26,9 +26,11 @@ struct Package {
     QString license; // The license under which the package is distributed. Mandatory.
     QString licenseId; // see https://spdx.org/licenses/. Optional.
     QStringList licenseFiles; // path to files containing the license text. Optional.
+    QStringList licenseFilesContents;
 
     QString copyright; // Text with copyright owner. Optional.
     QString copyrightFile; // Path to file containing copyright owners. Optional.
+    QString copyrightFileContents;
 
     QString packageComment; // Further comments about the package. Optional.
 };
diff --git a/src/qtattributionsscanner/qdocgenerator.cpp b/src/qtattributionsscanner/qdocgenerator.cpp
index fbe1df06a..77d4e6674 100644
--- a/src/qtattributionsscanner/qdocgenerator.cpp
+++ b/src/qtattributionsscanner/qdocgenerator.cpp
@@ -50,8 +50,7 @@ static void sourceCode(QTextStream &out, const QString &src)
     out << "\n\\endcode\n\n";
 }
 
-static void generate(QTextStream &out, const Package &package, const QDir &baseDir,
-                     LogLevel logLevel)
+static void generate(QTextStream &out, const Package &package, const QDir &baseDir)
 {
     out << "/*!\n\n";
     for (const QString &part : package.qtParts) {
@@ -117,21 +116,8 @@ static void generate(QTextStream &out, const Package &package, const QDir &baseD
     QString copyright;
     if (!package.copyright.isEmpty())
         copyright = package.copyright;
-
-    if (!package.copyrightFile.isEmpty()) {
-        const QDir packageDir(package.path);
-        QFile file(QDir(package.path).absoluteFilePath(package.copyrightFile));
-        if (!file.open(QIODevice::ReadOnly)) {
-            if (logLevel != SilentLog) {
-                std::cerr << qPrintable(
-                        tr("Path %1 : cannot open copyright file %2.\n")
-                                .arg(QDir::toNativeSeparators(package.path))
-                                .arg(QDir::toNativeSeparators(package.copyrightFile)));
-            }
-        } else {
-            copyright = QString::fromUtf8(file.readAll());
-        }
-    }
+    else if (!package.copyrightFileContents.isEmpty())
+        copyright = package.copyrightFileContents;
 
     if (!copyright.isEmpty()) {
         out << "\n";
@@ -148,18 +134,9 @@ static void generate(QTextStream &out, const Package &package, const QDir &baseD
         out << package.license << ".\n\n";
     }
 
-    foreach (const QString &licenseFile, package.licenseFiles) {
-        QFile file(licenseFile);
-        if (!file.open(QIODevice::ReadOnly)) {
-            if (logLevel != SilentLog) {
-                std::cerr << qPrintable(tr("Path %1 : cannot open license file %2.\n")
-                                                .arg(QDir::toNativeSeparators(package.path))
-                                                .arg(QDir::toNativeSeparators(licenseFile)));
-                out << "*/\n";
-            }
-            return;
-        }
-        sourceCode(out, QString::fromUtf8(file.readAll()).trimmed());
+    foreach (const QString &license, package.licenseFilesContents) {
+        out << "*/\n";
+        sourceCode(out, license);
     }
     out << "*/\n";
 }
@@ -172,7 +149,7 @@ void generate(QTextStream &out, const QList<Package> &packages, const QString &b
 
     QDir baseDir(baseDirectory);
     for (const Package &package : packages)
-        generate(out, package, baseDir, logLevel);
+        generate(out, package, baseDir);
 }
 
 } // namespace QDocGenerator
diff --git a/src/qtattributionsscanner/scanner.cpp b/src/qtattributionsscanner/scanner.cpp
index 741b7ed64..4b2dbb767 100644
--- a/src/qtattributionsscanner/scanner.cpp
+++ b/src/qtattributionsscanner/scanner.cpp
@@ -13,7 +13,6 @@
 #include <QtCore/qvariant.h>
 
 #include <iostream>
-#include <optional>
 
 namespace Scanner {
 
@@ -23,45 +22,58 @@ static void missingPropertyWarning(const QString &filePath, const QString &prope
                                 QDir::toNativeSeparators(filePath), property)) << std::endl;
 }
 
-static void validatePackage(Package &p, const QString &filePath, LogLevel logLevel)
+static bool validatePackage(Package &p, const QString &filePath, LogLevel logLevel)
 {
+    bool validPackage = true;
+
     if (p.qtParts.isEmpty())
         p.qtParts << QStringLiteral("libs");
 
-    if (logLevel != SilentLog) {
-        if (p.name.isEmpty()) {
-            if (p.id.startsWith(QLatin1String("chromium-"))) // Ignore invalid README.chromium files
-                return;
+    if (p.name.isEmpty()) {
+        if (p.id.startsWith(QLatin1String("chromium-"))) // Ignore invalid README.chromium files
+            return false;
 
+        if (logLevel != SilentLog)
             missingPropertyWarning(filePath, QStringLiteral("Name"));
-        }
+        validPackage = false;
+    }
 
-        if (p.id.isEmpty())
+    if (p.id.isEmpty()) {
+        if (logLevel != SilentLog)
             missingPropertyWarning(filePath, QStringLiteral("Id"));
-        if (p.license.isEmpty())
+        validPackage = false;
+    }
+    if (p.license.isEmpty()) {
+        if (logLevel != SilentLog)
             missingPropertyWarning(filePath, QStringLiteral("License"));
+        validPackage = false;
+    }
 
-        if (!p.copyright.isEmpty() && !p.copyrightFile.isEmpty()) {
+    if (!p.copyright.isEmpty() && !p.copyrightFile.isEmpty()) {
+        if (logLevel != SilentLog) {
             std::cerr << qPrintable(tr("File %1: Properties 'Copyright' and 'CopyrightFile' are "
                                        "mutually exclusive.")
                                             .arg(QDir::toNativeSeparators(filePath)))
                       << std::endl;
+            validPackage = false;
         }
+    }
 
-        for (const QString &part : std::as_const(p.qtParts)) {
-            if (part != QLatin1String("examples")
-                    && part != QLatin1String("tests")
-                    && part != QLatin1String("tools")
-                    && part != QLatin1String("libs")
-                    && logLevel != SilentLog) {
+    for (const QString &part : std::as_const(p.qtParts)) {
+        if (part != QLatin1String("examples") && part != QLatin1String("tests")
+            && part != QLatin1String("tools") && part != QLatin1String("libs")) {
+
+            if (logLevel != SilentLog) {
                 std::cerr << qPrintable(tr("File %1: Property 'QtPart' contains unknown element "
                                            "'%2'. Valid entries are 'examples', 'tests', 'tools' "
                                            "and 'libs'.").arg(
                                             QDir::toNativeSeparators(filePath), part))
                           << std::endl;
             }
+            validPackage = false;
         }
     }
+    return validPackage;
 }
 
 static std::optional<QStringList> toStringList(const QJsonValue &value)
@@ -78,9 +90,11 @@ static std::optional<QStringList> toStringList(const QJsonValue &value)
 }
 
 // Transforms a JSON object into a Package object
-static Package readPackage(const QJsonObject &object, const QString &filePath, LogLevel logLevel)
+static std::optional<Package> readPackage(const QJsonObject &object, const QString &filePath,
+                                          LogLevel logLevel)
 {
     Package p;
+    bool validPackage = true;
     const QString directory = QFileInfo(filePath).absolutePath();
     p.path = directory;
 
@@ -92,6 +106,7 @@ static Package readPackage(const QJsonObject &object, const QString &filePath, L
             if (logLevel != SilentLog)
                 std::cerr << qPrintable(tr("File %1: Expected JSON string as value of %2.").arg(
                                             QDir::toNativeSeparators(filePath), key)) << std::endl;
+            validPackage = false;
             continue;
         }
         const QString value = iter.value().toString();
@@ -117,10 +132,14 @@ static Package readPackage(const QJsonObject &object, const QString &filePath, L
             p.licenseFiles = QStringList(QDir(directory).absoluteFilePath(value));
         } else if (key == QLatin1String("LicenseFiles")) {
             auto strings = toStringList(iter.value());
-            if (!strings && (logLevel != SilentLog))
-                std::cerr << qPrintable(tr("File %1: Expected JSON array of strings in %2.")
-                                                .arg(QDir::toNativeSeparators(filePath), key))
-                          << std::endl;
+            if (!strings) {
+                if (logLevel != SilentLog)
+                    std::cerr << qPrintable(tr("File %1: Expected JSON array of strings in %2.")
+                                                    .arg(QDir::toNativeSeparators(filePath), key))
+                              << std::endl;
+                validPackage = false;
+                continue;
+            }
             const QDir dir(directory);
             for (auto iter : strings.value())
                 p.licenseFiles.push_back(dir.absoluteFilePath(iter));
@@ -138,19 +157,51 @@ static Package readPackage(const QJsonObject &object, const QString &filePath, L
             p.qtUsage = value;
         } else if (key == QLatin1String("QtParts")) {
             auto parts = toStringList(iter.value());
-            if (!parts && (logLevel != SilentLog))
-                std::cerr << qPrintable(tr("File %1: Expected JSON array of strings in %2.")
-                                                .arg(QDir::toNativeSeparators(filePath), key))
-                          << std::endl;
+            if (!parts) {
+                if (logLevel != SilentLog) {
+                    std::cerr << qPrintable(tr("File %1: Expected JSON array of strings in %2.")
+                                                    .arg(QDir::toNativeSeparators(filePath), key))
+                              << std::endl;
+                }
+                validPackage = false;
+                continue;
+            }
             p.qtParts = parts.value();
         } else {
-            if (logLevel != SilentLog)
+            if (logLevel != SilentLog) {
                 std::cerr << qPrintable(tr("File %1: Unknown key %2.").arg(
                                             QDir::toNativeSeparators(filePath), key)) << std::endl;
+            }
+            validPackage = false;
         }
     }
 
-    validatePackage(p, filePath, logLevel);
+    if (!p.copyrightFile.isEmpty()) {
+        QFile file(p.copyrightFile);
+        if (!file.open(QIODevice::ReadOnly)) {
+            std::cerr << qPrintable(tr("File %1: Cannot open 'CopyrightFile' %2.\n")
+                                            .arg(QDir::toNativeSeparators(filePath))
+                                            .arg(QDir::toNativeSeparators(p.copyrightFile)));
+            validPackage = false;
+        }
+        p.copyrightFileContents = QString::fromUtf8(file.readAll());
+    }
+
+    foreach (const QString &licenseFile, p.licenseFiles) {
+        QFile file(licenseFile);
+        if (!file.open(QIODevice::ReadOnly)) {
+            if (logLevel != SilentLog) {
+                std::cerr << qPrintable(tr("File %1: Cannot open 'LicenseFile' %2.\n")
+                                                .arg(QDir::toNativeSeparators(filePath))
+                                                .arg(QDir::toNativeSeparators(licenseFile)));
+            }
+            validPackage = false;
+        }
+        p.licenseFilesContents << QString::fromUtf8(file.readAll()).trimmed();
+    }
+
+    if (!validatePackage(p, filePath, logLevel) || !validPackage)
+        return std::nullopt;
 
     return p;
 }
@@ -225,14 +276,16 @@ static Package parseChromiumFile(QFile &file, const QString &filePath, LogLevel
             p.licenseFiles = QStringList(entries.at(0).absoluteFilePath());
     }
 
-    validatePackage(p, filePath, logLevel);
+    // let's ignore warnings regarding Chromium files for now
+    Q_UNUSED(validatePackage(p, filePath, logLevel));
 
     return p;
 }
 
-QList<Package> readFile(const QString &filePath, LogLevel logLevel)
+std::optional<QList<Package>> readFile(const QString &filePath, LogLevel logLevel)
 {
     QList<Package> packages;
+    bool errorsFound = false;
 
     if (logLevel == VerboseLog) {
         std::cerr << qPrintable(tr("Reading file %1...").arg(
@@ -243,7 +296,7 @@ QList<Package> readFile(const QString &filePath, LogLevel logLevel)
         if (logLevel != SilentLog)
             std::cerr << qPrintable(tr("Could not open file %1.").arg(
                                         QDir::toNativeSeparators(file.fileName()))) << std::endl;
-        return QList<Package>();
+        return std::nullopt;
     }
 
     if (filePath.endsWith(QLatin1String(".json"))) {
@@ -255,47 +308,68 @@ QList<Package> readFile(const QString &filePath, LogLevel logLevel)
                                             QDir::toNativeSeparators(file.fileName()),
                                             jsonParseError.errorString()))
                           << std::endl;
-            return QList<Package>();
+            return std::nullopt;
         }
 
         if (document.isObject()) {
-            packages << readPackage(document.object(), file.fileName(), logLevel);
+            std::optional<Package> p = readPackage(document.object(), file.fileName(), logLevel);
+            if (p) {
+                packages << *p;
+            } else {
+                errorsFound = true;
+            }
         } else if (document.isArray()) {
             QJsonArray array = document.array();
             for (int i = 0, size = array.size(); i < size; ++i) {
                 QJsonValue value = array.at(i);
                 if (value.isObject()) {
-                    packages << readPackage(value.toObject(), file.fileName(), logLevel);
+                    std::optional<Package> p =
+                            readPackage(value.toObject(), file.fileName(), logLevel);
+                    if (p) {
+                        packages << *p;
+                    } else {
+                        errorsFound = true;
+                    }
                 } else {
-                    if (logLevel != SilentLog)
+                    if (logLevel != SilentLog) {
                         std::cerr << qPrintable(tr("File %1: Expecting JSON object in array.")
                                         .arg(QDir::toNativeSeparators(file.fileName())))
                                   << std::endl;
+                    }
+                    errorsFound = true;
                 }
             }
         } else {
-            if (logLevel != SilentLog)
+            if (logLevel != SilentLog) {
                 std::cerr << qPrintable(tr("File %1: Expecting JSON object in array.").arg(
                                             QDir::toNativeSeparators(file.fileName()))) << std::endl;
+            }
+            errorsFound = true;
         }
     } else if (filePath.endsWith(QLatin1String(".chromium"))) {
         Package chromiumPackage = parseChromiumFile(file, filePath, logLevel);
         if (!chromiumPackage.name.isEmpty()) // Skip invalid README.chromium files
             packages << chromiumPackage;
     } else {
-        if (logLevel != SilentLog)
+        if (logLevel != SilentLog) {
             std::cerr << qPrintable(tr("File %1: Unsupported file type.")
                             .arg(QDir::toNativeSeparators(file.fileName())))
                       << std::endl;
+        }
+        errorsFound = true;
     }
 
+    if (errorsFound)
+        return std::nullopt;
     return packages;
 }
 
-QList<Package> scanDirectory(const QString &directory, InputFormats inputFormats, LogLevel logLevel)
+std::optional<QList<Package>> scanDirectory(const QString &directory, InputFormats inputFormats,
+                                            LogLevel logLevel)
 {
     QDir dir(directory);
     QList<Package> packages;
+    bool errorsFound = false;
 
     QStringList nameFilters = QStringList();
     if (inputFormats & InputFormat::QtAttributions)
@@ -314,12 +388,23 @@ QList<Package> scanDirectory(const QString &directory, InputFormats inputFormats
     const QFileInfoList entries = dir.entryInfoList();
     for (const QFileInfo &info : entries) {
         if (info.isDir()) {
-            packages += scanDirectory(info.filePath(), inputFormats, logLevel);
+            std::optional<QList<Package>> ps =
+                    scanDirectory(info.filePath(), inputFormats, logLevel);
+            if (!ps)
+                errorsFound = true;
+            else
+                packages += *ps;
         } else {
-            packages += readFile(info.filePath(), logLevel);
+            std::optional p = readFile(info.filePath(), logLevel);
+            if (!p)
+                errorsFound = true;
+            else
+                packages += *p;
         }
     }
 
+    if (errorsFound)
+        return std::nullopt;
     return packages;
 }
 
diff --git a/src/qtattributionsscanner/scanner.h b/src/qtattributionsscanner/scanner.h
index 3120a924a..77edaf30c 100644
--- a/src/qtattributionsscanner/scanner.h
+++ b/src/qtattributionsscanner/scanner.h
@@ -10,6 +10,8 @@
 #include <QtCore/qstring.h>
 #include <QtCore/qlist.h>
 
+#include <optional>
+
 namespace Scanner {
 
 enum class InputFormat {
@@ -19,9 +21,9 @@ enum class InputFormat {
 Q_DECLARE_FLAGS(InputFormats, InputFormat)
 Q_DECLARE_OPERATORS_FOR_FLAGS(InputFormats)
 
-QList<Package> readFile(const QString &filePath, LogLevel logLevel);
-QList<Package> scanDirectory(const QString &directory, InputFormats inputFormats, LogLevel logLevel);
-
+std::optional<QList<Package>> readFile(const QString &filePath, LogLevel logLevel);
+std::optional<QList<Package>> scanDirectory(const QString &directory, InputFormats inputFormats,
+                                            LogLevel logLevel);
 }
 
 #endif // SCANNER_H