Utils: Pass MacroExpander to ArgumentsAspect constructor

In the past we wanted to keep the aspect constructor simple but
it turned out that exceptions were needed and accumulating, so those
are likely here to stay.

By passing also the MacroExpander to the ArgumentsAspect constructor
allows other single-purpose warts like the ProjectConfiguration::doPostInit()
machinery can be removed.

Change-Id: I148b0ca1ab0740270eecd0d3134620de65a86d4f
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>

src/libs/utils/aspects.cpp

@@ -2392,7 +2392,7 @@ void AspectContainer::forEachAspect(const std::function<void(BaseAspect *)> &run
     }
 }
 
-BaseAspect::Data::Ptr BaseAspect::extractData(const MacroExpander *expander) const
+BaseAspect::Data::Ptr BaseAspect::extractData() const
 {
     QTC_ASSERT(d->m_dataCreator, return {});
     Data *data = d->m_dataCreator();
@@ -2400,7 +2400,7 @@ BaseAspect::Data::Ptr BaseAspect::extractData(const MacroExpander *expander) con
     data->m_id = id();
     data->m_cloner = d->m_dataCloner;
     for (const DataExtractor &extractor : d->m_dataExtractors)
-        extractor(data, expander);
+        extractor(data);
     return Data::Ptr(data);
 }
 

src/libs/utils/aspects.h

@@ -178,9 +178,9 @@ public:
 
     using DataCreator = std::function<Data *()>;
     using DataCloner = std::function<Data *(const Data *)>;
-    using DataExtractor = std::function<void(Data *data, const MacroExpander *expander)>;
+    using DataExtractor = std::function<void(Data *data)>;
 
-    Data::Ptr extractData(const MacroExpander *expander) const;
+    Data::Ptr extractData() const;
 
 signals:
     void changed();
@@ -205,26 +205,11 @@ protected:
         setDataClonerHelper([](const Data *data) {
             return new DataClass(*static_cast<const DataClass *>(data));
         });
