Utils: Allow FilePath::removeFile to return error reason

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

src/libs/gocmdbridge/client/bridgedfileaccess.cpp

@@ -468,23 +468,26 @@ expected_str<qint64> FileAccess::writeFileContents(const FilePath &filePath,
     }
 }
 
-bool FileAccess::removeFile(const Utils::FilePath &filePath) const
+expected_str<void> FileAccess::removeFile(const Utils::FilePath &filePath) const
 {
     try {
-        auto f = m_client->removeFile(filePath.nativePath());
-        QTC_ASSERT_EXPECTED(f, return false);
+        Utils::expected_str<QFuture<void>> f = m_client->removeFile(filePath.nativePath());
+        if (!f)
+            return make_unexpected(f.error());
         f->waitForFinished();
     } catch (const std::system_error &e) {
         if (e.code().value() == ENOENT)
-            return false;
+            return make_unexpected(Tr::tr("File does not exist"));
         qCWarning(faLog) << "Error removing file:" << e.what();
-        return false;
+        return make_unexpected(
+            Tr::tr("Error removing file: %1").arg(QString::fromLocal8Bit(e.what())));
     } catch (const std::exception &e) {
         qCWarning(faLog) << "Error removing file:" << e.what();
-        return false;
+        return make_unexpected(
+            Tr::tr("Error removing file: %1").arg(QString::fromLocal8Bit(e.what())));
     }
 
-    return true;
+    return {};
 }
 
 bool FileAccess::removeRecursively(const Utils::FilePath &filePath, QString *error) const

src/libs/gocmdbridge/client/bridgedfileaccess.h

@@ -69,7 +69,7 @@ protected:
     Utils::expected_str<qint64> writeFileContents(const Utils::FilePath &filePath,
                                                   const QByteArray &data) const override;
 
-    bool removeFile(const Utils::FilePath &filePath) const override;
+    Utils::expected_str<void> removeFile(const Utils::FilePath &filePath) const override;
     bool removeRecursively(const Utils::FilePath &filePath, QString *error) const override;
 
     bool ensureExistingFile(const Utils::FilePath &filePath) const override;

src/libs/utils/devicefileaccess.cpp

@@ -156,11 +156,12 @@ bool DeviceFileAccess::exists(const FilePath &filePath) const
     return false;
 }
 
-bool DeviceFileAccess::removeFile(const FilePath &filePath) const
+expected_str<void> DeviceFileAccess::removeFile(const FilePath &filePath) const
 {
     Q_UNUSED(filePath)
     QTC_CHECK(false);
-    return false;
+    return make_unexpected(
+        Tr::tr("removeFile is not implemented for \"%1\".").arg(filePath.toUserOutput()));
 }
 
 bool DeviceFileAccess::removeRecursively(const FilePath &filePath, QString *error) const
@@ -691,9 +692,12 @@ bool DesktopDeviceFileAccess::exists(const FilePath &filePath) const
     return !filePath.isEmpty() && QFileInfo::exists(filePath.path());
 }
 
-bool DesktopDeviceFileAccess::removeFile(const FilePath &filePath) const
+expected_str<void> DesktopDeviceFileAccess::removeFile(const FilePath &filePath) const
 {
-    return QFile::remove(filePath.path());
+    QFile f(filePath.path());
+    if (!f.remove())
+        return make_unexpected(f.errorString());
+    return {};
 }
 
 static bool checkToRefuseRemoveStandardLocationDirectory(const QString &dirPath,
@@ -1129,11 +1133,14 @@ bool UnixDeviceFileAccess::exists(const FilePath &filePath) const
     return runInShellSuccess({"test", {"-e", path}, OsType::OsTypeLinux});
 }
 
