Check for duplicate product names.

Strangely, no such check exists currently. Instead, one will typically
get an internal error on the artifact level later on.

Change-Id: Id9cd749a70564100bd7286b896f64cafbe9bfb73
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

9 files changed, 78 insertions, 3 deletions

diff --git a/src/lib/buildgraph/buildgraph.cpp b/src/lib/buildgraph/buildgraph.cpp
index 819de79cc..f8c4f8b6a 100644
--- a/src/lib/buildgraph/buildgraph.cpp
+++ b/src/lib/buildgraph/buildgraph.cpp
@@ -396,7 +396,7 @@ void insertArtifact(const ResolvedProductPtr &product, Artifact *artifact, const
                         pl.append(QString("  - %1 \n").arg(p->name));
                 }
                 throw ErrorInfo(QString ("BUG: already inserted in this project: %1\n%2")
-                            .arg(artifact->filePath()).arg(pl));
+                            .arg(artifact->filePath()).arg(pl), CodeLocation(), true);
             }
         }
     }
@@ -462,18 +462,27 @@ static void doSanityChecksForProduct(const ResolvedProductConstPtr &product, con
     }
 }
 
-void doSanityChecks(const ResolvedProjectPtr &project, const Logger &logger)
+static void doSanityChecks(const ResolvedProjectPtr &project, QSet<QString> &productNames,
+                           const Logger &logger)
 {
     logger.qbsDebug() << "Sanity checking project '" << project->name << "'";
     foreach (const ResolvedProjectPtr &subProject, project->subProjects)
-        doSanityChecks(subProject, logger);
+        doSanityChecks(subProject, productNames, logger);
 
     foreach (const ResolvedProductConstPtr &product, project->products) {
         QBS_CHECK(product->project == project);
         QBS_CHECK(product->topLevelProject() == project->topLevelProject());
         doSanityChecksForProduct(product, logger);
+        QBS_CHECK(!productNames.contains(product->name));
+        productNames << product->name;
     }
 }
 
+void doSanityChecks(const ResolvedProjectPtr &project, const Logger &logger)
+{
+    QSet<QString> productNames;
+    doSanityChecks(project, productNames, logger);
+}
+
 } // namespace Internal
 } // namespace qbs
diff --git a/src/lib/language/projectresolver.cpp b/src/lib/language/projectresolver.cpp
index 26284ac6d..75601399e 100644
--- a/src/lib/language/projectresolver.cpp
+++ b/src/lib/language/projectresolver.cpp
@@ -81,6 +81,25 @@ void ProjectResolver::setProgressObserver(ProgressObserver *observer)
     m_progressObserver = observer;
 }
 
+static void checkForDuplicateProductNames(const TopLevelProjectConstPtr &project)
+{
+    const QList<ResolvedProductPtr> allProducts = project->allProducts();
+    for (int i = 0; i < allProducts.count(); ++i) {
+        const ResolvedProductConstPtr product1 = allProducts.at(i);
+        const QString productName = product1->name;
+        for (int j = i + 1; j < allProducts.count(); ++j) {
+            const ResolvedProductConstPtr product2 = allProducts.at(j);
+            if (product2->name == productName) {
+                ErrorInfo error;
+                error.append(Tr::tr("Duplicate product name '%1'.").arg(productName));
+                error.append(Tr::tr("First product defined here."), product1->location);
+                error.append(Tr::tr("Second product defined here."), product2->location);
+                throw error;
+            }
+        }
+    }
+}
+
 TopLevelProjectPtr ProjectResolver::resolve(ModuleLoaderResult &loadResult,
         const QString &buildRoot, const QVariantMap &overriddenProperties,
         const QVariantMap &buildConfiguration)
@@ -98,6 +117,7 @@ TopLevelProjectPtr ProjectResolver::resolve(ModuleLoaderResult &loadResult,
     m_moduleContext = 0;
     resolveTopLevelProject(loadResult.root, &projectContext);
     TopLevelProjectPtr top = projectContext.project.staticCast<TopLevelProject>();
+    checkForDuplicateProductNames(top);
     top->buildSystemFiles.unite(loadResult.qbsFiles);
     return top;
 }
