RemoteLinux: Use a structure to return check results

Instead of the combined bool value + QString * out parameter.

Change-Id: I8840f48b74aaacd1b0c0412efd8abcdc2be12d58
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/remotelinux/abstractremotelinuxdeployservice.cpp

@@ -126,9 +126,9 @@ void AbstractRemoteLinuxDeployService::start()
 {
     QTC_ASSERT(d->state == Inactive, return);
 
-    QString errorMsg;
+    const CheckResult check = isDeploymentPossible();
-    if (!isDeploymentPossible(&errorMsg)) {
+    if (!check) {
-        emit errorMessage(errorMsg);
+        emit errorMessage(check.errorMessage());
         emit finished();
         return;
     }
@@ -165,14 +165,11 @@ void AbstractRemoteLinuxDeployService::stop()
     }
 }
 
-bool AbstractRemoteLinuxDeployService::isDeploymentPossible(QString *whyNot) const
+CheckResult AbstractRemoteLinuxDeployService::isDeploymentPossible() const
 {
-    if (!deviceConfiguration()) {
+    if (!deviceConfiguration())
-        if (whyNot)
+        return CheckResult::failure(tr("No device configuration set."));
-            *whyNot = tr("No device configuration set.");
+    return CheckResult::success();
-        return false;
-    }
-    return true;
 }
 
 QVariantMap AbstractRemoteLinuxDeployService::exportDeployTimes() const

src/plugins/remotelinux/abstractremotelinuxdeployservice.h

@@ -43,6 +43,22 @@ class Target;
 namespace RemoteLinux {
 namespace Internal { class AbstractRemoteLinuxDeployServicePrivate; }
 
+class REMOTELINUX_EXPORT CheckResult
+{
+public:
+    static CheckResult success() { return {true, {}}; }
+    static CheckResult failure(const QString &error = {}) { return {false, error}; }
+
+    operator bool() const { return m_ok; }
+    QString errorMessage() const { return m_error; }
+
+private:
+    CheckResult(bool ok, const QString &error) : m_ok(ok), m_error(error) {}
+
+    bool m_ok = false;
+    QString m_error;
+};
+
 class REMOTELINUX_EXPORT AbstractRemoteLinuxDeployService : public QObject
 {
     Q_OBJECT
@@ -60,7 +76,7 @@ public:
     QVariantMap exportDeployTimes() const;
     void importDeployTimes(const QVariantMap &map);
 
-    virtual bool isDeploymentPossible(QString *whyNot = nullptr) const;
+    virtual CheckResult isDeploymentPossible() const;
 
 signals:
     void errorMessage(const QString &message);

src/plugins/remotelinux/abstractremotelinuxdeploystep.cpp

@@ -69,11 +69,12 @@ QVariantMap AbstractRemoteLinuxDeployStep::toMap() const
 
 bool AbstractRemoteLinuxDeployStep::init()
 {
-    QString error;
     deployService()->setTarget(target());
-    const bool canDeploy = initInternal(&error);
+    const CheckResult canDeploy = initInternal();
-    if (!canDeploy)
+    if (!canDeploy) {
-        emit addOutput(tr("Cannot deploy: %1").arg(error), OutputFormat::ErrorMessage);
+        emit addOutput(tr("Cannot deploy: %1").arg(canDeploy.errorMessage()),
+                       OutputFormat::ErrorMessage);
+    }
     return canDeploy;
 }
 

src/plugins/remotelinux/abstractremotelinuxdeploystep.h

@@ -27,12 +27,11 @@
 
 #include "remotelinux_export.h"
 
+#include "abstractremotelinuxdeployservice.h"
+
 #include <projectexplorer/buildstep.h>
 
-#include <QVariantMap>
-
 namespace RemoteLinux {
-class AbstractRemoteLinuxDeployService;
 
 namespace Internal { class AbstractRemoteLinuxDeployStepPrivate; }
 
@@ -52,7 +51,7 @@ protected:
     void doCancel() override;
 
     explicit AbstractRemoteLinuxDeployStep(ProjectExplorer::BuildStepList *bsl, Core::Id id);
-    virtual bool initInternal(QString *error = nullptr) = 0;
+    virtual CheckResult initInternal() = 0;
 
 private:
     void handleProgressMessage(const QString &message);

src/plugins/remotelinux/genericdirectuploadstep.cpp

@@ -71,11 +71,11 @@ GenericDirectUploadStep::~GenericDirectUploadStep()
     delete d;
 }
 
-bool GenericDirectUploadStep::initInternal(QString *error)
+CheckResult GenericDirectUploadStep::initInternal()
 {
     d->deployService.setIncrementalDeployment(d->incrementalAspect->value());
     d->deployService.setIgnoreMissingFiles(d->ignoreMissingFilesAspect->value());
-    return d->deployService.isDeploymentPossible(error);
+    return d->deployService.isDeploymentPossible();
 }
 
 GenericDirectUploadService *GenericDirectUploadStep::deployService() const

src/plugins/remotelinux/genericdirectuploadstep.h

@@ -40,7 +40,7 @@ public:
     explicit GenericDirectUploadStep(ProjectExplorer::BuildStepList *bsl);
     ~GenericDirectUploadStep() override;
 
-    bool initInternal(QString *error = nullptr) override;
+    CheckResult initInternal() override;
 
     static Core::Id stepId();
     static QString displayName();

src/plugins/remotelinux/remotelinuxcheckforfreediskspaceservice.cpp

@@ -102,18 +102,15 @@ void RemoteLinuxCheckForFreeDiskSpaceService::handleProcessFinished()
     stopDeployment();
 }
 
-bool RemoteLinuxCheckForFreeDiskSpaceService::isDeploymentPossible(QString *whyNot) const
+CheckResult RemoteLinuxCheckForFreeDiskSpaceService::isDeploymentPossible() const
 {
-    if (!AbstractRemoteLinuxDeployService::isDeploymentPossible(whyNot))
-        return false;
     if (!d->pathToCheck.startsWith(QLatin1Char('/'))) {
-        if (whyNot) {
+        return CheckResult::failure(
-            *whyNot = tr("Cannot check for free disk space: \"%1\" is not an absolute path.")
+           tr("Cannot check for free disk space: \"%1\" is not an absolute path.")
-                    .arg(d->pathToCheck);
+                    .arg(d->pathToCheck));
     }
-        return false;
+
-    }
+    return AbstractRemoteLinuxDeployService::isDeploymentPossible();
-    return true;
 }
 
 void RemoteLinuxCheckForFreeDiskSpaceService::doDeploy()

src/plugins/remotelinux/remotelinuxcheckforfreediskspaceservice.h

@@ -49,7 +49,7 @@ private:
     void doDeviceSetup() override { handleDeviceSetupDone(true); }
     void stopDeviceSetup() override { handleDeviceSetupDone(false); }
 
-    bool isDeploymentPossible(QString *whyNot) const override;
+    CheckResult isDeploymentPossible() const override;
     void doDeploy() override;
     void stopDeployment() override;
 

src/plugins/remotelinux/remotelinuxcheckforfreediskspacestep.cpp

@@ -80,14 +80,13 @@ RemoteLinuxCheckForFreeDiskSpaceStep::~RemoteLinuxCheckForFreeDiskSpaceStep()
     delete d;
 }
 
