From 0259c67e630d0a39bc65dd85a5de3ba58180cb9d Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 21 Jun 2018 10:23:23 +0200 Subject: Provide an error location for invalid property assignments ... of the form "x.y.z: value", where there is no module x.y. The errors printed in this case did not have a location, because none of the value items are proper module instances and we did not keep track of the chain of parent items. Change-Id: I2da4da7beb5bd6f6d185a63c90d42340c9e30492 Reviewed-by: Joerg Bornemann --- src/lib/corelib/language/moduleloader.cpp | 29 +++++++++++----------- .../erroneous/original-in-export-item2.qbs | 14 +++++++++++ tests/auto/language/tst_language.cpp | 3 +++ 3 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 tests/auto/language/testdata/erroneous/original-in-export-item2.qbs diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp index 567cf0aa0..1cc894232 100644 --- a/src/lib/corelib/language/moduleloader.cpp +++ b/src/lib/corelib/language/moduleloader.cpp @@ -366,7 +366,7 @@ class PropertyDeclarationCheck : public ValueHandler { const Set &m_disabledItems; Set m_handledItems; - Item *m_parentItem; + std::vector m_parentItems; Item *m_currentModuleInstance = nullptr; QualifiedId m_currentModuleName; QString m_currentName; @@ -376,7 +376,6 @@ public: PropertyDeclarationCheck(const Set &disabledItems, const SetupProjectParameters ¶ms, Logger &logger) : m_disabledItems(disabledItems) - , m_parentItem(nullptr) , m_params(params) , m_logger(logger) { @@ -406,13 +405,13 @@ private: bool checkItemValue(ItemValue *value) { // TODO: Remove once QBS-1030 is fixed. - if (m_parentItem->type() == ItemType::Artifact) + if (parentItem()->type() == ItemType::Artifact) return false; - if (m_parentItem->type() == ItemType::Properties) + if (parentItem()->type() == ItemType::Properties) return false; - if (m_parentItem->isOfTypeOrhasParentOfType(ItemType::Export)) { + if (parentItem()->isOfTypeOrhasParentOfType(ItemType::Export)) { // Export item prototypes do not have instantiated modules. // The module instances are where the Export is used. QBS_ASSERT(m_currentModuleInstance, return false); @@ -430,14 +429,15 @@ private: if (!itemIsModuleInstance && value->item()->type() != ItemType::ModulePrefix - && (!m_parentItem->file() || !m_parentItem->file()->idScope() - || !m_parentItem->file()->idScope()->hasProperty(m_currentName)) + && (!parentItem()->file() || !parentItem()->file()->idScope() + || !parentItem()->file()->idScope()->hasProperty(m_currentName)) && !value->createdByPropertiesBlock()) { + CodeLocation location = value->location(); + for (int i = int(m_parentItems.size() - 1); !location.isValid() && i >= 0; --i) + location = m_parentItems.at(i)->location(); const ErrorInfo error(Tr::tr("Item '%1' is not declared. " "Did you forget to add a Depends item?") - .arg(m_currentModuleName.toString()), - value->location().isValid() ? value->location() - : m_parentItem->location()); + .arg(m_currentModuleName.toString()), location); handlePropertyError(error, m_params, m_logger); return false; } @@ -458,8 +458,7 @@ private: return; } - Item *oldParentItem = m_parentItem; - m_parentItem = item; + m_parentItems.push_back(item); for (Item::PropertyMap::const_iterator it = item->properties().constBegin(); it != item->properties().constEnd(); ++it) { const PropertyDeclaration decl = item->propertyDeclaration(it.key()); @@ -490,13 +489,13 @@ private: } m_currentName = it.key(); const QualifiedId oldModuleName = m_currentModuleName; - if (m_parentItem->type() != ItemType::ModulePrefix) + if (parentItem()->type() != ItemType::ModulePrefix) m_currentModuleName.clear(); m_currentModuleName.push_back(m_currentName); it.value()->apply(this); m_currentModuleName = oldModuleName; } - m_parentItem = oldParentItem; + m_parentItems.pop_back(); for (Item * const child : item->children()) { switch (child->type()) { case ItemType::Export: @@ -526,6 +525,8 @@ private: } void handle(VariantValue *) override { /* only created internally - no need to check */ } + + Item *parentItem() const { return m_parentItems.back(); } }; void ModuleLoader::handleTopLevelProject(ModuleLoaderResult *loadResult, Item *projectItem, diff --git a/tests/auto/language/testdata/erroneous/original-in-export-item2.qbs b/tests/auto/language/testdata/erroneous/original-in-export-item2.qbs new file mode 100644 index 000000000..d932d4aee --- /dev/null +++ b/tests/auto/language/testdata/erroneous/original-in-export-item2.qbs @@ -0,0 +1,14 @@ +import qbs + +Project { + Product { + name: "a" + Export { + x.y.z: original + } + } + Product { + name: "b" + Depends { name: "a" } + } +} diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp index 3c4637018..a82646e4e 100644 --- a/tests/auto/language/tst_language.cpp +++ b/tests/auto/language/tst_language.cpp @@ -880,6 +880,9 @@ void TestLanguage::erroneousFiles_data() QTest::newRow("original-in-export-item") << "original-in-export-item.qbs:7:32.*The special value 'original' cannot be used " "on the right-hand side of a property declaration."; + QTest::newRow("original-in-export-item2") + << "original-in-export-item2.qbs:6:9.*Item 'x.y' is not declared. Did you forget " + "to add a Depends item"; QTest::newRow("mismatching-multiplex-dependency") << "mismatching-multiplex-dependency.qbs:9:5.*Dependency from product " "'b \\{\"architecture\":\"mips\"\\}' to product 'a \\{\"architecture\":\"mips\"\\}'" -- cgit v1.2.3