From d94ba7080d32050d7c5d12d9cf03ecb7b9b7ff46 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 27 Jul 2016 13:52:52 +0200 Subject: Separate file scope and imports scope When evaluating, imported names need to have a higher precedence than names in the item scope, as otherwise they can clash with dependencies of the product (see bug report and autotest). We cannot simply move the file scope's precedence up, because in addition to imports and the "file" and "filePath" properties, it also potentially contains ids from e.g. a module prototype, which would then override the respective entry in the module scope. Task-number: QBS-930 Change-Id: Ifb8b7c933225f872ccc3e878b2bf01b7b0ac0f99 Reviewed-by: Joerg Bornemann --- src/lib/corelib/language/evaluator.cpp | 9 +++++++++ src/lib/corelib/language/evaluator.h | 2 ++ src/lib/corelib/language/evaluatorscriptclass.cpp | 3 +++ src/lib/corelib/language/moduleloader.cpp | 2 ++ .../blackbox/testdata/imports-conflict/imports-conflict.qbs | 11 +++++++++++ .../testdata/imports-conflict/modules/themodule/m.qbs | 5 +++++ .../testdata/imports-conflict/modules/themodule/utils.js | 1 + tests/auto/blackbox/tst_blackbox.cpp | 6 ++++++ tests/auto/blackbox/tst_blackbox.h | 1 + 9 files changed, 40 insertions(+) create mode 100644 tests/auto/blackbox/testdata/imports-conflict/imports-conflict.qbs create mode 100644 tests/auto/blackbox/testdata/imports-conflict/modules/themodule/m.qbs create mode 100644 tests/auto/blackbox/testdata/imports-conflict/modules/themodule/utils.js diff --git a/src/lib/corelib/language/evaluator.cpp b/src/lib/corelib/language/evaluator.cpp index 329ea4716..8e4fdde5b 100644 --- a/src/lib/corelib/language/evaluator.cpp +++ b/src/lib/corelib/language/evaluator.cpp @@ -232,6 +232,15 @@ QScriptValue Evaluator::fileScope(const FileContextConstPtr &file) result = m_scriptEngine->newObject(); result.setProperty(QLatin1String("filePath"), file->filePath()); result.setProperty(QLatin1String("path"), file->dirPath()); + return result; +} + +QScriptValue Evaluator::importScope(const FileContextConstPtr &file) +{ + QScriptValue &result = m_importScopeMap[file]; + if (result.isObject()) + return result; + result = m_scriptEngine->newObject(); m_scriptEngine->import(file, result); JsExtensions::setupExtensions(file->jsExtensions(), result); return result; diff --git a/src/lib/corelib/language/evaluator.h b/src/lib/corelib/language/evaluator.h index 2c12abb34..e20c0f8d9 100644 --- a/src/lib/corelib/language/evaluator.h +++ b/src/lib/corelib/language/evaluator.h @@ -75,6 +75,7 @@ public: QScriptValue scriptValue(const Item *item); QScriptValue fileScope(const FileContextConstPtr &file); + QScriptValue importScope(const FileContextConstPtr &file); void setCachingEnabled(bool enabled); @@ -93,6 +94,7 @@ private: EvaluatorScriptClass *m_scriptClass; mutable QHash m_scriptValueMap; mutable QHash m_fileScopeMap; + mutable QHash m_importScopeMap; }; class EvalCacheEnabler diff --git a/src/lib/corelib/language/evaluatorscriptclass.cpp b/src/lib/corelib/language/evaluatorscriptclass.cpp index bfac9d6ac..ed0ec2c49 100644 --- a/src/lib/corelib/language/evaluatorscriptclass.cpp +++ b/src/lib/corelib/language/evaluatorscriptclass.cpp @@ -194,10 +194,12 @@ private: if (alternative->value->definingItem()) pushItemScopes(alternative->value->definingItem()); engine->currentContext()->pushScope(conditionScope); + engine->currentContext()->pushScope(data->evaluator->importScope(value->file())); const QScriptValue cr = engine->evaluate(alternative->condition); const QScriptValue overrides = engine->evaluate(alternative->overrideListProperties); engine->currentContext()->popScope(); engine->currentContext()->popScope(); + engine->currentContext()->popScope(); popScopes(); if (engine->hasErrorOrException(cr)) { *result = engine->lastErrorValue(cr); @@ -267,6 +269,7 @@ private: if (value->definingItem()) pushItemScopes(value->definingItem()); pushScope(extraScope); + pushScope(data->evaluator->importScope(value->file())); *result = engine->evaluate(value->sourceCodeForEvaluation(), value->file()->filePath(), value->line()); popScopes(); diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp index bcdad9cdf..471755e8d 100644 --- a/src/lib/corelib/language/moduleloader.cpp +++ b/src/lib/corelib/language/moduleloader.cpp @@ -2151,6 +2151,7 @@ void ModuleLoader::resolveProbe(ProductContext *productContext, Item *parent, It QScriptValue scope = m_engine->newObject(); m_engine->currentContext()->pushScope(m_evaluator->scriptValue(parent)); m_engine->currentContext()->pushScope(m_evaluator->fileScope(configureScript->file())); + m_engine->currentContext()->pushScope(m_evaluator->importScope(configureScript->file())); m_engine->currentContext()->pushScope(scope); foreach (const ProbeProperty &b, probeBindings) scope.setProperty(b.first, b.second); @@ -2185,6 +2186,7 @@ void ModuleLoader::resolveProbe(ProductContext *productContext, Item *parent, It m_engine->currentContext()->popScope(); m_engine->currentContext()->popScope(); m_engine->currentContext()->popScope(); + m_engine->currentContext()->popScope(); if (evalError.hasError()) throw evalError; } diff --git a/tests/auto/blackbox/testdata/imports-conflict/imports-conflict.qbs b/tests/auto/blackbox/testdata/imports-conflict/imports-conflict.qbs new file mode 100644 index 000000000..1c6be24bd --- /dev/null +++ b/tests/auto/blackbox/testdata/imports-conflict/imports-conflict.qbs @@ -0,0 +1,11 @@ +import qbs + +Project { + Product { + name: "Utils" + } + Product { + Depends { name: "Utils" } + Depends { name: "themodule" } + } +} diff --git a/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/m.qbs b/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/m.qbs new file mode 100644 index 000000000..48145d380 --- /dev/null +++ b/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/m.qbs @@ -0,0 +1,5 @@ +import "utils.js" as Utils + +Module { + validate: { Utils.helper(); } +} diff --git a/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/utils.js b/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/utils.js new file mode 100644 index 000000000..ad54d4d0f --- /dev/null +++ b/tests/auto/blackbox/testdata/imports-conflict/modules/themodule/utils.js @@ -0,0 +1 @@ +function helper() { console.info("helper"); } diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index c885bec4f..454644de3 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -3978,6 +3978,12 @@ void TestBlackbox::importingProduct() QCOMPARE(runQbs(), 0); } +void TestBlackbox::importsConflict() +{ + QDir::setCurrent(testDataDir + "/imports-conflict"); + QCOMPARE(runQbs(), 0); +} + void TestBlackbox::infoPlist() { if (!HostOsInfo::isMacosHost()) diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h index 06df7f53c..4b5c410ac 100644 --- a/tests/auto/blackbox/tst_blackbox.h +++ b/tests/auto/blackbox/tst_blackbox.h @@ -85,6 +85,7 @@ private slots: void iconsetApp(); void importInPropertiesCondition(); void importingProduct(); + void importsConflict(); void infoPlist(); void inputsFromDependencies(); void installable(); -- cgit v1.2.3