DependenciesResolver: Fix module name prefix collision check

The algorithm did not properly handle names with more than two
components.

Fixes: QBS-1772
Change-Id: Ia680c69907c449e46977f2c5615707805a3b785f
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

16 files changed, 66 insertions, 10 deletions

diff --git a/src/lib/corelib/loader/dependenciesresolver.cpp b/src/lib/corelib/loader/dependenciesresolver.cpp
index b46ed44a0..2110e3254 100644
--- a/src/lib/corelib/loader/dependenciesresolver.cpp
+++ b/src/lib/corelib/loader/dependenciesresolver.cpp
@@ -993,21 +993,30 @@ void DependenciesResolver::checkForModuleNamePrefixCollision(
         return;
 
     for (const Item::Module &m : m_product.item->modules()) {
-        if (m.name.length() == dependency.name.length()
-            || m.name.front() != dependency.name.front()) {
+        if (m.name.length() == dependency.name.length())
             continue;
-        }
+
         QualifiedId shortName;
         QualifiedId longName;
-        if (m.name < dependency.name) {
+        if (m.name.length() < dependency.name.length()) {
             shortName = m.name;
             longName = dependency.name;
         } else {
             shortName = dependency.name;
             longName = m.name;
         }
-        throw ErrorInfo(Tr::tr("The name of module '%1' is equal to the first component of the "
-                               "name of module '%2', which is not allowed")
+        const auto isPrefix = [&] {
+            for (int i = 0; i < shortName.length(); ++i) {
+                if (shortName.at(i) != longName.at(i))
+                    return false;
+            }
+            return true;
+        };
+        if (!isPrefix())
+            continue;
+
+        throw ErrorInfo(Tr::tr("The name of module '%1' is a prefix of the name of module '%2', "
+                               "which is not allowed")
                             .arg(shortName.toString(), longName.toString()), dependency.location());
     }
 }
diff --git a/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs b/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs
new file mode 100644
index 000000000..cd55aa946
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/complex-collision.qbs
@@ -0,0 +1,4 @@
+Product {
+    Depends { name: "prefix1.middle1" }
+    Depends { name: "prefix1.middle1.suffix1" }
+}
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs
new file mode 100644
index 000000000..5e45122de
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/middle1.qbs
@@ -0,0 +1 @@
+Module {}
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs
new file mode 100644
index 000000000..5e45122de
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix1/suffix1.qbs
@@ -0,0 +1 @@
+Module {}
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs
new file mode 100644
index 000000000..5e45122de
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle1/suffix2/suffix2.qbs
@@ -0,0 +1 @@
+Module {}
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs
new file mode 100644
index 000000000..5e45122de
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/middle2/suffix/suffix.qbs
@@ -0,0 +1 @@
+Module {}
diff --git a/tests/auto/language/testdata/erroneous/modules/prefix1/prefix1.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/prefix1.qbs
index 84957060c..84957060c 100644
--- a/tests/auto/language/testdata/erroneous/modules/prefix1/prefix1.qbs
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/prefix1.qbs
diff --git a/tests/auto/language/testdata/erroneous/modules/prefix1/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/suffix/suffix.qbs
index 218a4feb7..218a4feb7 100644
--- a/tests/auto/language/testdata/erroneous/modules/prefix1/suffix/suffix.qbs
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix1/suffix/suffix.qbs
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs
new file mode 100644
index 000000000..a5aaa6f8b
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/prefix2.qbs
@@ -0,0 +1,3 @@
+Module {
+    Depends { name: "prefix2.suffix" }
+}
diff --git a/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs
new file mode 100644
index 000000000..84957060c
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/modules/prefix2/suffix/suffix.qbs
@@ -0,0 +1,2 @@
+Module {
+}
diff --git a/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs b/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs
new file mode 100644
index 000000000..e5a94ad66
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/no-collision1.qbs
@@ -0,0 +1,4 @@
+Product {
+    Depends { name: "prefix1.middle1.suffix1" }
+    Depends { name: "prefix1.middle1.suffix2" }
+}
diff --git a/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs b/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs
new file mode 100644
index 000000000..664ec1729
--- /dev/null
+++ b/tests/auto/language/testdata/module-name-collisions/no-collision2.qbs
@@ -0,0 +1,4 @@
+Product {
+    Depends { name: "prefix1.middle1" }
+    Depends { name: "prefix1.middle2.suffix" }
+}
diff --git a/tests/auto/language/testdata/erroneous/same-module-prefix1.qbs b/tests/auto/language/testdata/module-name-collisions/simple-collision1.qbs
index 8aba31c2a..8aba31c2a 100644
--- a/tests/auto/language/testdata/erroneous/same-module-prefix1.qbs
+++ b/tests/auto/language/testdata/module-name-collisions/simple-collision1.qbs
diff --git a/tests/auto/language/testdata/erroneous/same-module-prefix2.qbs b/tests/auto/language/testdata/module-name-collisions/simple-collision2.qbs
index 6679091c2..6679091c2 100644
--- a/tests/auto/language/testdata/erroneous/same-module-prefix2.qbs
+++ b/tests/auto/language/testdata/module-name-collisions/simple-collision2.qbs
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 2fbffdc03..2f6817231 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -956,10 +956,6 @@ void TestLanguage::erroneousFiles_data()
                "cannot read property 'includes' of undefined";
     QTest::newRow("misused-inherited-property") << "Binding to non-item property";
     QTest::newRow("undeclared_property_in_Properties_item") << "Item 'blubb' is not declared";
-    QTest::newRow("same-module-prefix1") << "The name of module 'prefix1' is equal to the first "
-                                            "component of the name of module 'prefix1.suffix'";
-    QTest::newRow("same-module-prefix2") << "The name of module 'prefix2' is equal to the first "
-                                            "component of the name of module 'prefix2.suffix'";
     QTest::newRow("conflicting-properties-in-export-items")
             << "Export item in inherited item redeclares property 'theProp' with different type.";
     QTest::newRow("invalid-property-option")
@@ -1765,6 +1761,34 @@ void TestLanguage::moduleMergingVariantValues()
     QCOMPARE(exceptionCaught, false);
 }
 
+void TestLanguage::moduleNameCollisions_data()
+{
+    QTest::addColumn<QString>("projectFile");
+    QTest::addColumn<bool>("collisionExpected");
+
+    QTest::newRow("simple collision (one order)") << "simple-collision1.qbs" << true;
+    QTest::newRow("simple collision (other order)") << "simple-collision2.qbs" << true;
+    QTest::newRow("collision with more components") << "complex-collision.qbs" << true;
+    QTest::newRow("no collision (same length)") << "no-collision1.qbs" << false;
+    QTest::newRow("no collision (different length)") << "no-collision2.qbs" << false;
+}
+
+void TestLanguage::moduleNameCollisions()
+{
+    QFETCH(QString, projectFile);
+    QFETCH(bool, collisionExpected);
+
+    try {
+        const QString compositeProjectFilePath = QString("module-name-collisions/") + projectFile;
+        QVERIFY(resolveProject(qPrintable(compositeProjectFilePath)));
+        QVERIFY(!collisionExpected);
+    } catch (const ErrorInfo &e) {
+        const QString errorString = e.toString();
+        QVERIFY2(collisionExpected, qPrintable(errorString));
+        QVERIFY2(errorString.contains("not allowed"), qPrintable(errorString));
+    }
+}
+
 void TestLanguage::moduleParameters_data()
 {
     QTest::addColumn<QVariantMap>("inputProperties");
diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h
index a7c39cec2..870b7d1f8 100644
--- a/tests/auto/language/tst_language.h
+++ b/tests/auto/language/tst_language.h
@@ -119,6 +119,8 @@ private slots:
     void jsImportUsedInMultipleScopes();
     void localProfileAsTopLevelProfile();
     void moduleMergingVariantValues();
+    void moduleNameCollisions_data();
+    void moduleNameCollisions();
     void moduleParameters_data();
     void moduleParameters();
     void modulePrioritizationBySearchPath_data();