-bool RemoteLinuxCheckForFreeDiskSpaceStep::initInternal(QString *error)
+CheckResult RemoteLinuxCheckForFreeDiskSpaceStep::initInternal()
 {
-    Q_UNUSED(error);
     d->deployService.setPathToCheck(
         static_cast<BaseStringAspect *>(aspect(PathToCheckAspectId))->value());
     d->deployService.setRequiredSpaceInBytes(
         static_cast<BaseIntegerAspect *>(aspect(RequiredSpaceAspectId))->value());
-    return true;
+    return CheckResult::success();
 }
 
 AbstractRemoteLinuxDeployService *RemoteLinuxCheckForFreeDiskSpaceStep::deployService() const

src/plugins/remotelinux/remotelinuxcheckforfreediskspacestep.h

@@ -41,7 +41,7 @@ public:
     static QString displayName();
 
 protected:
-    bool initInternal(QString *error) override;
+    CheckResult initInternal() override;
     AbstractRemoteLinuxDeployService *deployService() const override;
 
 private:

src/plugins/remotelinux/remotelinuxcustomcommanddeploymentstep.cpp

@@ -58,10 +58,10 @@ RemoteLinuxCustomCommandDeploymentStep::~RemoteLinuxCustomCommandDeploymentStep(
     delete d;
 }
 
-bool RemoteLinuxCustomCommandDeploymentStep::initInternal(QString *error)
+CheckResult RemoteLinuxCustomCommandDeploymentStep::initInternal()
 {
     d->service.setCommandLine(d->commandLineAspect->value().trimmed());
-    return d->service.isDeploymentPossible(error);
+    return d->service.isDeploymentPossible();
 }
 
 AbstractRemoteLinuxDeployService *RemoteLinuxCustomCommandDeploymentStep::deployService() const

src/plugins/remotelinux/remotelinuxcustomcommanddeploymentstep.h

@@ -42,7 +42,7 @@ public:
     static QString displayName();
 
 private:
-    bool initInternal(QString *error) override;
+    CheckResult initInternal() override;
     AbstractRemoteLinuxDeployService *deployService() const override;
 
     Internal::RemoteLinuxCustomCommandDeploymentStepPrivate *d;

src/plugins/remotelinux/remotelinuxcustomcommanddeployservice.cpp

@@ -66,19 +66,14 @@ void RemoteLinuxCustomCommandDeployService::setCommandLine(const QString &comman
     d->commandLine = commandLine;
 }
 
