MacroExpander: Return error from expandArgs and handle them

Change-Id: Ie4eea11d6e85ab45bc6abf7d255aa204b9c73c92
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/macroexpander.cpp

@@ -19,8 +19,6 @@
 namespace Utils {
 namespace Internal {
 
-static Q_LOGGING_CATEGORY(expanderLog, "qtc.utils.macroexpander", QtWarningMsg)
-
 const char kFilePathPostfix[] = ":FilePath";
 const char kPathPostfix[] = ":Path";
 const char kNativeFilePathPostfix[] = ":NativeFilePath";
@@ -413,7 +411,8 @@ QVariant MacroExpander::expandVariant(const QVariant &v) const
     return v;
 }
 
-QString MacroExpander::expandProcessArgs(const QString &argsWithVariables, Utils::OsType osType) const
+expected_str<QString> MacroExpander::expandProcessArgs(
+    const QString &argsWithVariables, Utils::OsType osType) const
 {
     QString result = argsWithVariables;
     const bool ok = ProcessArgs::expandMacros(
@@ -421,7 +420,10 @@ QString MacroExpander::expandProcessArgs(const QString &argsWithVariables, Utils
         [this](const QString &str, int *pos, QString *ret) { return d->findMacro(str, pos, ret); },
         osType);
 
-    QTC_ASSERT(ok, qCDebug(expanderLog) << "Expanding failed: " << argsWithVariables);
+    if (!ok) {
+        return make_unexpected(
+            Tr::tr("Failed to expand macros in process arguments: %1").arg(argsWithVariables));
+    }
     return result;
 }
 

src/libs/utils/macroexpander.h

@@ -37,7 +37,7 @@ public:
     QByteArray expand(const QByteArray &stringWithVariables) const;
     QVariant expandVariant(const QVariant &v) const;
 
-    QString expandProcessArgs(
+    expected_str<QString> expandProcessArgs(
         const QString &argsWithVariables, Utils::OsType osType = Utils::HostOsInfo::hostOs()) const;
 
     using PrefixFunction = std::function<QString(QString)>;

src/plugins/coreplugin/dialogs/externaltoolconfig.cpp

@@ -923,7 +923,12 @@ void ExternalToolConfig::addCategory()
 
 void ExternalToolConfig::updateEffectiveArguments()
 {
-    m_arguments->setToolTip(Utils::globalMacroExpander()->expandProcessArgs(m_arguments->text()));
+    const expected_str<QString> result = Utils::globalMacroExpander()->expandProcessArgs(
+        m_arguments->text());
+    if (result)
+        m_arguments->setToolTip(*result);
+    else
+        m_arguments->setToolTip(result.error());
 }
 
 void ExternalToolConfig::editEnvironmentChanges()

src/plugins/coreplugin/externaltool.cpp

@@ -598,7 +598,10 @@ bool ExternalToolRunner::resolve()
         }
     }
 
-    m_resolvedArguments = expander->expandProcessArgs(m_tool->arguments());
+    const expected_str<QString> args = expander->expandProcessArgs(m_tool->arguments());
+    QTC_ASSERT_EXPECTED(args, return false);
+
+    m_resolvedArguments = *args;
     m_resolvedInput = expander->expand(m_tool->input());
     m_resolvedWorkingDirectory = expander->expand(m_tool->workingDirectory());
 

src/plugins/projectexplorer/runconfigurationaspects.cpp

@@ -328,9 +328,11 @@ QString ArgumentsAspect::arguments() const
         return m_arguments;
 
     m_currentlyExpanding = true;
-    const QString expanded = macroExpander()->expandProcessArgs(m_arguments);
+    const expected_str<QString> expanded = macroExpander()->expandProcessArgs(m_arguments);
+    QTC_ASSERT_EXPECTED(expanded, return m_arguments);
+
     m_currentlyExpanding = false;
-    return expanded;
+    return *expanded;
 }
 
 /*!
@@ -962,7 +964,7 @@ X11ForwardingAspect::X11ForwardingAspect(AspectContainer *container)
 
 QString X11ForwardingAspect::display() const
 {
-    return !isChecked() ? QString() : macroExpander()->expandProcessArgs(value());
+    return !isChecked() ? QString() : macroExpander()->expand(value());
 }
 
 

src/plugins/qmakeprojectmanager/qmakebuildconfiguration.cpp

@@ -241,6 +241,7 @@ void QmakeBuildConfiguration::updateProblemLabel()
     bool targetMismatch = false;
     bool incompatibleBuild = false;
     bool allGood = false;
+    bool invalidArguments = false;
     // we only show if we actually have a qmake and makestep
     QString errorString;
     if (qmakeStep() && makeStep()) {
@@ -258,6 +259,9 @@ void QmakeBuildConfiguration::updateProblemLabel()
         case QmakeBuildConfiguration::MakefileForWrongProject:
             targetMismatch = true;
             break;
+        case QmakeBuildConfiguration::InvalidArguments:
+            invalidArguments = true;
+            break;
         }
     }
 
@@ -305,6 +309,10 @@ void QmakeBuildConfiguration::updateProblemLabel()
     } else if (unalignedBuildDir) {
         buildDirectoryAspect()->setProblem(unalignedBuildDirWarning());
         return;
+    } else if (invalidArguments) {
+        buildDirectoryAspect()->setProblem(
+            Tr::tr("Starting qmake failed with the following error: %1").arg(errorString));
+        return;
     }
 
     buildDirectoryAspect()->setProblem({});
@@ -516,8 +524,16 @@ QmakeBuildConfiguration::MakefileState QmakeBuildConfiguration::compareToImportF
     // and compare that on its own
     FilePath workingDirectory = makefile.parentDir();
     QStringList actualArgs;
-    QString allArgs = macroExpander()->expandProcessArgs(qs->allArguments(
+    expected_str<QString> expandResult = macroExpander()->expandProcessArgs(
-        QtKitAspect::qtVersion(target()->kit()), QMakeStep::ArgumentFlag::Expand));
+        qs->allArguments(QtKitAspect::qtVersion(target()->kit()), QMakeStep::ArgumentFlag::Expand));
+
+    if (!expandResult) {
+        if (errorString)
+            *errorString = expandResult.error();
+        return InvalidArguments;
+    }
+
+    QString allArgs = *expandResult;
     // This copies the settings from allArgs to actualArgs (minus some we
     // are not interested in), splitting them up into individual strings:
     extractSpecFromArguments(&allArgs, workingDirectory, version, &actualArgs);

src/plugins/qmakeprojectmanager/qmakebuildconfiguration.h

@@ -61,7 +61,13 @@ public:
 
     Utils::FilePath makefile() const;
 
-    enum MakefileState { MakefileMatches, MakefileForWrongProject, MakefileIncompatible, MakefileMissing };
+    enum MakefileState {
+        MakefileMatches,
+        MakefileForWrongProject,
+        MakefileIncompatible,
+        MakefileMissing,
+        InvalidArguments
+    };
     MakefileState compareToImportFrom(const Utils::FilePath &makefile, QString *errorString = nullptr);
     static QString extractSpecFromArguments(
             QString *arguments, const Utils::FilePath &directory, const QtSupport::QtVersion *version,