-bool UnixDeviceFileAccess::removeFile(const FilePath &filePath) const
+expected_str<void> UnixDeviceFileAccess::removeFile(const FilePath &filePath) const
 {
     if (disconnected())
-        return false;
-    return runInShellSuccess({"rm", {filePath.path()}, OsType::OsTypeLinux});
+        return make_unexpected_disconnected();
+    RunResult result = runInShell({"rm", {filePath.path()}, OsType::OsTypeLinux});
+    if (result.exitCode != 0)
+        return make_unexpected(QString::fromUtf8(result.stdErr));
+    return {};
 }
 
 bool UnixDeviceFileAccess::removeRecursively(const FilePath &filePath, QString *error) const

src/libs/utils/devicefileaccess.h

@@ -42,7 +42,7 @@ protected:
     virtual bool ensureExistingFile(const FilePath &filePath) const;
     virtual bool createDirectory(const FilePath &filePath) const;
     virtual bool exists(const FilePath &filePath) const;
-    virtual bool removeFile(const FilePath &filePath) const;
+    virtual expected_str<void> removeFile(const FilePath &filePath) const;
     virtual bool removeRecursively(const FilePath &filePath, QString *error) const;
     virtual expected_str<void> copyFile(const FilePath &filePath, const FilePath &target) const;
     virtual expected_str<void> copyRecursively(const FilePath &filePath,
@@ -101,7 +101,7 @@ protected:
     bool ensureExistingFile(const FilePath &filePath) const override;
     bool createDirectory(const FilePath &filePath) const override;
     bool exists(const FilePath &filePath) const override;
-    bool removeFile(const FilePath &filePath) const override;
+    expected_str<void> removeFile(const FilePath &filePath) const override;
     bool removeRecursively(const FilePath &filePath, QString *error) const override;
     expected_str<void> copyFile(const FilePath &filePath, const FilePath &target) const override;
     bool renameFile(const FilePath &filePath, const FilePath &target) const override;
@@ -159,7 +159,7 @@ protected:
     bool ensureExistingFile(const FilePath &filePath) const override;
     bool createDirectory(const FilePath &filePath) const override;
     bool exists(const FilePath &filePath) const override;
-    bool removeFile(const FilePath &filePath) const override;
+    expected_str<void> removeFile(const FilePath &filePath) const override;
     bool removeRecursively(const FilePath &filePath, QString *error) const override;
     expected_str<void> copyFile(const FilePath &filePath, const FilePath &target) const override;
     bool renameFile(const FilePath &filePath, const FilePath &target) const override;

src/libs/utils/filepath.cpp

@@ -1973,7 +1973,7 @@ OsType FilePath::osType() const
     return s_deviceHooks.osType(*this);
 }
 
-bool FilePath::removeFile() const
+expected_str<void> FilePath::removeFile() const
 {
     return fileAccess()->removeFile(*this);
 }

src/libs/utils/filepath.h

@@ -138,7 +138,7 @@ public:
     QFile::Permissions permissions() const;
     bool setPermissions(QFile::Permissions permissions) const;
     OsType osType() const;
-    bool removeFile() const;
+    expected_str<void> removeFile() const;
     bool removeRecursively(QString *error = nullptr) const;
     expected_str<void> copyRecursively(const FilePath &target) const;
     expected_str<void> copyFile(const FilePath &target) const;

src/plugins/coreplugin/session.cpp

@@ -349,9 +349,11 @@ bool SessionManager::deleteSession(const QString &session)
     d->m_lastActiveTimes.remove(session);
     emit instance()->sessionRemoved(session);
     FilePath sessionFile = sessionNameToFileName(session);
-    if (sessionFile.exists())
-        return sessionFile.removeFile();
-    return false;
+    if (!sessionFile.exists())
+        return false;
+    expected_str<void> result = sessionFile.removeFile();
+    QTC_CHECK_EXPECTED(result);
+    return result.has_value();
 }
 
 void SessionManager::deleteSessions(const QStringList &sessions)

src/plugins/languageclient/languageclientutils.cpp

@@ -390,7 +390,7 @@ bool applyDocumentChange(const Client *client, const DocumentChange &change)
             if (filePath.isDir() && options->recursive().value_or(false))
                 return filePath.removeRecursively();
         }
-        return filePath.removeFile();
+        return filePath.removeFile().has_value();
     }
     return false;
 }