qmllint: add an "error" logging category

Add support for a critical error category in qmllint called "error",
so that the qds plugin can show errors for unsupported components for
example.

Add a test. The error category makes qmllint return -1.

Simplify a bit the logic of the "success" bool in lintFile() by making
it a LintResult and returning it.

Change-Id: Ide6a6a265f1aeede0fcbc35d9e4d0abe3f5c2aa3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/qmlcompiler/qqmljslinter.cpp

@@ -196,7 +196,7 @@ bool QQmlJSLinter::Plugin::parseMetaData(const QJsonObject &metaData, QString pl
         if (!QQmlJS::LoggingUtils::applyLevelToCategory(level, m_categories.last())) {
             qWarning() << "Invalid logging level" << level << "provided for"
                        << m_categories.last().id().name().toString()
-                       << "(allowed are: disable, info, warning) found in plugin metadata";
+                       << "(allowed are: disable, info, warning, error) found in plugin metadata.";
         }
     }
 
@@ -444,7 +444,7 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
     QJsonArray warnings;
     QJsonObject result;
 
-    bool success = true;
+    LintResult success = LintSuccess;
 
     QScopeGuard jsonOutput([&] {
         if (!json)
@@ -452,7 +452,7 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
 
         result[u"filename"_s] = QFileInfo(filename).absoluteFilePath();
         result[u"warnings"] = warnings;
-        result[u"success"] = success;
+        result[u"success"] = success == LintSuccess;
 
         json->append(result);
     });
@@ -469,11 +469,11 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
                                                             .arg(filename, file.errorString()),
                                                     QtCriticalMsg, QQmlJS::SourceLocation() },
                         qmlImport.name());
-                success = false;
             } else if (!silent) {
                 qWarning() << "Failed to open file" << filename << file.error();
             }
-            return FailedToOpen;
+            success = FailedToOpen;
+            return success;
         }
 
         code = QString::fromUtf8(file.readAll());
@@ -495,10 +495,9 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
     lexer.setCode(code, /*lineno = */ 1, /*qmlMode=*/!isJavaScript);
     QQmlJS::Parser parser(&engine);
 
-    success = isJavaScript ? (isESModule ? parser.parseModule() : parser.parseProgram())
+    if (!(isJavaScript ? (isESModule ? parser.parseModule() : parser.parseProgram())
-                           : parser.parse();
+                       : parser.parse())) {
-
+        success = FailedToParse;
-    if (!success) {
         const auto diagnosticMessages = parser.diagnosticMessages();
         for (const QQmlJS::DiagnosticMessage &m : diagnosticMessages) {
             if (json) {
@@ -511,10 +510,10 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
                                                 .arg(m.message);
             }
         }
-        return FailedToParse;
+        return success;
     }
 
-    if (success && !isJavaScript) {
+    if (!isJavaScript) {
         const auto check = [&](QQmlJSResourceFileMapper *mapper) {
             if (m_importer.importPaths() != qmlImportPaths)
                 m_importer.setImportPaths(qmlImportPaths);
@@ -594,13 +593,13 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
             }
             passMan->analyze(QQmlJSScope::createQQmlSAElement(v.result()));
 
-            success = !m_logger->hasWarnings() && !m_logger->hasErrors();
-
             if (m_logger->hasErrors()) {
+                success = HasErrors;
                 if (json)
                     processMessages(warnings);
                 return;
-            }
+            } else if (m_logger->hasWarnings())
+                success = HasWarnings;
 
             if (passMan) {
                 // passMan now has a pointer to the moved from type resolver
@@ -627,7 +626,10 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
                 m_logger->processMessages(globalWarnings, qmlImport);
             }
 
-            success &= !m_logger->hasWarnings() && !m_logger->hasErrors();
+            if (m_logger->hasErrors())
+                success = HasErrors;
+            else if (m_logger->hasWarnings())
+                success = HasWarnings;
 
             if (json)
                 processMessages(warnings);
@@ -641,7 +643,7 @@ QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename,
         }
     }
 
