From 00df54b0be63759add6d0bb10925459af87bf25c Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 19 Jun 2025 12:24:54 +0200 Subject: [PATCH] QmlDesigner: Notify error for alias cycle When a cyclic dependency is introduced through QML aliases, the code model's synchronization mechanism is disrupted. This interruption prevents the model from updating type information consistently across components, resulting in stale or incorrect type data. Such inconsistencies can lead to unpredictable behavior or runtime errors in the application. This commit documents the issue and highlights the need for user intervention to manually resolve the cycle. Change-Id: I3c8283b0bbeba0634463e783ee18d5c99d8c919a Reviewed-by: Thomas Hartmann --- .../projectstorage/projectstorage.cpp | 18 ++++++++++++++++-- .../projectstorageerrornotifierinterface.h | 4 ++++ .../project/projectstorageerrornotifier.cpp | 14 ++++++++++++++ .../project/projectstorageerrornotifier.h | 3 +++ .../mocks/projectstorageerrornotifiermock.h | 6 ++++++ .../projectstorage/projectstorage-test.cpp | 19 +++++++++++++++++-- 6 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorage.cpp b/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorage.cpp index b66d15c5676..66d6c243e36 100644 --- a/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorage.cpp +++ b/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorage.cpp @@ -786,6 +786,12 @@ struct ProjectStorage::Statements "FROM propertyDeclarations " "WHERE propertyDeclarationId=?1 LIMIT 1", database}; + mutable Sqlite::ReadStatement<2, 1> selectPropertyDeclarationNameAndTypeIdForPropertyDeclarationIdStatement{ + "SELECT name, typeId " + "FROM propertyDeclarations " + "WHERE propertyDeclarationId=?1 " + "LIMIT 1", + database}; mutable Sqlite::ReadStatement<1, 1> selectSignalDeclarationNamesForTypeStatement{ "WITH RECURSIVE " " prototypes(typeId) AS ( " @@ -4447,9 +4453,17 @@ void ProjectStorage::checkForAliasChainCycle(PropertyDeclarationId propertyDecla NanotraceHR::Tracer tracer{"check for alias chain cycle", category(), keyValue("property declaration id", propertyDeclarationId)}; - auto callback = [=](PropertyDeclarationId currentPropertyDeclarationId) { - if (propertyDeclarationId == currentPropertyDeclarationId) + auto callback = [&](PropertyDeclarationId currentPropertyDeclarationId) { + if (propertyDeclarationId == currentPropertyDeclarationId) { + auto [propertyName, typeId] = s->selectPropertyDeclarationNameAndTypeIdForPropertyDeclarationIdStatement + .value>( + propertyDeclarationId); + auto [typeName, sourceId] = s->selectTypeNameAndSourceIdByTypeIdStatement + .value>(typeId); + errorNotifier->aliasCycle(typeName, propertyName, sourceId); + throw AliasChainCycle{}; + } }; s->selectPropertyDeclarationIdsForAliasChainStatement.readCallback(callback, diff --git a/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorageerrornotifierinterface.h b/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorageerrornotifierinterface.h index 18bd1d03842..5ac01683abc 100644 --- a/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorageerrornotifierinterface.h +++ b/src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorageerrornotifierinterface.h @@ -33,6 +33,10 @@ public: virtual void qmltypesFileMissing(QStringView qmltypesPath) = 0; virtual void prototypeCycle(Utils::SmallStringView typeName, SourceId typeSourceId) = 0; + virtual void aliasCycle(Utils::SmallStringView typeName, + Utils::SmallStringView propertyName, + SourceId typeSourceId) + = 0; protected: ~ProjectStorageErrorNotifierInterface() = default; diff --git a/src/plugins/qmldesigner/project/projectstorageerrornotifier.cpp b/src/plugins/qmldesigner/project/projectstorageerrornotifier.cpp index b03d68370ff..74a2894610d 100644 --- a/src/plugins/qmldesigner/project/projectstorageerrornotifier.cpp +++ b/src/plugins/qmldesigner/project/projectstorageerrornotifier.cpp @@ -98,4 +98,18 @@ void ProjectStorageErrorNotifier::prototypeCycle(Utils::SmallStringView typeName m_pathCache.sourcePath(typeSourceId)); } +void ProjectStorageErrorNotifier::aliasCycle(Utils::SmallStringView typeName, + Utils::SmallStringView propertyName, + SourceId typeSourceId) +{ + const QString typeNameString{typeName}; + const QString propertyNameString{propertyName}; + + logIssue(ProjectExplorer::Task::Error, + Tr::tr("Alias cycle detected for type %1 and property %2 in %3.") + .arg(typeNameString) + .arg(propertyNameString), + m_pathCache.sourcePath(typeSourceId)); +} + } // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/project/projectstorageerrornotifier.h b/src/plugins/qmldesigner/project/projectstorageerrornotifier.h index 3fd4441bd52..88c8f19e564 100644 --- a/src/plugins/qmldesigner/project/projectstorageerrornotifier.h +++ b/src/plugins/qmldesigner/project/projectstorageerrornotifier.h @@ -29,6 +29,9 @@ public: SourceId qmldirSourceId) override; void qmltypesFileMissing(QStringView qmltypesPath) override; void prototypeCycle(Utils::SmallStringView typeName, SourceId typeSourceId) override; + void aliasCycle(Utils::SmallStringView typeName, + Utils::SmallStringView propertyName, + SourceId typeSourceId) override; private: PathCacheType &m_pathCache; diff --git a/tests/unit/tests/mocks/projectstorageerrornotifiermock.h b/tests/unit/tests/mocks/projectstorageerrornotifiermock.h index 0a576075560..a2ddf0e5dc9 100644 --- a/tests/unit/tests/mocks/projectstorageerrornotifiermock.h +++ b/tests/unit/tests/mocks/projectstorageerrornotifiermock.h @@ -37,4 +37,10 @@ public: prototypeCycle, (Utils::SmallStringView typeName, QmlDesigner::SourceId typeSourceId), (override)); + MOCK_METHOD(void, + aliasCycle, + (Utils::SmallStringView typeName, + Utils::SmallStringView propertyName, + QmlDesigner::SourceId typeSourceId), + (override)); }; diff --git a/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp b/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp index 4004033216b..2fb71174d30 100644 --- a/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp +++ b/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp @@ -4677,7 +4677,7 @@ TEST_F(ProjectStorage, update_aliases_after_change_property_to_alias) "objects")))))); } -TEST_F(ProjectStorage, check_for_proto_type_cycle_throws) +TEST_F(ProjectStorage, check_for_alias_type_cycle_throws) { auto package{createSynchronizationPackageWithRecursiveAliases()}; package.types[1].propertyDeclarations.clear(); @@ -4688,7 +4688,7 @@ TEST_F(ProjectStorage, check_for_proto_type_cycle_throws) ASSERT_THROW(storage.synchronize(package), QmlDesigner::AliasChainCycle); } -TEST_F(ProjectStorage, check_for_proto_type_cycle_after_update_throws) +TEST_F(ProjectStorage, check_for_alias_type_cycle_after_update_throws) { auto package{createSynchronizationPackageWithRecursiveAliases()}; storage.synchronize(package); @@ -4702,6 +4702,21 @@ TEST_F(ProjectStorage, check_for_proto_type_cycle_after_update_throws) QmlDesigner::AliasChainCycle); } +TEST_F(ProjectStorage, check_for_alias_type_cycle_after_update_notifies_about_error) +{ + auto package{createSynchronizationPackageWithRecursiveAliases()}; + storage.synchronize(package); + package.types[1].propertyDeclarations.clear(); + package.types[1].propertyDeclarations.push_back(Storage::Synchronization::PropertyDeclaration{ + "objects", Storage::Synchronization::ImportedType{"AliasItem2"}, "objects"}); + importsSourceId2.emplace_back(qtQuickModuleId, Storage::Version{}, sourceId2); + + EXPECT_CALL(errorNotifierMock, aliasCycle(Eq("QObject"), Eq("objects"), sourceId2)); + + EXPECT_ANY_THROW(storage.synchronize( + SynchronizationPackage{importsSourceId2, {package.types[1]}, {sourceId2}})); +} + TEST_F(ProjectStorage, qualified_prototype) { auto package{createSimpleSynchronizationPackage()};