-        addDataExtractorHelper([aspect, p, q](Data *data, const MacroExpander *) {
+        addDataExtractorHelper([aspect, p, q](Data *data) {
             static_cast<DataClass *>(data)->*q = (aspect->*p)();
         });
     }
 
-    template <typename AspectClass, typename DataClass, typename Type>
-    void addDataExtractor(AspectClass *aspect,
-                          Type(AspectClass::*p)(const MacroExpander *) const,
-                          Type DataClass::*q) {
-        setDataCreatorHelper([] {
-            return new DataClass;
-        });
-        setDataClonerHelper([](const Data *data) {
-            return new DataClass(*static_cast<const DataClass *>(data));
-        });
-        addDataExtractorHelper([aspect, p, q](Data *data, const MacroExpander *expander) {
-            static_cast<DataClass *>(data)->*q = (aspect->*p)(expander);
-        });
-    }
-
     template <class Widget, typename ...Args>
     Widget *createSubWidget(Args && ...args) {
         auto w = new Widget(args...);

src/plugins/android/androidrunconfiguration.cpp

@@ -75,15 +75,14 @@ AndroidRunConfiguration::AndroidRunConfiguration(Target *target, Utils::Id id)
     auto envAspect = addAspect<EnvironmentAspect>();
     envAspect->addSupportedBaseEnvironment(tr("Clean Environment"), {});
 
-    auto extraAppArgsAspect = addAspect<ArgumentsAspect>();
+    auto extraAppArgsAspect = addAspect<ArgumentsAspect>(macroExpander());
 
-    connect(extraAppArgsAspect, &BaseAspect::changed,
+    connect(extraAppArgsAspect, &BaseAspect::changed, this, [target, extraAppArgsAspect] {
-            this, [target, extraAppArgsAspect]() {
         if (target->buildConfigurations().first()->buildType() == BuildConfiguration::BuildType::Release) {
             const QString buildKey = target->activeBuildKey();
             target->buildSystem()->setExtraData(buildKey,
                                             Android::Constants::AndroidApplicationArgs,
-                                            extraAppArgsAspect->arguments(target->macroExpander()));
+                                            extraAppArgsAspect->arguments());
         }
     });
 

src/plugins/baremetal/baremetalrunconfiguration.cpp

@@ -45,14 +45,14 @@ class BareMetalRunConfiguration final : public RunConfiguration
     Q_DECLARE_TR_FUNCTIONS(BareMetal::Internal::BareMetalRunConfiguration)
 
 public:
-    explicit BareMetalRunConfiguration(Target *target, Utils::Id id)
+    explicit BareMetalRunConfiguration(Target *target, Id id)
         : RunConfiguration(target, id)
     {
         const auto exeAspect = addAspect<ExecutableAspect>(target);
         exeAspect->setDisplayStyle(StringAspect::LabelDisplay);
         exeAspect->setPlaceHolderText(tr("Unknown"));
 
-        addAspect<ArgumentsAspect>();
+        addAspect<ArgumentsAspect>(macroExpander());
         addAspect<WorkingDirectoryAspect>(nullptr);
 
         setUpdater([this, exeAspect] {
@@ -79,7 +79,7 @@ public:
         exeAspect->setHistoryCompleter("BareMetal.CustomRunConfig.History");
         exeAspect->setExpectedKind(PathChooser::Any);
 
-        addAspect<ArgumentsAspect>();
+        addAspect<ArgumentsAspect>(macroExpander());
         addAspect<WorkingDirectoryAspect>(nullptr);
 
         setDefaultDisplayName(RunConfigurationFactory::decoratedTargetName(tr("Custom Executable"), target));

src/plugins/boot2qt/qdbrunconfiguration.cpp

@@ -56,9 +56,9 @@ public:
         auto exeAspect = rc->aspect<ExecutableAspect>();
         auto argumentsAspect = rc->aspect<ArgumentsAspect>();
 
-        auto updateCommandLine = [this, rc, exeAspect, argumentsAspect] {
+        auto updateCommandLine = [this, exeAspect, argumentsAspect] {
             const QString usedExecutable = exeAspect->executable().toString();
-            const QString args = argumentsAspect->arguments(rc->macroExpander());
+            const QString args = argumentsAspect->arguments();
             setValue(QString(Constants::AppcontrollerFilepath)
                      + ' ' + usedExecutable + ' ' + args);
         };
@@ -101,7 +101,7 @@ QdbRunConfiguration::QdbRunConfiguration(Target *target, Id id)
 
     auto envAspect = addAspect<RemoteLinux::RemoteLinuxEnvironmentAspect>(target);
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     addAspect<FullCommandLineAspect>(this);
 

src/plugins/ios/iosrunconfiguration.cpp

@@ -83,7 +83,7 @@ IosRunConfiguration::IosRunConfiguration(Target *target, Id id)
     auto executableAspect = addAspect<ExecutableAspect>(target);
     executableAspect->setDisplayStyle(StringAspect::LabelDisplay);
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
 
     m_deviceTypeAspect = addAspect<IosDeviceTypeAspect>(this);
 

src/plugins/mesonprojectmanager/project/mesonrunconfiguration.cpp

@@ -45,7 +45,7 @@ MesonRunConfiguration::MesonRunConfiguration(ProjectExplorer::Target *target, Ut
     auto envAspect = addAspect<ProjectExplorer::LocalEnvironmentAspect>(target);
 
     addAspect<ProjectExplorer::ExecutableAspect>(target);
-    addAspect<ProjectExplorer::ArgumentsAspect>();
+    addAspect<ProjectExplorer::ArgumentsAspect>(macroExpander());
     addAspect<ProjectExplorer::WorkingDirectoryAspect>(envAspect);
     addAspect<ProjectExplorer::TerminalAspect>();
 

src/plugins/nim/project/nimblebuildstep.cpp

@@ -55,14 +55,13 @@ private:
 NimbleBuildStep::NimbleBuildStep(BuildStepList *parentList, Id id)
     : AbstractProcessStep(parentList, id)
 {
-    auto arguments = addAspect<ArgumentsAspect>();
+    auto arguments = addAspect<ArgumentsAspect>(macroExpander());
     arguments->setSettingsKey(Constants::C_NIMBLEBUILDSTEP_ARGUMENTS);
     arguments->setResetter([this] { return defaultArguments(); });
     arguments->setArguments(defaultArguments());
 
     setCommandLineProvider([this, arguments] {
-        return CommandLine(Nim::nimblePathFromKit(kit()),
+        return CommandLine(Nim::nimblePathFromKit(kit()), {"build", arguments->arguments()});
-                           {"build", arguments->arguments(macroExpander())});
     });
     setWorkingDirectoryProvider([this] { return project()->projectDirectory(); });
     setEnvironmentModifier([this](Environment &env) {

src/plugins/nim/project/nimblerunconfiguration.cpp

@@ -53,7 +53,7 @@ public:
     {
         auto envAspect = addAspect<LocalEnvironmentAspect>(target);
         addAspect<ExecutableAspect>(target);
-        addAspect<ArgumentsAspect>();
+        addAspect<ArgumentsAspect>(macroExpander());
         addAspect<WorkingDirectoryAspect>(envAspect);
         addAspect<TerminalAspect>();
 
@@ -90,7 +90,7 @@ public:
         : RunConfiguration(target, id)
     {
         addAspect<ExecutableAspect>(target)->setExecutable(Nim::nimblePathFromKit(target->kit()));
-        addAspect<ArgumentsAspect>()->setArguments("test");
+        addAspect<ArgumentsAspect>(macroExpander())->setArguments("test");
         addAspect<WorkingDirectoryAspect>(nullptr)->setDefaultWorkingDirectory(project()->projectDirectory());
         addAspect<TerminalAspect>();
 

src/plugins/nim/project/nimrunconfiguration.cpp

@@ -51,7 +51,7 @@ public:
     {
         auto envAspect = addAspect<LocalEnvironmentAspect>(target);
         addAspect<ExecutableAspect>(target);
-        addAspect<ArgumentsAspect>();
+        addAspect<ArgumentsAspect>(macroExpander());
         addAspect<WorkingDirectoryAspect>(envAspect);
         addAspect<TerminalAspect>();
 

src/plugins/projectexplorer/customexecutablerunconfiguration.cpp

@@ -53,7 +53,7 @@ CustomExecutableRunConfiguration::CustomExecutableRunConfiguration(Target *targe
     exeAspect->setExpectedKind(PathChooser::ExistingCommand);
     exeAspect->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(envAspect->environment()));
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     addAspect<TerminalAspect>();
 

src/plugins/projectexplorer/desktoprunconfiguration.cpp

@@ -69,7 +69,7 @@ DesktopRunConfiguration::DesktopRunConfiguration(Target *target, Id id, Kind kin
     auto envAspect = addAspect<LocalEnvironmentAspect>(target);
 
     addAspect<ExecutableAspect>(target);
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     addAspect<TerminalAspect>();
 

src/plugins/projectexplorer/projectconfiguration.cpp

@@ -135,17 +135,11 @@ bool ProjectConfiguration::fromMap(const QVariantMap &map)
     return true;
 }
 
-Utils::BaseAspect *ProjectConfiguration::aspect(Utils::Id id) const
+BaseAspect *ProjectConfiguration::aspect(Id id) const
 {
     return m_aspects.aspect(id);
 }
 
-void ProjectConfiguration::doPostInit()
-{
-    for (const std::function<void()> &postInit : qAsConst(m_postInit))
-        postInit();
-}
-
 FilePath ProjectConfiguration::mapFromBuildDeviceToGlobalPath(const FilePath &path) const
 {
     IDevice::ConstPtr dev = BuildDeviceKitAspect::device(kit());
@@ -153,9 +147,9 @@ FilePath ProjectConfiguration::mapFromBuildDeviceToGlobalPath(const FilePath &pa
     return dev->mapToGlobalPath(path);
 }
 
-Utils::Id ProjectExplorer::idFromMap(const QVariantMap &map)
+Id ProjectExplorer::idFromMap(const QVariantMap &map)
 {
-    return Utils::Id::fromSetting(map.value(QLatin1String(CONFIGURATION_ID_KEY)));
+    return Id::fromSetting(map.value(QLatin1String(CONFIGURATION_ID_KEY)));
 }
 
 QString ProjectConfiguration::expandedDisplayName() const

src/plugins/projectexplorer/projectconfiguration.h

@@ -89,9 +89,6 @@ public:
 
     Utils::FilePath mapFromBuildDeviceToGlobalPath(const Utils::FilePath &path) const;
 
-    void addPostInit(const std::function<void()> &fixup) { m_postInit.append(fixup); }
-    void doPostInit();
-
 signals:
     void displayNameChanged();
     void toolTipChanged();
@@ -104,7 +101,6 @@ private:
     const Utils::Id m_id;
     Utils::DisplayName m_displayName;
     QString m_toolTip;
-    QList<std::function<void()>> m_postInit;
 };
 
 // helper function:

src/plugins/projectexplorer/runconfiguration.cpp

@@ -209,21 +209,16 @@ RunConfiguration::RunConfiguration(Target *target, Utils::Id id)
                                      [this] { return commandLine().executable(); });
 
 
-    m_commandLineGetter = [target, this] {
+    m_commandLineGetter = [this] {
         FilePath executable;
         if (const auto executableAspect = aspect<ExecutableAspect>())
             executable = executableAspect->executable();
         QString arguments;
         if (const auto argumentsAspect = aspect<ArgumentsAspect>())
-            arguments = argumentsAspect->arguments(macroExpander());
+            arguments = argumentsAspect->arguments();
 
         return CommandLine{executable, arguments, CommandLine::Raw};
     };
-
-    addPostInit([this] {
-        if (const auto wdAspect = aspect<WorkingDirectoryAspect>())
-            wdAspect->setMacroExpander(&m_expander);
-    });
 }
 
 RunConfiguration::~RunConfiguration() = default;
@@ -280,7 +275,7 @@ AspectContainerData RunConfiguration::aspectData() const
 {
     AspectContainerData data;
     for (BaseAspect *aspect : m_aspects)
-        data.append(aspect->extractData(&m_expander));
+        data.append(aspect->extractData());
     return data;
 }
 
@@ -585,7 +580,6 @@ RunConfiguration *RunConfigurationFactory::create(Target *target) const
     for (const RunConfiguration::AspectFactory &factory : theAspectFactories)
         rc->m_aspects.registerAspect(factory(target));
 
-    rc->doPostInit();
     return rc;
 }
 

src/plugins/projectexplorer/runconfigurationaspects.cpp

@@ -298,11 +298,6 @@ void WorkingDirectoryAspect::setDefaultWorkingDirectory(const FilePath &defaultW
     }
 }
 
-void WorkingDirectoryAspect::setMacroExpander(MacroExpander *macroExpander)
-{
-    m_macroExpander = macroExpander;
-}
-
 /*!
     \internal
 */
@@ -320,7 +315,8 @@ PathChooser *WorkingDirectoryAspect::pathChooser() const
     arguments for an executable.
 */
 
-ArgumentsAspect::ArgumentsAspect()
+ArgumentsAspect::ArgumentsAspect(const MacroExpander *macroExpander)
+    : m_macroExpander(macroExpander)
 {
     setDisplayName(tr("Arguments"));
     setId("ArgumentsAspect");
@@ -336,14 +332,14 @@ ArgumentsAspect::ArgumentsAspect()
 
     Macros in the value are expanded using \a expander.
 */
-QString ArgumentsAspect::arguments(const MacroExpander *expander) const
+QString ArgumentsAspect::arguments() const
 {
-    QTC_ASSERT(expander, return m_arguments);
+    QTC_ASSERT(m_macroExpander, return m_arguments);
     if (m_currentlyExpanding)
         return m_arguments;
 
     m_currentlyExpanding = true;
-    const QString expanded = expander->expandProcessArgs(m_arguments);
+    const QString expanded = m_macroExpander->expandProcessArgs(m_arguments);
     m_currentlyExpanding = false;
     return expanded;
 }

src/plugins/projectexplorer/runconfigurationaspects.h

@@ -89,7 +89,6 @@ public:
     Utils::FilePath defaultWorkingDirectory() const;
     Utils::FilePath unexpandedWorkingDirectory() const;
     void setDefaultWorkingDirectory(const Utils::FilePath &defaultWorkingDirectory);
-    void setMacroExpander(Utils::MacroExpander *macroExpander);
     Utils::PathChooser *pathChooser() const;
 
 private:
@@ -111,11 +110,11 @@ class PROJECTEXPLORER_EXPORT ArgumentsAspect : public Utils::BaseAspect
     Q_OBJECT
 
 public:
-    ArgumentsAspect();
+    explicit ArgumentsAspect(const Utils::MacroExpander *macroExpander);
 
     void addToLayout(Utils::LayoutBuilder &builder) override;
 
-    QString arguments(const Utils::MacroExpander *expander) const;
+    QString arguments() const;
     QString unexpandedArguments() const;
 
     void setArguments(const QString &arguments);
@@ -143,6 +142,7 @@ private:
     bool m_multiLine = false;
     mutable bool m_currentlyExpanding = false;
     std::function<QString()> m_resetter;
+    const Utils::MacroExpander *m_macroExpander = nullptr;
 };
 
 class PROJECTEXPLORER_EXPORT UseLibraryPathsAspect : public Utils::BoolAspect

src/plugins/python/pythonrunconfiguration.cpp

@@ -176,17 +176,17 @@ public:
 
         addAspect<LocalEnvironmentAspect>(target);
 
-        auto argumentsAspect = addAspect<ArgumentsAspect>();
+        auto argumentsAspect = addAspect<ArgumentsAspect>(macroExpander());
 
         addAspect<WorkingDirectoryAspect>(nullptr);
         addAspect<TerminalAspect>();
 
-        setCommandLineGetter([this, bufferedAspect, interpreterAspect, argumentsAspect, scriptAspect] {
+        setCommandLineGetter([bufferedAspect, interpreterAspect, argumentsAspect, scriptAspect] {
             CommandLine cmd{interpreterAspect->currentInterpreter().command};
             if (!bufferedAspect->value())
                 cmd.addArg("-u");
             cmd.addArg(scriptAspect->filePath().fileName());
-            cmd.addArgs(argumentsAspect->arguments(macroExpander()), CommandLine::Raw);
+            cmd.addArgs(argumentsAspect->arguments(), CommandLine::Raw);
             return cmd;
         });
 

src/plugins/qmakeprojectmanager/qmakestep.cpp

@@ -89,7 +89,7 @@ QMakeStep::QMakeStep(BuildStepList *bsl, Id id)
     m_buildType->addOption(tr("Debug"));
     m_buildType->addOption(tr("Release"));
 
-    m_userArgs = addAspect<ArgumentsAspect>();
+    m_userArgs = addAspect<ArgumentsAspect>(macroExpander());
     m_userArgs->setSettingsKey(QMAKE_ARGUMENTS_KEY);
     m_userArgs->setLabelText(tr("Additional arguments:"));
 
@@ -469,7 +469,7 @@ QStringList QMakeStep::parserArguments()
 
 QString QMakeStep::userArguments() const
 {
-    return m_userArgs->arguments(macroExpander());
+    return m_userArgs->arguments();
 }
 
 QString QMakeStep::mkspec() const

src/plugins/qmlprojectmanager/qmlprojectrunconfiguration.cpp

@@ -98,7 +98,7 @@ QmlProjectRunConfiguration::QmlProjectRunConfiguration(Target *target, Id id)
     m_qmlViewerAspect->setDisplayStyle(StringAspect::LineEditDisplay);
     m_qmlViewerAspect->setHistoryCompleter("QmlProjectManager.viewer.history");
 
-    auto argumentAspect = addAspect<ArgumentsAspect>();
+    auto argumentAspect = addAspect<ArgumentsAspect>(macroExpander());
     argumentAspect->setSettingsKey(Constants::QML_VIEWER_ARGUMENTS_KEY);
 
     setCommandLineGetter([this] {
@@ -206,7 +206,7 @@ FilePath QmlProjectRunConfiguration::qmlRuntimeFilePath() const
 QString QmlProjectRunConfiguration::commandLineArguments() const
 {
     // arguments in .user file
-    QString args = aspect<ArgumentsAspect>()->arguments(macroExpander());
+    QString args = aspect<ArgumentsAspect>()->arguments();
     const IDevice::ConstPtr device = DeviceKitAspect::device(kit());
     const OsType osType = device ? device->osType() : HostOsInfo::hostOs();
 

src/plugins/qnx/qnxrunconfiguration.cpp

@@ -44,7 +44,7 @@ using namespace Utils;
 namespace Qnx {
 namespace Internal {
 
-QnxRunConfiguration::QnxRunConfiguration(Target *target, Utils::Id id)
+QnxRunConfiguration::QnxRunConfiguration(Target *target, Id id)
     : RunConfiguration(target, id)
 {
     auto exeAspect = addAspect<ExecutableAspect>(target);
@@ -60,7 +60,7 @@ QnxRunConfiguration::QnxRunConfiguration(Target *target, Utils::Id id)
 
     auto envAspect = addAspect<RemoteLinuxEnvironmentAspect>(target);
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     addAspect<TerminalAspect>();
 

src/plugins/remotelinux/remotelinuxcustomrunconfiguration.cpp

@@ -71,7 +71,7 @@ RemoteLinuxCustomRunConfiguration::RemoteLinuxCustomRunConfiguration(Target *tar
     symbolsAspect->setLabelText(tr("Local executable:"));
     symbolsAspect->setDisplayStyle(SymbolFileAspect::PathChooserDisplay);
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     if (HostOsInfo::isAnyUnixHost())
         addAspect<TerminalAspect>();

src/plugins/remotelinux/remotelinuxrunconfiguration.cpp

@@ -70,7 +70,7 @@ RemoteLinuxRunConfiguration::RemoteLinuxRunConfiguration(Target *target, Id id)
     symbolsAspect->setLabelText(tr("Executable on host:"));
     symbolsAspect->setDisplayStyle(SymbolFileAspect::LabelDisplay);
 
-    addAspect<ArgumentsAspect>();
+    addAspect<ArgumentsAspect>(macroExpander());
     addAspect<WorkingDirectoryAspect>(envAspect);
     if (HostOsInfo::isAnyUnixHost())
         addAspect<TerminalAspect>();