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 <joerg.bornemann@qt.io>

9 files changed, 40 insertions, 0 deletions

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<const Item *, QScriptValue> m_scriptValueMap;
     mutable QHash<FileContextConstPtr, QScriptValue> m_fileScopeMap;
+    mutable QHash<FileContextConstPtr, QScriptValue> 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();