QmlCompiler: Introduce transactions for the logger

Certain compile passes may be run multiple times and only the last run
counts. We need to be able to roll back the logger to the state before
the pass in that case.

Amends commit d70abd83dc

Pick-to: 6.9
Task-number: QTBUG-124913
Change-Id: Ie6174fc3d6ce60ca3f0a3fa27484a58e6febcc17
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ulf Hermann 2025-01-20 14:02:11 +01:00
parent 87866c09b2
commit d3492ff7c1
10 changed files with 170 additions and 53 deletions

View File

@ -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.

View File

@ -84,6 +84,9 @@ public:
const QString &prefix = QString());
QString colorify(QStringView message, int color = -1) const;
void flushBuffer();
void discardBuffer();
private:
QScopedPointer<QColorOutputPrivate> d;
Q_DISABLE_COPY_MOVE(QColorOutput)

View File

@ -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;

View File

@ -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<QQmlJS::DiagnosticMessage> &messages,
@ -310,6 +327,52 @@ void QQmlJSLogger::processMessages(const QList<QQmlJS::DiagnosticMessage> &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)
{

View File

@ -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<Message> &infos() const { return m_infos; }
const QList<Message> &warnings() const { return m_warnings; }
const QList<Message> &errors() const { return m_errors; }
qsizetype numWarnings() const { return m_numWarnings; }
qsizetype numErrors() const { return m_numErrors; }
const QList<Message> &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<QString, QQmlJS::LoggerCategory> m_categories;
@ -210,6 +215,8 @@ private:
bool showFileName, const std::optional<QQmlJSFixSuggestion> &suggestion,
const QString overrideFileName);
void countMessage(const Message &message);
QString m_filePath;
QString m_code;
@ -224,11 +231,14 @@ private:
QHash<QString, bool> m_categoryChanged;
QList<Message> m_infos;
QList<Message> m_warnings;
QList<Message> m_errors;
QList<Message> m_pendingMessages;
QList<Message> m_committedMessages;
QHash<uint32_t, QSet<QString>> 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;
};

View File

@ -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) };
}

View File

@ -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());

View File

@ -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
}
}
}

View File

@ -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()

View File

@ -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;
}