QmlDesigner: Don't use extension for prototypes and properties

Extensions form a secondary inheritance hierarchy can be confusing. To
avoid ambiguity, the methods ProjectStorage::prototypeIds(TypeId type),
ProjectStorage::prototypeAndSelfIds(TypeId typeId),
ProjectStorage::heirIds(TypeId typeId), and
ProjectStorage::fetchPropertyDeclarationIds(TypeId baseTypeId) now
iterate only over the prototype chain.

Change-Id: I7efc7037e836c3f79e222befb0e12abc90162dfd
Reviewed-by: Burak Hancerli <burak.hancerli@qt.io>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/plugins/qmldesigner/libs/designercore/projectstorage/projectstorage.cpp

@@ -539,6 +539,16 @@ struct ProjectStorage::Statements
         "      WHERE extensionId IS NOT NULL) "
         "SELECT typeId FROM prototypes",
         database};
+    mutable Sqlite::ReadStatement<1, 1> selectPrototypeIdsStatement{
+        "WITH RECURSIVE "
+        "  prototypes(typeId) AS (  "
+        "      SELECT prototypeId FROM types WHERE typeId=?1 AND prototypeId IS NOT NULL "
+        "    UNION ALL "
+        "      SELECT prototypeId "
+        "      FROM types JOIN prototypes USING(typeId) "
+        "      WHERE prototypeId IS NOT NULL) "
+        "SELECT typeId FROM prototypes",
+        database};
     Sqlite::WriteStatement<3> updatePropertyDeclarationAliasIdAndTypeNameIdStatement{
         "UPDATE propertyDeclarations "
         "SET aliasPropertyDeclarationId=?2, "
@@ -875,7 +885,17 @@ struct ProjectStorage::Statements
         "SELECT p.value FROM json_each(?1) AS p", database};
     mutable Sqlite::ReadStatement<1, 1> selectTypeIdsByModuleIdStatement{
         "SELECT DISTINCT typeId FROM exportedTypeNames WHERE moduleId=?", database};
-    mutable Sqlite::ReadStatement<1, 1> selectHeirTypeIdsStatement{
+    mutable Sqlite::ReadStatement<1, 1> selectLegitimateHeirTypeIdsStatement{
+        "WITH RECURSIVE "
+        "  typeSelection(typeId) AS ("
+        "      SELECT typeId FROM types WHERE prototypeId=?1"
+        "    UNION ALL "
+        "      SELECT t.typeId "
+        "      FROM types AS t JOIN typeSelection AS ts "
+        "      WHERE prototypeId=ts.typeId)"
+        "SELECT typeId FROM typeSelection",
+        database};
+    mutable Sqlite::ReadStatement<1, 1> selectAllHeirTypeIdsStatement{
         "WITH RECURSIVE "
         "  typeSelection(typeId) AS ("
         "      SELECT typeId FROM types WHERE prototypeId=?1 OR extensionId=?1"
@@ -2029,8 +2049,7 @@ SmallTypeIds<16> ProjectStorage::prototypeIds(TypeId type) const
     using NanotraceHR::keyValue;
     NanotraceHR::Tracer tracer{"get prototypes", projectStorageCategory(), keyValue("type id", type)};
 
-    auto prototypeIds = s->selectPrototypeAndExtensionIdsStatement
+    auto prototypeIds = s->selectPrototypeIdsStatement.valuesWithTransaction<SmallTypeIds<16>>(type);
-                            .valuesWithTransaction<SmallTypeIds<16>>(type);
 
     tracer.end(keyValue("type ids", prototypeIds));
 
@@ -2045,7 +2064,7 @@ SmallTypeIds<16> ProjectStorage::prototypeAndSelfIds(TypeId typeId) const
     SmallTypeIds<16> prototypeAndSelfIds;
     prototypeAndSelfIds.push_back(typeId);
 
-    s->selectPrototypeAndExtensionIdsStatement.readToWithTransaction(prototypeAndSelfIds, typeId);
+    s->selectPrototypeIdsStatement.readToWithTransaction(prototypeAndSelfIds, typeId);
 
     tracer.end(keyValue("type ids", prototypeAndSelfIds));
 
@@ -2057,7 +2076,8 @@ SmallTypeIds<64> ProjectStorage::heirIds(TypeId typeId) const
     using NanotraceHR::keyValue;
     NanotraceHR::Tracer tracer{"get heirs", projectStorageCategory()};
 
-    auto heirIds = s->selectHeirTypeIdsStatement.valuesWithTransaction<SmallTypeIds<64>>(typeId);
+    auto heirIds = s->selectLegitimateHeirTypeIdsStatement.valuesWithTransaction<SmallTypeIds<64>>(
+        typeId);
 
     tracer.end(keyValue("type ids", heirIds));
 
@@ -3523,7 +3543,7 @@ QVarLengthArray<PropertyDeclarationId, 128> ProjectStorage::fetchPropertyDeclara
 
     s->selectLocalPropertyDeclarationIdsForTypeStatement.readTo(propertyDeclarationIds, baseTypeId);
 
-    auto range = s->selectPrototypeAndExtensionIdsStatement.range<TypeId>(baseTypeId);
+    auto range = s->selectPrototypeIdsStatement.range<TypeId>(baseTypeId);
 
     for (TypeId prototype : range) {
         s->selectLocalPropertyDeclarationIdsForTypeStatement.readTo(propertyDeclarationIds, prototype);

tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp

@@ -881,7 +881,10 @@ protected:
         return package;
     }
 
-    auto createPackageWithProperties()
+    enum class ExchangePrototypeAndExtension { No, Yes };
+
+    auto createPackageWithProperties(
+        ExchangePrototypeAndExtension exchange = ExchangePrototypeAndExtension::No)
     {
         SynchronizationPackage package;
 
@@ -906,8 +909,12 @@ protected:
             {Storage::Synchronization::SignalDeclaration{"valuesChanged", {}}}});
         package.types.push_back(Storage::Synchronization::Type{
             "QObject2",
-            Storage::Synchronization::ImportedType{"Object"},
+            exchange == ExchangePrototypeAndExtension::No
-            Storage::Synchronization::ImportedType{},
+                ? Storage::Synchronization::ImportedType{"Object"}
+                : Storage::Synchronization::ImportedType{},
+            exchange == ExchangePrototypeAndExtension::Yes
+                ? Storage::Synchronization::ImportedType{"Object"}
+                : Storage::Synchronization::ImportedType{},
             TypeTraitsKind::Reference,
             sourceId1,
             {Storage::Synchronization::ExportedType{qmlModuleId, "Object2", Storage::Version{}}},
@@ -7159,22 +7166,15 @@ TEST_F(ProjectStorage, get_property_declaration_ids_over_prototype_chain)
                                      HasName("children3")));
 }
 