-bool RemoteLinuxCustomCommandDeployService::isDeploymentPossible(QString *whyNot) const
+CheckResult RemoteLinuxCustomCommandDeployService::isDeploymentPossible() const
 {
-    QTC_ASSERT(d->state == Inactive, return false);
+    QTC_ASSERT(d->state == Inactive, return CheckResult::failure());
 
-    if (!AbstractRemoteLinuxDeployService::isDeploymentPossible(whyNot))
+    if (d->commandLine.isEmpty())
-        return false;
+        return CheckResult::failure(tr("No command line given."));
-    if (d->commandLine.isEmpty()) {
-        if (whyNot)
-            *whyNot = tr("No command line given.");
-        return false;
-    }
 
-    return true;
+    return AbstractRemoteLinuxDeployService::isDeploymentPossible();
 }
 
 void RemoteLinuxCustomCommandDeployService::doDeploy()

src/plugins/remotelinux/remotelinuxcustomcommanddeployservice.h

@@ -41,7 +41,7 @@ public:
     void setCommandLine(const QString &commandLine);
 
     bool isDeploymentNecessary() const override { return true; }
-    bool isDeploymentPossible(QString *whyNot = nullptr) const override;
+    CheckResult isDeploymentPossible() const override;
 
 protected:
     void doDeviceSetup() override { handleDeviceSetupDone(true); }

src/plugins/remotelinux/remotelinuxkillappstep.cpp

@@ -44,15 +44,14 @@ RemoteLinuxKillAppStep::RemoteLinuxKillAppStep(BuildStepList *bsl, Core::Id id)
     setWidgetExpandedByDefault(false);
 }
 
-bool RemoteLinuxKillAppStep::initInternal(QString *error)
+CheckResult RemoteLinuxKillAppStep::initInternal()
 {
-    Q_UNUSED(error);
     Target * const theTarget = target();
-    QTC_ASSERT(theTarget, return false);
+    QTC_ASSERT(theTarget, return CheckResult::failure());
     RunConfiguration * const rc = theTarget->activeRunConfiguration();
     const QString remoteExe = rc ? rc->runnable().executable : QString();
     m_service->setRemoteExecutable(remoteExe);
-    return true;
+    return CheckResult::success();
 }
 
 AbstractRemoteLinuxDeployService *RemoteLinuxKillAppStep::deployService() const

src/plugins/remotelinux/remotelinuxkillappstep.h

@@ -41,7 +41,7 @@ public:
     static QString displayName();
 
 private:
-    bool initInternal(QString *error) override;
+    CheckResult initInternal() override;
     AbstractRemoteLinuxDeployService *deployService() const override;
 
     RemoteLinuxKillAppService * const m_service;

src/plugins/remotelinux/rsyncdeploystep.cpp

@@ -199,10 +199,10 @@ RsyncDeployStep::~RsyncDeployStep()
     delete d;
 }
 
-bool RsyncDeployStep::initInternal(QString *error)
+CheckResult RsyncDeployStep::initInternal()
 {
     d->deployService.setIgnoreMissingFiles(d->ignoreMissingFilesAspect->value());
-    return d->deployService.isDeploymentPossible(error);
+    return d->deployService.isDeploymentPossible();
 }
 
 AbstractRemoteLinuxDeployService *RsyncDeployStep::deployService() const

src/plugins/remotelinux/rsyncdeploystep.h

@@ -57,7 +57,7 @@ private:
     AbstractRemoteLinuxDeployService *deployService() const override;
     void doRun() override;
 
-    bool initInternal(QString *error = nullptr) override;
+    CheckResult initInternal() override;
 
     class RsyncDeployStepPrivate;
     RsyncDeployStepPrivate * const d;

src/plugins/remotelinux/uploadandinstalltarpackagestep.cpp

@@ -69,7 +69,7 @@ UploadAndInstallTarPackageStep::UploadAndInstallTarPackageStep(BuildStepList *bs
     setWidgetExpandedByDefault(false);
 }
 
-bool UploadAndInstallTarPackageStep::initInternal(QString *error)
+CheckResult UploadAndInstallTarPackageStep::initInternal()
 {
     const TarPackageCreationStep *pStep = nullptr;
 
@@ -79,13 +79,11 @@ bool UploadAndInstallTarPackageStep::initInternal(QString *error)
         if ((pStep = dynamic_cast<TarPackageCreationStep *>(step)))
             break;
     }
-    if (!pStep) {
+    if (!pStep)
-        if (error)
+        return CheckResult::failure(tr("No tarball creation step found."));
-            *error = tr("No tarball creation step found.");
+
-        return false;
-    }
     m_deployService->setPackageFilePath(pStep->packageFilePath());
-    return m_deployService->isDeploymentPossible(error);
+    return m_deployService->isDeploymentPossible();
 }
 
 Core::Id UploadAndInstallTarPackageStep::stepId()

src/plugins/remotelinux/uploadandinstalltarpackagestep.h

@@ -55,7 +55,7 @@ class REMOTELINUX_EXPORT UploadAndInstallTarPackageStep : public AbstractRemoteL
 public:
     explicit UploadAndInstallTarPackageStep(ProjectExplorer::BuildStepList *bsl);
 
-    bool initInternal(QString *error = nullptr) override;
+    CheckResult initInternal() override;
 
     static Core::Id stepId();
     static QString displayName();