-    return success ? LintSuccess : HasWarnings;
+    return success;
 }
 
 QQmlJSLinter::LintResult QQmlJSLinter::lintModule(

src/qmlcompiler/qqmljslinter_p.h

@@ -44,7 +44,7 @@ public:
     QQmlJSLinter(const QStringList &importPaths, const QStringList &extraPluginPaths = {},
                  bool useAbsolutePath = false);
 
-    enum LintResult { FailedToOpen, FailedToParse, HasWarnings, LintSuccess };
+    enum LintResult { FailedToOpen, FailedToParse, HasWarnings, HasErrors, LintSuccess };
     enum FixResult { NothingToFix, FixError, FixSuccess };
 
     class Q_QMLCOMPILER_EXPORT Plugin

src/qmlcompiler/qqmljsloggingutils.cpp

@@ -139,7 +139,6 @@ namespace LoggingUtils {
 
 QString levelToString(const QQmlJS::LoggerCategory &category)
 {
-    Q_ASSERT(category.isIgnored() || category.level() != QtCriticalMsg);
     if (category.isIgnored())
         return QStringLiteral("disable");
 
@@ -148,6 +147,8 @@ QString levelToString(const QQmlJS::LoggerCategory &category)
         return QStringLiteral("info");
     case QtWarningMsg:
         return QStringLiteral("warning");
+    case QtCriticalMsg:
+        return QStringLiteral("error");
     default:
         Q_UNREACHABLE();
         break;
@@ -188,6 +189,10 @@ static QString levelValueForCategory(const LoggerCategory &category,
 
 bool applyLevelToCategory(const QStringView level, LoggerCategory &category)
 {
+    // you can't downgrade errors
+    if (category.level() == QtCriticalMsg && !category.isIgnored() && level != "error"_L1)
+        return false;
+
     if (level == "disable"_L1) {
         category.setLevel(QtCriticalMsg);
         category.setIgnored(true);
@@ -203,6 +208,12 @@ bool applyLevelToCategory(const QStringView level, LoggerCategory &category)
         category.setIgnored(false);
         return true;
     }
+    if (level == "error"_L1) {
+        category.setLevel(QtCriticalMsg);
+        category.setIgnored(false);
+        return true;
+    }
+
     return false;
 };
 
@@ -227,9 +238,9 @@ void updateLogLevels(QList<LoggerCategory> &categories,
         if (!applyLevelToCategory(value, category)) {
             qWarning() << "Invalid logging level" << value << "provided for"
                        << category.id().name().toString()
-                       << "(allowed are: disable, info, warning)";
+                       << "(allowed are: disable, info, warning, error)\n."
+                          "You can't change categories that have level \"error\" by default.";
             success = false;
-
         }
     }
     if (!success && parser)

tests/auto/qml/qmllint/data/HasUnqualified.qml

@@ -0,0 +1,5 @@
+import QtQuick
+
+Item {
+    property int myI: asdf
+}

tests/auto/qml/qmllint/testPlugin.json

@@ -27,6 +27,12 @@
             "settingsName": "TestDefaultValue3",
             "description": "A warning to test defaults",
             "defaultLevel": "warning"
+        },
+        {
+            "name": "TestDefaultValue4",
+            "settingsName": "TestDefaultValue3",
+            "description": "A warning to test defaults",
+            "defaultLevel": "error"
         }
     ]
 }

tests/auto/qml/qmllint/tst_qmllint.cpp

@@ -138,6 +138,7 @@ private Q_SLOTS:
 #endif
 
     void replayImportWarnings();
+    void errorCategory();
 
 private:
     enum DefaultImportOption { NoDefaultImports, UseDefaultImports };
@@ -2261,6 +2262,9 @@ void TestQmllint::hasTestPlugin()
             } else if (category.name() == u"testPlugin.TestDefaultValue3"){
                 QCOMPARE(category.level(), QtWarningMsg);
                 QCOMPARE(category.isIgnored(), false);
+            } else if (category.name() == u"testPlugin.TestDefaultValue4"){
+                QCOMPARE(category.level(), QtCriticalMsg);
+                QCOMPARE(category.isIgnored(), false);
             } else {
                 QFAIL("This category was not tested!");
             }
@@ -2651,5 +2655,19 @@ void TestQmllint::replayImportWarnings()
     searchWarnings(warnings, "Ambiguous type detected. T 1.0 is defined multiple times.");
 }
 
+void TestQmllint::errorCategory()
+{
+    {
+        const QString output = runQmllint(testFile(u"HasUnqualified.qml"_s), false,
+                                          QStringList{ u"--unqualified"_s, u"error"_s });
+        QVERIFY(output.startsWith("Error: "));
+    }
+    {
+        const QString output = runQmllint(testFile(u"HasUnqualified.qml"_s), true);
+        QVERIFY(output.startsWith("Warning: "));
+    }
+
+}
+
 QTEST_GUILESS_MAIN(TestQmllint)
 #include "tst_qmllint.moc"

tools/qmllint/main.cpp

@@ -77,6 +77,7 @@ All warnings can be set to three levels:
     disable - Fully disables the warning.
     info - Displays the warning but does not influence the return code.
     warning - Displays the warning and leads to a non-zero exit code if encountered.
+    error - Displays the warning as error and leads to a non-zero exit code if encountered.
 )"));
 
     parser.addHelpOption();