diff --git a/tests/auto/blackbox/testdata/duplicateProductNames/explicit.qbs b/tests/auto/blackbox/testdata/duplicateProductNames/explicit.qbs
new file mode 100644
index 000000000..0183b411c
--- /dev/null
+++ b/tests/auto/blackbox/testdata/duplicateProductNames/explicit.qbs
@@ -0,0 +1,7 @@
+import qbs
+
+Project {
+    Product { name: "blubb" }
+    Product { name: "blubb" }
+    Product { name: "blubb" }
+}
diff --git a/tests/auto/blackbox/testdata/duplicateProductNames/implicit-indirect.qbs b/tests/auto/blackbox/testdata/duplicateProductNames/implicit-indirect.qbs
new file mode 100644
index 000000000..7164aa1c5
--- /dev/null
+++ b/tests/auto/blackbox/testdata/duplicateProductNames/implicit-indirect.qbs
@@ -0,0 +1,5 @@
+import qbs
+
+Project {
+    references: ["subdir1/subproject.qbs", "subdir2/subproject.qbs"]
+}
diff --git a/tests/auto/blackbox/testdata/duplicateProductNames/implicit.qbs b/tests/auto/blackbox/testdata/duplicateProductNames/implicit.qbs
new file mode 100644
index 000000000..f39f80626
--- /dev/null
+++ b/tests/auto/blackbox/testdata/duplicateProductNames/implicit.qbs
@@ -0,0 +1,7 @@
+import qbs
+
+Project {
+    Product { }
+    Product { }
+    Product { }
+}
diff --git a/tests/auto/blackbox/testdata/duplicateProductNames/subdir1/subproject.qbs b/tests/auto/blackbox/testdata/duplicateProductNames/subdir1/subproject.qbs
new file mode 100644
index 000000000..6d16a3c53
--- /dev/null
+++ b/tests/auto/blackbox/testdata/duplicateProductNames/subdir1/subproject.qbs
@@ -0,0 +1,3 @@
+import qbs
+
+Product { }
diff --git a/tests/auto/blackbox/testdata/duplicateProductNames/subdir2/subproject.qbs b/tests/auto/blackbox/testdata/duplicateProductNames/subdir2/subproject.qbs
new file mode 100644
index 000000000..6d16a3c53
--- /dev/null
+++ b/tests/auto/blackbox/testdata/duplicateProductNames/subdir2/subproject.qbs
@@ -0,0 +1,3 @@
+import qbs
+
+Product { }
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 8cbbbb50a..ddebcce0e 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1188,6 +1188,25 @@ void TestBlackbox::disabledProject()
     QCOMPARE(runQbs(), 0);
 }
 
+void TestBlackbox::duplicateProductNames()
+{
+    QDir::setCurrent(testDataDir + "/duplicateProductNames");
+    QFETCH(QString, projectFileName);
+    QbsRunParameters params;
+    params.expectFailure = true;
+    params.arguments = QStringList() << "-f" << projectFileName;
+    QVERIFY(runQbs(params) != 0);
+    QVERIFY(m_qbsStderr.contains("Duplicate product name"));
+}
+
+void TestBlackbox::duplicateProductNames_data()
+{
+    QTest::addColumn<QString>("projectFileName");
+    QTest::newRow("Names explicitly set") << QString("explicit.qbs");
+    QTest::newRow("Unnamed products in same file") << QString("implicit.qbs");
+    QTest::newRow("Unnamed products in files of the same name") << QString("implicit-indirect.qbs");
+}
+
 void TestBlackbox::dynamicLibs()
 {
     QDir::setCurrent(testDataDir + "/dynamicLibs");
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index c9a5d975b..042446a96 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -101,6 +101,8 @@ private slots:
     void dependenciesProperty();
     void disabledProduct();
     void disabledProject();
+    void duplicateProductNames();
+    void duplicateProductNames_data();
     void dynamicLibs();
     void explicitlyDependsOn();
     void fileDependencies();