-TEST_F(ProjectStorage, get_property_declaration_ids_over_extension_chain)
+TEST_F(ProjectStorage, get_no_property_declaration_ids_over_extension_chain)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject3");
 
     auto propertyIds = storage.propertyDeclarationIds(typeId);
 
-    ASSERT_THAT(propertyIds,
+    ASSERT_THAT(propertyIds, Not(AnyOf(Contains(HasName("data")), Contains(HasName("children")))));
-                UnorderedElementsAre(HasName("data"),
-                                     HasName("children"),
-                                     HasName("data2"),
-                                     HasName("children2"),
-                                     HasName("data3"),
-                                     HasName("children3")));
 }
 
 TEST_F(ProjectStorage, get_property_declaration_ids_are_returned_sorted)
@@ -7249,8 +7249,7 @@ TEST_F(ProjectStorage, get_property_declaration_id_over_prototype_chain)
 
 TEST_F(ProjectStorage, get_property_declaration_id_over_extension_chain)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject3");
 
@@ -7416,8 +7415,7 @@ TEST_F(ProjectStorage, get_only_signal_declaration_names_from_up_into_the_protot
 
 TEST_F(ProjectStorage, get_only_signal_declaration_names_from_up_into_the_extension_chain)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject2");
 
@@ -7472,8 +7470,7 @@ TEST_F(ProjectStorage, get_only_function_declaration_names_from_up_into_the_prot
 
 TEST_F(ProjectStorage, get_only_function_declaration_names_from_up_into_the_extension_chain)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject2");
 
@@ -7893,17 +7890,15 @@ TEST_F(ProjectStorage, get_no_prototype_ids_for_no_prototype)
     ASSERT_THAT(prototypeIds, IsEmpty());
 }
 
-TEST_F(ProjectStorage, get_prototype_ids_with_extension)
+TEST_F(ProjectStorage, get_prototype_ids_without_extension)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject3");
 
     auto prototypeIds = storage.prototypeIds(typeId);
 
-    ASSERT_THAT(prototypeIds,
+    ASSERT_THAT(prototypeIds, Not(Contains(fetchTypeId(sourceId1, "QObject")))) << package;
-                ElementsAre(fetchTypeId(sourceId1, "QObject2"), fetchTypeId(sourceId1, "QObject")));
 }
 
 TEST_F(ProjectStorage, get_prototype_and_self_ids)
@@ -7933,17 +7928,13 @@ TEST_F(ProjectStorage, get_self_for_no_prototype_ids)
 
 TEST_F(ProjectStorage, get_prototype_and_self_ids_with_extension)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject3");
 
     auto prototypeAndSelfIds = storage.prototypeAndSelfIds(typeId);
 
-    ASSERT_THAT(prototypeAndSelfIds,
+    ASSERT_THAT(prototypeAndSelfIds, Not(Contains(fetchTypeId(sourceId1, "QObject"))));
-                ElementsAre(fetchTypeId(sourceId1, "QObject3"),
-                            fetchTypeId(sourceId1, "QObject2"),
-                            fetchTypeId(sourceId1, "QObject")));
 }
 
 TEST_F(ProjectStorage, is_based_on_for_direct_prototype)
@@ -7973,8 +7964,7 @@ TEST_F(ProjectStorage, is_based_on_for_indirect_prototype)
 
 TEST_F(ProjectStorage, is_based_on_for_direct_extension)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject2");
     auto baseTypeId = fetchTypeId(sourceId1, "QObject");
@@ -7986,8 +7976,7 @@ TEST_F(ProjectStorage, is_based_on_for_direct_extension)
 
 TEST_F(ProjectStorage, is_based_on_for_indirect_extension)
 {
-    auto package{createPackageWithProperties()};
+    auto package{createPackageWithProperties(ExchangePrototypeAndExtension::Yes)};
-    std::swap(package.types[1].extension, package.types[1].prototype);
     storage.synchronize(package);
     auto typeId = fetchTypeId(sourceId1, "QObject3");
     auto baseTypeId = fetchTypeId(sourceId1, "QObject");
@@ -8987,9 +8976,7 @@ TEST_F(ProjectStorage, get_hair_ids)
 
     ASSERT_THAT(heirIds,
                 UnorderedElementsAre(fetchTypeId(sourceId1, "QObject2"),
-                                     fetchTypeId(sourceId1, "QObject3"),
+                                     fetchTypeId(sourceId1, "QObject3")));
-                                     fetchTypeId(sourceId1, "QObject4"),
-                                     fetchTypeId(sourceId1, "QObject5")));
 }
 
 TEST_F(ProjectStorage, get_no_hair_ids_for_invalid_type_id)