Clean up ModuleProviderLoader interface

Most importantly, get rid of the reverse dependency on
ProjectTreeBuilder.

Change-Id: I1ad657767f9a083cdbd4c18779de15a4c5f2fbd5
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

4 files changed, 86 insertions, 71 deletions

diff --git a/src/lib/corelib/language/moduleproviderloader.cpp b/src/lib/corelib/language/moduleproviderloader.cpp
index 387b102d0..5b7cfa7a2 100644
--- a/src/lib/corelib/language/moduleproviderloader.cpp
+++ b/src/lib/corelib/language/moduleproviderloader.cpp
@@ -55,6 +55,7 @@
 #include <tools/fileinfo.h>
 #include <tools/jsliterals.h>
 #include <tools/scripttools.h>
+#include <tools/setupprojectparameters.h>
 #include <tools/stlutils.h>
 #include <tools/stringconstants.h>
 
@@ -63,9 +64,11 @@
 namespace qbs {
 namespace Internal {
 
-ModuleProviderLoader::ModuleProviderLoader(ItemReader *reader, Evaluator *evaluator,
-                                           ProbesResolver *probesResolver, Logger &logger)
-    : m_reader(reader)
+ModuleProviderLoader::ModuleProviderLoader(
+    const SetupProjectParameters &parameters, ItemReader &reader, Evaluator &evaluator,
+    ProbesResolver &probesResolver, Logger &logger)
+    : m_parameters(parameters)
+    , m_reader(reader)
     , m_evaluator(evaluator)
     , m_probesResolver(probesResolver)
     , m_logger(logger)
@@ -73,7 +76,7 @@ ModuleProviderLoader::ModuleProviderLoader(ItemReader *reader, Evaluator *evalua
 }
 
 ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModuleProviders(
-        ProductContext &productContext,
+        const ProductContext &productContext,
         const CodeLocation &dependsItemLocation,
         const QualifiedId &moduleName,
         FallbackMode fallbackMode)
@@ -82,7 +85,7 @@ ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModulePr
     std::vector<Provider> providersToRun;
     qCDebug(lcModuleLoader) << "Module" << moduleName.toString()
                             << "not found, checking for module providers";
-    const auto providerNames = getModuleProviders(productContext.item);
+    const auto providerNames = getModuleProviders(productContext.productItem);
     if (providerNames) {
         providersToRun = transformed<std::vector<Provider>>(*providerNames, [](const auto &name) {
             return Provider{name, ModuleProviderLookup::Named}; });
@@ -109,7 +112,7 @@ ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModulePr
 }
 
 ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModuleProvidersHelper(
-        ProductContext &product,
+        const ProductContext &product,
         const CodeLocation &dependsItemLocation,
         const std::vector<Provider> &providers)
 {
@@ -117,9 +120,11 @@ ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModulePr
         return {};
     QStringList allSearchPaths;
     ModuleProviderResult result;
+    result.providerConfig = product.providerConfig ? *product.providerConfig
+                                                   : getModuleProviderConfig(product);
     const auto qbsModule = evaluateQbsModule(product);
     for (const auto &[name, lookupType] : providers) {
-        const QVariantMap config = getModuleProviderConfig(product).value(name.toString()).toMap();
+        const QVariantMap config = result.providerConfig.value(name.toString()).toMap();
         ModuleProviderInfo &info = m_storedModuleProviderInfo.providers[
             {name.toString(), config, qbsModule, int(lookupType)}];
         const bool fromCache = !info.name.isEmpty();
@@ -129,8 +134,10 @@ ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModulePr
             info.providerFile = findModuleProviderFile(name, lookupType);
             if (!info.providerFile.isEmpty()) {
                 qCDebug(lcModuleLoader) << "Running provider" << name << "at" << info.providerFile;
-                info.searchPaths = evaluateModuleProvider(
-                        product, dependsItemLocation, name, info.providerFile, config, qbsModule);
+                const auto evalResult = evaluateModuleProvider(
+                    product, dependsItemLocation, name, info.providerFile, config, qbsModule);
+                info.searchPaths = evalResult.first;
+                result.probes << evalResult.second;
                 info.transientOutput = m_parameters.dryRun();
             }
         }
@@ -156,20 +163,17 @@ ModuleProviderLoader::ModuleProviderResult ModuleProviderLoader::executeModulePr
     if (allSearchPaths.isEmpty())
         return result;
 
-    m_reader->pushExtraSearchPaths(allSearchPaths);
+    m_reader.pushExtraSearchPaths(allSearchPaths);
     result.providerAddedSearchPaths = true;
 
     return result;
 }
 
-QVariantMap ModuleProviderLoader::getModuleProviderConfig(
-        ProductContext &product)
+QVariantMap ModuleProviderLoader::getModuleProviderConfig(const ProductContext &product)
 {
-    if (product.theModuleProviderConfig)
-        return *product.theModuleProviderConfig;
     QVariantMap providerConfig;
     const ItemValueConstPtr configItemValue =
-            product.item->itemProperty(StringConstants::moduleProviders());
+        product.productItem->itemProperty(StringConstants::moduleProviders());
     if (configItemValue) {
         const std::function<void(const Item *, QualifiedId)> collectMap
                 = [this, &providerConfig, &collectMap](const Item *item, const QualifiedId &name) {
@@ -184,9 +188,9 @@ QVariantMap ModuleProviderLoader::getModuleProviderConfig(
                     continue;
                 }
                 case Value::JSSourceValueType: {
-                    const ScopedJsValue sv(m_evaluator->engine()->context(),
-                                           m_evaluator->value(item, it.key()));
-                    value = getJsVariant(m_evaluator->engine()->context(), sv);
+                    const ScopedJsValue sv(m_evaluator.engine()->context(),
+                                           m_evaluator.value(item, it.key()));
+                    value = getJsVariant(m_evaluator.engine()->context(), sv);
                     break;
                 }
                 case Value::VariantValueType:
@@ -198,7 +202,7 @@ QVariantMap ModuleProviderLoader::getModuleProviderConfig(
                 providerConfig.insert(name.toString(), m);
             }
         };
-        configItemValue->item()->setScope(product.item);
+        configItemValue->item()->setScope(product.productItem);
         collectMap(configItemValue->item(), QualifiedId());
     }
     for (auto it = product.moduleProperties.begin(); it != product.moduleProperties.end(); ++it) {
@@ -215,14 +219,14 @@ QVariantMap ModuleProviderLoader::getModuleProviderConfig(
         }
         providerConfig.insert(provider, currentMapForProvider);
     }
-    return *(product.theModuleProviderConfig = std::move(providerConfig));
+    return providerConfig;
 }
 
 std::optional<std::vector<QualifiedId>> ModuleProviderLoader::getModuleProviders(Item *item)
 {
     while (item) {
         const auto providers =
-                m_evaluator->optionalStringListValue(item, StringConstants::qbsModuleProviders());
+            m_evaluator.optionalStringListValue(item, StringConstants::qbsModuleProviders());
         if (providers) {
             return transformed<std::vector<QualifiedId>>(*providers, [](const auto &provider) {
                 return QualifiedId::fromString(provider); });
@@ -235,7 +239,7 @@ std::optional<std::vector<QualifiedId>> ModuleProviderLoader::getModuleProviders
 QString ModuleProviderLoader::findModuleProviderFile(
         const QualifiedId &name, ModuleProviderLookup lookupType)
 {
-    for (const QString &path : m_reader->allSearchPaths()) {
+    for (const QString &path : m_reader.allSearchPaths()) {
         QString fullPath = FileInfo::resolvePath(path, QStringLiteral("module-providers"));
         switch (lookupType) {
         case ModuleProviderLookup::Named: {
@@ -265,19 +269,19 @@ QString ModuleProviderLoader::findModuleProviderFile(
     return {};
 }
 
-QVariantMap ModuleProviderLoader::evaluateQbsModule(ProductContext &product) const
+QVariantMap ModuleProviderLoader::evaluateQbsModule(const ProductContext &product) const
 {
     const QString properties[] = {
         QStringLiteral("sysroot"),
         QStringLiteral("toolchain"),
     };
     const auto qbsItemValue = std::static_pointer_cast<ItemValue>(
-        product.item->property(StringConstants::qbsModule()));
+        product.productItem->property(StringConstants::qbsModule()));
     QVariantMap result;
     for (const auto &property : properties) {
-        const ScopedJsValue val(m_evaluator->engine()->context(),
-                                m_evaluator->value(qbsItemValue->item(), property));
-        auto value = getJsVariant(m_evaluator->engine()->context(), val);
+        const ScopedJsValue val(m_evaluator.engine()->context(),
+                                m_evaluator.value(qbsItemValue->item(), property));
+        auto value = getJsVariant(m_evaluator.engine()->context(), val);
         if (!value.isValid())
             continue;
 
@@ -291,26 +295,24 @@ QVariantMap ModuleProviderLoader::evaluateQbsModule(ProductContext &product) con
     return result;
 }
 
-Item *ModuleProviderLoader::createProviderScope(
-    ProductContext &product, const QVariantMap &qbsModule)
+Item *ModuleProviderLoader::createProviderScope(const ProductContext &product, const QVariantMap &qbsModule)
 {
     const auto qbsItemValue = std::static_pointer_cast<ItemValue>(
-        product.item->property(StringConstants::qbsModule()));
+        product.productItem->property(StringConstants::qbsModule()));
 
-    Item *fakeQbsModule = Item::create(product.item->pool(), ItemType::Scope);
+    Item *fakeQbsModule = Item::create(product.productItem->pool(), ItemType::Scope);
 
     for (auto it = qbsModule.begin(), end = qbsModule.end(); it != end; ++it) {
         fakeQbsModule->setProperty(it.key(), VariantValue::create(it.value()));
     }
 
-    Item *scope = Item::create(product.item->pool(), ItemType::Scope);
+    Item *scope = Item::create(product.productItem->pool(), ItemType::Scope);
     scope->setFile(qbsItemValue->item()->file());
     scope->setProperty(StringConstants::qbsModule(), ItemValue::create(fakeQbsModule));
     return scope;
 }
 
-QStringList ModuleProviderLoader::evaluateModuleProvider(
-        ProductContext &product,
+std::pair<QStringList, std::vector<ProbeConstPtr> > ModuleProviderLoader::evaluateModuleProvider(const ProductContext &product,
         const CodeLocation &dependsItemLocation,
         const QualifiedId &name,
         const QString &providerFile,
@@ -325,7 +327,7 @@ QStringList ModuleProviderLoader::evaluateModuleProvider(
     }
     m_tempQbsFiles << dummyItemFile.fileName();
     qCDebug(lcModuleLoader) << "Instantiating module provider at" << providerFile;
-    const QString projectBuildDir = product.project->item->variantProperty(
+    const QString projectBuildDir = product.projectItem->variantProperty(
                 StringConstants::buildDirectoryProperty())->value().toString();
     const QString searchPathBaseDir = ModuleProviderInfo::outputDirPath(projectBuildDir, name);
 
@@ -350,8 +352,8 @@ QStringList ModuleProviderLoader::evaluateModuleProvider(
            << endl;
     stream << "}" << endl;
     stream.flush();
-    Item * const providerItem = m_reader->setupItemFromFile(
-        dummyItemFile.fileName(), dependsItemLocation, *m_evaluator);
+    Item * const providerItem = m_reader.setupItemFromFile(
+        dummyItemFile.fileName(), dependsItemLocation, m_evaluator);
     if (providerItem->type() != ItemType::ModuleProvider) {
         throw ErrorInfo(Tr::tr("File '%1' declares an item of type '%2', "
                                "but '%3' was expected.")
@@ -360,14 +362,13 @@ QStringList ModuleProviderLoader::evaluateModuleProvider(
     }
 
     providerItem->setScope(createProviderScope(product, qbsModule));
-
     providerItem->overrideProperties(moduleConfig, name, m_parameters, m_logger);
+    std::vector<ProbeConstPtr> probes = m_probesResolver.resolveProbes(
+        {product.name, product.uniqueName}, providerItem);
 
-    product.info.probes << m_probesResolver->resolveProbes({product.name, product.uniqueName()},
-                                                           providerItem);
-
-    EvalContextSwitcher contextSwitcher(m_evaluator->engine(), EvalContext::ModuleProvider);
-    return m_evaluator->stringListValue(providerItem, QStringLiteral("searchPaths"));
+    EvalContextSwitcher contextSwitcher(m_evaluator.engine(), EvalContext::ModuleProvider);
+    return std::make_pair(m_evaluator.stringListValue(providerItem, QStringLiteral("searchPaths")),
+                          std::move(probes));
 }
 
 } // namespace Internal
diff --git a/src/lib/corelib/language/moduleproviderloader.h b/src/lib/corelib/language/moduleproviderloader.h
index 0e4198092..4216b4430 100644
--- a/src/lib/corelib/language/moduleproviderloader.h
+++ b/src/lib/corelib/language/moduleproviderloader.h
@@ -41,25 +41,32 @@
 #ifndef MODULEPROVIDERLOADER_H
 #define MODULEPROVIDERLOADER_H
 
-#include "projecttreebuilder.h"
+#include "forward_decls.h"
 #include "moduleproviderinfo.h"
 
-#include <tools/setupprojectparameters.h>
-
 #include <QtCore/qmap.h>
 #include <QtCore/qvariant.h>
 
+#include <optional>
+#include <vector>
+
 namespace qbs {
+class SetupProjectParameters;
 namespace Internal {
+class Evaluator;
+class Item;
 class ItemReader;
 class Logger;
 class ProbesResolver;
 
+enum class FallbackMode { Enabled, Disabled };
+
 class ModuleProviderLoader
 {
 public:
-    explicit ModuleProviderLoader(ItemReader *itemReader, Evaluator *evaluator,
-                                  ProbesResolver *probesResolver, Logger &logger);
+    explicit ModuleProviderLoader(const SetupProjectParameters &parameters, ItemReader &itemReader,
+                                  Evaluator &evaluator, ProbesResolver &probesResolver,
+                                  Logger &logger);
 
     enum class ModuleProviderLookup { Scoped, Named, Fallback };
 
@@ -74,6 +81,9 @@ public:
         ModuleProviderResult() = default;
         ModuleProviderResult(bool ran, bool added)
             : providerFound(ran), providerAddedSearchPaths(added) {}
+
+        std::vector<ProbeConstPtr> probes;
+        QVariantMap providerConfig;
         bool providerFound = false;
         bool providerAddedSearchPaths = false;
     };
@@ -88,33 +98,36 @@ public:
         m_storedModuleProviderInfo = std::move(moduleProviderInfo);
     }
 
-    void setProjectParameters(SetupProjectParameters parameters)
-    {
-        m_parameters = std::move(parameters);
-    }
-
     const Set<QString> &tempQbsFiles() const { return m_tempQbsFiles; }
 
+    struct ProductContext {
+        Item * const productItem;
+        Item * const projectItem;
+        const QString &name;
+        const QString &uniqueName;
+        const QVariantMap &moduleProperties;
+        const std::optional<QVariantMap> providerConfig;
+    };
     ModuleProviderResult executeModuleProviders(
-            ProductContext &productContext,
+            const ProductContext &productContext,
             const CodeLocation &dependsItemLocation,
             const QualifiedId &moduleName,
             FallbackMode fallbackMode);
 
 private:
     ModuleProviderResult executeModuleProvidersHelper(
-            ProductContext &product,
+            const ProductContext &product,
             const CodeLocation &dependsItemLocation,
             const std::vector<Provider> &providers);
-    QVariantMap getModuleProviderConfig(ProductContext &product);
+    QVariantMap getModuleProviderConfig(const ProductContext &product);
 
     std::optional<std::vector<QualifiedId>> getModuleProviders(Item *item);
 
     QString findModuleProviderFile(const QualifiedId &name, ModuleProviderLookup lookupType);
-    QVariantMap evaluateQbsModule(ProductContext &product) const;
-    Item *createProviderScope(ProductContext &product, const QVariantMap &qbsModule);
-    QStringList evaluateModuleProvider(
-            ProductContext &product,
+    QVariantMap evaluateQbsModule(const ProductContext &product) const;
+    Item *createProviderScope(const ProductContext &product, const QVariantMap &qbsModule);
+    std::pair<QStringList, std::vector<ProbeConstPtr>> evaluateModuleProvider(
+            const ProductContext &product,
             const CodeLocation &location,
             const QualifiedId &name,
             const QString &providerFile,
@@ -122,11 +135,10 @@ private:
             const QVariantMap &qbsModule);
 
 private:
-    ItemReader *const m_reader{nullptr};
-    Evaluator *const m_evaluator{nullptr};
-    ProbesResolver *const m_probesResolver{nullptr};
-
-    SetupProjectParameters m_parameters;
+    const SetupProjectParameters &m_parameters;
+    ItemReader &m_reader;
+    Evaluator &m_evaluator;
+    ProbesResolver &m_probesResolver;
     Logger &m_logger;
     StoredModuleProviderInfo m_storedModuleProviderInfo;
     Set<QString> m_tempQbsFiles;
diff --git a/src/lib/corelib/language/projecttreebuilder.cpp b/src/lib/corelib/language/projecttreebuilder.cpp
index 9910325ac..66cf34e81 100644
--- a/src/lib/corelib/language/projecttreebuilder.cpp
+++ b/src/lib/corelib/language/projecttreebuilder.cpp
@@ -49,7 +49,6 @@
 #include "moduleinstantiator.h"
 #include "moduleloader.h"
 #include "modulepropertymerger.h"
-#include "moduleproviderloader.h"
 #include "probesresolver.h"
 #include "productitemmultiplexer.h"
 #include "scriptengine.h"
@@ -185,7 +184,7 @@ public:
     TimingData timingData;
     ItemReader reader{logger};
     ProbesResolver probesResolver{parameters, evaluator, logger};
-    ModuleProviderLoader moduleProviderLoader{&reader, &evaluator, &probesResolver, logger};
+    ModuleProviderLoader moduleProviderLoader{parameters, reader, evaluator, probesResolver, logger};
     ModuleLoader moduleLoader{parameters, reader, evaluator, logger};
     ModulePropertyMerger propertyMerger{parameters, evaluator, logger};
     ModuleInstantiator moduleInstantiator{parameters, itemPool, propertyMerger, logger};
@@ -235,7 +234,6 @@ ProjectTreeBuilder::ProjectTreeBuilder(const SetupProjectParameters &parameters,
 {
     d->reader.setDeprecationWarningMode(parameters.deprecationWarningMode());
     d->reader.setEnableTiming(parameters.logElapsedTime());
-    d->moduleProviderLoader.setProjectParameters(parameters);
     d->settings = std::make_unique<Settings>(parameters.settingsDirectory());
 }
 
@@ -2021,10 +2019,15 @@ Item *ProjectTreeBuilder::Private::searchAndLoadModuleFile(
         AccumulatingTimer providersTimer(
             parameters.logElapsedTime() ? &timingData.moduleProviders : nullptr);
         auto result = moduleProviderLoader.executeModuleProviders(
-            *productContext,
+            {productContext->item, productContext->project->item, productContext->name,
+             productContext->uniqueName(), productContext->moduleProperties,
+             productContext->theModuleProviderConfig},
             dependsItemLocation,
             moduleName,
             fallbackMode);
+        productContext->info.probes << result.probes;
+        if (!productContext->theModuleProviderConfig)
+            productContext->theModuleProviderConfig = result.providerConfig;
         if (result.providerAddedSearchPaths) {
             qCDebug(lcModuleLoader) << "Re-checking for module" << moduleName.toString()
                                     << "with newly added search paths from module provider";
diff --git a/src/lib/corelib/language/projecttreebuilder.h b/src/lib/corelib/language/projecttreebuilder.h
index db2d621c5..76a4df372 100644
--- a/src/lib/corelib/language/projecttreebuilder.h
+++ b/src/lib/corelib/language/projecttreebuilder.h
@@ -42,6 +42,7 @@
 #include "forward_decls.h"
 #include "filetags.h"
 #include "moduleproviderinfo.h"
+#include "moduleproviderloader.h"
 #include "qualifiedid.h"
 
 #include <tools/filetime.h>
@@ -74,8 +75,6 @@ public:
 
 class ProjectContext;
 
-enum class FallbackMode { Enabled, Disabled };
-
 using ModulePropertiesPerGroup = std::unordered_map<const Item *, QualifiedIdSet>;
 
 // TODO: This class only needs to be known inside the ProjectResolver; no need to