diff --git a/src/qmlcompiler/qcoloroutput.cpp b/src/qmlcompiler/qcoloroutput.cpp index 8e0d57faf5..01de4eab23 100644 --- a/src/qmlcompiler/qcoloroutput.cpp +++ b/src/qmlcompiler/qcoloroutput.cpp @@ -25,8 +25,7 @@ public: inline void write(const QString &msg) { - const QByteArray encodedMsg = msg.toLocal8Bit(); - fwrite(encodedMsg.constData(), size_t(1), size_t(encodedMsg.size()), stderr); + m_buffer.append(msg.toLocal8Bit()); } static QString escapeCode(const QString &in) @@ -51,8 +50,19 @@ public: bool coloringEnabled() const { return m_coloringEnabled; } + void flushBuffer() + { + fwrite(m_buffer.constData(), size_t(1), size_t(m_buffer.size()), stderr); + m_buffer.clear(); + } + + void discardBuffer() + { + m_buffer.clear(); + } + private: - QFile m_out; + QByteArray m_buffer; QColorOutput::ColorMapping m_colorMapping; int m_currentColorID = -1; bool m_coloringEnabled = false; @@ -313,6 +323,16 @@ QString QColorOutput::colorify(const QStringView message, int colorID) const return message.toString(); } +void QColorOutput::flushBuffer() +{ + d->flushBuffer(); +} + +void QColorOutput::discardBuffer() +{ + d->discardBuffer(); +} + /*! \internal Adds a color mapping from \a colorID to \a colorCode, for this QColorOutput instance. diff --git a/src/qmlcompiler/qcoloroutput_p.h b/src/qmlcompiler/qcoloroutput_p.h index af80e13f7e..1afc96d1f6 100644 --- a/src/qmlcompiler/qcoloroutput_p.h +++ b/src/qmlcompiler/qcoloroutput_p.h @@ -84,6 +84,9 @@ public: const QString &prefix = QString()); QString colorify(QStringView message, int color = -1) const; + void flushBuffer(); + void discardBuffer(); + private: QScopedPointer d; Q_DISABLE_COPY_MOVE(QColorOutput) diff --git a/src/qmlcompiler/qqmljslinter.cpp b/src/qmlcompiler/qqmljslinter.cpp index d85c9fa8c4..cd250c0dc5 100644 --- a/src/qmlcompiler/qqmljslinter.cpp +++ b/src/qmlcompiler/qqmljslinter.cpp @@ -424,12 +424,8 @@ static void addJsonWarning(QJsonArray &warnings, const QQmlJS::DiagnosticMessage void QQmlJSLinter::processMessages(QJsonArray &warnings) { - for (const auto &error : m_logger->errors()) - addJsonWarning(warnings, error, error.id, error.fixSuggestion); - for (const auto &warning : m_logger->warnings()) - addJsonWarning(warnings, warning, warning.id, warning.fixSuggestion); - for (const auto &info : m_logger->infos()) - addJsonWarning(warnings, info, info.id, info.fixSuggestion); + for (const Message &message : m_logger->messages()) + addJsonWarning(warnings, message, message.id, message.fixSuggestion); } QQmlJSLinter::LintResult QQmlJSLinter::lintFile(const QString &filename, @@ -837,21 +833,20 @@ QQmlJSLinter::FixResult QQmlJSLinter::applyFixes(QString *fixedCode, bool silent if (isESModule || isJavaScript) return NothingToFix; - for (const auto &messages : { m_logger->infos(), m_logger->warnings(), m_logger->errors() }) - for (const Message &msg : messages) { - if (!msg.fixSuggestion.has_value() || !msg.fixSuggestion->isAutoApplicable()) - continue; + for (const Message &msg : m_logger->messages()) { + if (!msg.fixSuggestion.has_value() || !msg.fixSuggestion->isAutoApplicable()) + continue; - // Ignore fix suggestions for other files - const QString filename = msg.fixSuggestion->filename(); - if (!filename.isEmpty() - && QFileInfo(filename).absoluteFilePath() != currentFileAbsolutePath) { - continue; - } - - fixesToApply << msg.fixSuggestion.value(); + // Ignore fix suggestions for other files + const QString filename = msg.fixSuggestion->filename(); + if (!filename.isEmpty() + && QFileInfo(filename).absoluteFilePath() != currentFileAbsolutePath) { + continue; } + fixesToApply << msg.fixSuggestion.value(); + } + if (fixesToApply.isEmpty()) return NothingToFix; diff --git a/src/qmlcompiler/qqmljslogger.cpp b/src/qmlcompiler/qqmljslogger.cpp index 1b6e2296b9..18bf1de15f 100644 --- a/src/qmlcompiler/qqmljslogger.cpp +++ b/src/qmlcompiler/qqmljslogger.cpp @@ -279,11 +279,11 @@ void QQmlJSLogger::log(const QString &message, QQmlJS::LoggerWarningId id, diagMsg.type = type; diagMsg.fixSuggestion = suggestion; - switch (type) { - case QtWarningMsg: m_warnings.push_back(diagMsg); break; - case QtCriticalMsg: m_errors.push_back(diagMsg); break; - case QtInfoMsg: m_infos.push_back(diagMsg); break; - default: break; + if (m_inTransaction) { + m_pendingMessages.push_back(std::move(diagMsg)); + } else { + countMessage(diagMsg); + m_committedMessages.push_back(std::move(diagMsg)); } if (srcLocation.length > 0 && !m_code.isEmpty() && showContext) @@ -291,6 +291,23 @@ void QQmlJSLogger::log(const QString &message, QQmlJS::LoggerWarningId id, if (suggestion.has_value()) printFix(suggestion.value()); + + if (!m_inTransaction) + m_output.flushBuffer(); +} + +void QQmlJSLogger::countMessage(const Message &message) +{ + switch (message.type) { + case QtWarningMsg: + ++m_numWarnings; + break; + case QtCriticalMsg: + ++m_numErrors; + break; + default: + break; + } } void QQmlJSLogger::processMessages(const QList &messages, @@ -310,6 +327,52 @@ void QQmlJSLogger::processMessages(const QList &messa m_output.write(QStringLiteral("---\n\n")); } +/*! + \internal + Starts a transaction for a compile pass. This buffers all messages until the + transaction completes. If you commit the transaction, the messages are printed + and added to the list of committed messages. If you roll it back, the logger + reverts to the state before the start of the transaction. + + This is useful for compile passes that potentially have to be repeated, such + as the type propagator. We don't want to see the same messages logged multiple + times. + */ +void QQmlJSLogger::startTransaction() +{ + Q_ASSERT(!m_inTransaction); + m_inTransaction = true; +} + +/*! + \internal + Commit the current transaction. Print all pending messages, and add them to + the list of committed messages. Then, clear the transaction flag. + */ +void QQmlJSLogger::commit() +{ + Q_ASSERT(m_inTransaction); + for (const Message &message : std::as_const(m_pendingMessages)) + countMessage(message); + + m_committedMessages.append(std::exchange(m_pendingMessages, {})); + m_output.flushBuffer(); + m_inTransaction = false; +} + +/*! + \internal + Roll back the current transaction and revert the logger to the state before + it was started. + */ +void QQmlJSLogger::rollback() +{ + Q_ASSERT(m_inTransaction); + m_pendingMessages.clear(); + m_output.discardBuffer(); + m_inTransaction = false; +} + void QQmlJSLogger::printContext(const QString &overrideFileName, const QQmlJS::SourceLocation &location) { diff --git a/src/qmlcompiler/qqmljslogger_p.h b/src/qmlcompiler/qqmljslogger_p.h index c043a8b342..8c994f9558 100644 --- a/src/qmlcompiler/qqmljslogger_p.h +++ b/src/qmlcompiler/qqmljslogger_p.h @@ -123,12 +123,13 @@ public: QQmlJSLogger(); ~QQmlJSLogger() = default; - bool hasWarnings() const { return !m_warnings.isEmpty(); } - bool hasErrors() const { return !m_errors.isEmpty(); } + bool hasWarnings() const { return m_numWarnings > 0; } + bool hasErrors() const { return m_numErrors > 0; } - const QList &infos() const { return m_infos; } - const QList &warnings() const { return m_warnings; } - const QList &errors() const { return m_errors; } + qsizetype numWarnings() const { return m_numWarnings; } + qsizetype numErrors() const { return m_numErrors; } + + const QList &messages() const { return m_committedMessages; } QtMsgType categoryLevel(QQmlJS::LoggerWarningId id) const { @@ -199,6 +200,10 @@ public: void setFilePath(const QString &filePath) { m_filePath = filePath; } QString filePath() const { return m_filePath; } + void startTransaction(); + void commit(); + void rollback(); + private: QMap m_categories; @@ -210,6 +215,8 @@ private: bool showFileName, const std::optional &suggestion, const QString overrideFileName); + void countMessage(const Message &message); + QString m_filePath; QString m_code; @@ -224,11 +231,14 @@ private: QHash m_categoryChanged; - QList m_infos; - QList m_warnings; - QList m_errors; + QList m_pendingMessages; + QList m_committedMessages; QHash> m_ignoredWarnings; + qsizetype m_numWarnings = 0; + qsizetype m_numErrors = 0; + bool m_inTransaction = false; + // the compiler needs private log() function at the moment friend class QQmlJSAotCompiler; }; diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index bcf1e4f0a1..4ed8a637c1 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -52,8 +52,12 @@ QQmlJSCompilePass::BlocksAndAnnotations QQmlJSTypePropagator::run(const Function do { // Reset the error if we need to do another pass - if (m_state.needsMorePasses) + if (m_state.needsMorePasses) { m_errors->clear(); + m_logger->rollback(); + } + + m_logger->startTransaction(); m_prevStateAnnotations = m_state.annotations; m_state = PassState(); @@ -68,6 +72,7 @@ QQmlJSCompilePass::BlocksAndAnnotations QQmlJSTypePropagator::run(const Function // This means that we won't start over for the same reason again. } while (m_state.needsMorePasses); + m_logger->commit(); return { std::move(m_basicBlocks), std::move(m_state.annotations) }; } diff --git a/src/qmlls/qqmllintsuggestions.cpp b/src/qmlls/qqmllintsuggestions.cpp index 10a16c1f4e..b46df5f686 100644 --- a/src/qmlls/qqmllintsuggestions.cpp +++ b/src/qmlls/qqmllintsuggestions.cpp @@ -369,20 +369,18 @@ void QmlLintSuggestions::diagnoseHelper(const QByteArray &url, if (const QQmlJSLogger *logger = linter.logger()) { qsizetype nDiagnostics = diagnostics.size(); - for (const auto &messages : { logger->infos(), logger->warnings(), logger->errors() }) { - for (const Message &message : messages) { - if (!message.message.contains(u"Failed to import")) { - diagnostics.append(messageToDiagnostic(message)); - continue; - } - - Message modified {message}; - modified.message.append( - u" Did you build your project? If yes, did you set the " - u"\"QT_QML_GENERATE_QMLLS_INI\" CMake variable on your project to \"ON\"?"); - - diagnostics.append(messageToDiagnostic(modified)); + for (const Message &message : logger->messages()) { + if (!message.message.contains(u"Failed to import")) { + diagnostics.append(messageToDiagnostic(message)); + continue; } + + Message modified {message}; + modified.message.append( + u" Did you build your project? If yes, did you set the " + u"\"QT_QML_GENERATE_QMLLS_INI\" CMake variable on your project to \"ON\"?"); + + diagnostics.append(messageToDiagnostic(modified)); } if (diagnostics.size() != nDiagnostics && imports.size() == 1) diagnostics.append(createMissingBuildDirDiagnostic()); diff --git a/tests/auto/qml/qmllint/data/multiplePasses.qml b/tests/auto/qml/qmllint/data/multiplePasses.qml new file mode 100644 index 0000000000..691354c8d9 --- /dev/null +++ b/tests/auto/qml/qmllint/data/multiplePasses.qml @@ -0,0 +1,17 @@ +import QtQuick + +Item { + property int u: 50 + Item { + function wait(timeout) { + if (timeout === undefined) + timeout = 5000 + + var i = 0; + while (i < timeout) + i += u // unqualified, produces warning, but only once + + return i + } + } +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index afa22b4e64..c242f836eb 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -1228,6 +1228,12 @@ expression: \${expr} \${expr} \\\${expr} \\\${expr}`)", Message { QStringLiteral( "Type QQC2.Label is used but it is not resolved") } }, }; + + // The warning should show up only once even though + // we have to run the type propagator multiple times. + QTest::newRow("multiplePasses") + << testFile("multiplePasses.qml") + << Result {{ Message { QStringLiteral("Unqualified access") }}}; } void TestQmllint::dirtyQmlCode() diff --git a/tools/qmllint/main.cpp b/tools/qmllint/main.cpp index c8a0518242..32091b8fa3 100644 --- a/tools/qmllint/main.cpp +++ b/tools/qmllint/main.cpp @@ -430,11 +430,11 @@ All warnings can be set to three levels: qmldirFiles, resourceFiles, categories); } success &= (lintResult == QQmlJSLinter::LintSuccess || lintResult == QQmlJSLinter::HasWarnings); - if (success) - { - int value = parser.isSet(maxWarnings) ? parser.value(maxWarnings).toInt() - : settings.value(maxWarningsSetting).toInt(); - if (value != -1 && value < linter.logger()->warnings().size()) + if (success) { + const qsizetype value = parser.isSet(maxWarnings) + ? parser.value(maxWarnings).toInt() + : settings.value(maxWarningsSetting).toInt(); + if (value != -1 && value < linter.logger()->numWarnings()) success = false; }