Do not allow id properties in Module itemsHEADmaster

This file-scope QML construct is generally rather dubious in qbs, but we
make it work in most contexts by injecting the id scope into e.g. the
surrounding module instance. However, for modules itself this is not
possible, and there we should prevent id usage altogether, rather than
letting users run into weird follow-up problems.

Task-number: QBS-1770
Change-Id: Ib3efca21e77758c22313d19afe60cd877be0d98a
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

7 files changed, 17 insertions, 11 deletions

diff --git a/share/qbs/modules/xcode/xcode.qbs b/share/qbs/modules/xcode/xcode.qbs
index c052da44c..2c0cd001a 100644
--- a/share/qbs/modules/xcode/xcode.qbs
+++ b/share/qbs/modules/xcode/xcode.qbs
@@ -9,21 +9,19 @@ import qbs.PropertyList
 import qbs.Utilities
 
 Module {
-    id: xcodeModule
-
     Probes.XcodeLocationProbe {
         id: xcodeLocationProbe
-        condition: !xcodeModule.developerPath
+        condition: !developerPath
     }
 
     Probes.XcodeProbe {
         id: xcodeProbe
-        developerPath: xcodeModule.developerPath
-        platformType: xcodeModule.platformType
-        platformPath: xcodeModule.platformPath
-        devicePlatformPath: xcodeModule.devicePlatformPath
-        xcodebuildPath: xcodeModule.xcodebuildPath
-        sdksPath: xcodeModule.sdksPath
+        developerPath: parent.developerPath
+        platformType: parent.platformType
+        platformPath: parent.platformPath
+        devicePlatformPath: parent.devicePlatformPath
+        xcodebuildPath: parent.xcodebuildPath
+        sdksPath: parent.sdksPath
     }
 
     condition: qbs.targetOS.includes("darwin") &&
diff --git a/src/lib/corelib/loader/itemreaderastvisitor.cpp b/src/lib/corelib/loader/itemreaderastvisitor.cpp
index fe0d6c190..c3e6b9b89 100644
--- a/src/lib/corelib/loader/itemreaderastvisitor.cpp
+++ b/src/lib/corelib/loader/itemreaderastvisitor.cpp
@@ -248,6 +248,8 @@ bool ItemReaderASTVisitor::visit(AST::UiScriptBinding *ast)
         const auto * const idExp = AST::cast<AST::IdentifierExpression *>(expStmt->expression);
         if (Q_UNLIKELY(!idExp || idExp->name.isEmpty()))
             throw ErrorInfo(Tr::tr("id: must be followed by identifier"));
+        if (m_item->type() == ItemType::Module)
+            throw ErrorInfo(Tr::tr("Module items cannot have an id property."));
         m_item->m_id = idExp->name.toString();
         m_file->ensureIdScope(m_itemPool);
         ItemValueConstPtr existingId = m_file->idScope()->itemProperty(m_item->id(), *m_itemPool);
diff --git a/tests/auto/language/testdata/erroneous/module-with-id.qbs b/tests/auto/language/testdata/erroneous/module-with-id.qbs
new file mode 100644
index 000000000..5b980100f
--- /dev/null
+++ b/tests/auto/language/testdata/erroneous/module-with-id.qbs
@@ -0,0 +1,4 @@
+Product {
+    name: "p"
+    Depends { name: "module-with-id" }
+}
diff --git a/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithId.qbs b/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithId.qbs
new file mode 100644
index 000000000..d20b96784
--- /dev/null
+++ b/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithId.qbs
@@ -0,0 +1 @@
+ModuleWithIdParent { id: foo }
diff --git a/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithIdParent.qbs b/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithIdParent.qbs
new file mode 100644
index 000000000..5e45122de
--- /dev/null
+++ b/tests/auto/language/testdata/erroneous/modules/module-with-id/ModuleWithIdParent.qbs
@@ -0,0 +1 @@
+Module {}
diff --git a/tests/auto/language/testdata/modules/dummyqt/core/dummycore.qbs b/tests/auto/language/testdata/modules/dummyqt/core/dummycore.qbs
index 746c7e37b..2764af9c2 100644
--- a/tests/auto/language/testdata/modules/dummyqt/core/dummycore.qbs
+++ b/tests/auto/language/testdata/modules/dummyqt/core/dummycore.qbs
@@ -1,11 +1,10 @@
 Module {
-    id: qtcore
     property int versionMajor: 5
     property int versionMinor: 0
     property int versionPatch: 0
     property string version: versionMajor.toString() + "." + versionMinor.toString() + "." + versionPatch.toString()
     property string coreProperty: "coreProperty"
-    property string coreVersion: qtcore.version
+    property string coreVersion: version
     property string zort: "zort in dummyqt.core"
 
     Depends { name: "dummy" }
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 456e8b9d0..6a3344842 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -1017,6 +1017,7 @@ void TestLanguage::erroneousFiles_data()
     QTest::newRow("frozen-object-list") << "object is not extensible";
     QTest::newRow("module-property-binding-in-project")
         << "Module properties cannot be set in Project items";
+    QTest::newRow("module-with-id") << "Module items cannot have an id property";
 }
 
 void TestLanguage::erroneousFiles()