Utils: Let FilePath::renameFile return an error

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

src/libs/gocmdbridge/client/bridgedfileaccess.cpp

@@ -544,17 +544,23 @@ expected_str<void> FileAccess::copyFile(const Utils::FilePath &filePath,
     }
 }
 
-bool FileAccess::renameFile(const Utils::FilePath &filePath, const Utils::FilePath &target) const
+expected_str<void> FileAccess::renameFile(
+    const Utils::FilePath &filePath, const Utils::FilePath &target) const
 {
     try {
-        auto f = m_client->renameFile(filePath.nativePath(), target.nativePath());
+        Utils::expected_str<QFuture<void>> f
-        QTC_ASSERT_EXPECTED(f, return false);
+            = m_client->renameFile(filePath.nativePath(), target.nativePath());
+        if (!f)
+            return make_unexpected(f.error());
         f->waitForFinished();
-        return true;
+        if (!f)
+            return make_unexpected(f.error());
     } catch (const std::exception &e) {
-        qCWarning(faLog) << "Error renaming file:" << e.what();
+        return make_unexpected(
-        return false;
+            Tr::tr("Error renaming file: %1").arg(QString::fromLocal8Bit(e.what())));
     }
+
+    return {};
 }
 
 Environment FileAccess::deviceEnvironment() const

src/libs/gocmdbridge/client/bridgedfileaccess.h

@@ -78,7 +78,8 @@ protected:
     Utils::expected_str<void> copyFile(const Utils::FilePath &filePath,
                                        const Utils::FilePath &target) const override;
 
-    bool renameFile(const Utils::FilePath &filePath, const Utils::FilePath &target) const override;
+    Utils::expected_str<void> renameFile(
+        const Utils::FilePath &filePath, const Utils::FilePath &target) const override;
 
     Utils::expected_str<Utils::FilePath> createTempFile(const Utils::FilePath &filePath) override;
 

src/libs/utils/devicefileaccess.cpp

@@ -275,12 +275,13 @@ expected_str<void> DeviceFileAccess::copyRecursively(const FilePath &src,
 #endif
 }
 
-bool DeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const
+expected_str<void> DeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const
 {
     Q_UNUSED(filePath)
     Q_UNUSED(target)
     QTC_CHECK(false);
-    return false;
+    return make_unexpected(
+        Tr::tr("renameFile is not implemented for \"%1\".").arg(filePath.toUserOutput()));
 }
 
 FilePath DeviceFileAccess::symLinkTarget(const FilePath &filePath) const
@@ -786,9 +787,16 @@ expected_str<void> DesktopDeviceFileAccess::copyFile(const FilePath &filePath,
             .arg(filePath.toUserOutput(), target.toUserOutput(), srcFile.errorString()));
 }
 
-bool DesktopDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const
+expected_str<void> DesktopDeviceFileAccess::renameFile(
+    const FilePath &filePath, const FilePath &target) const
 {
-    return QFile::rename(filePath.path(), target.path());
+    QFile f(filePath.path());
+
+    if (f.rename(target.path()))
+        return {};
+    return make_unexpected(
+        Tr::tr("Failed to rename file \"%1\" to \"%2\": %3")
+            .arg(filePath.toUserOutput(), target.toUserOutput(), f.errorString()));
 }
 
 FilePathInfo DesktopDeviceFileAccess::filePathInfo(const FilePath &filePath) const
@@ -1181,12 +1189,19 @@ expected_str<void> UnixDeviceFileAccess::copyFile(const FilePath &filePath,
     return {};
 }
 
-bool UnixDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const
+expected_str<void> UnixDeviceFileAccess::renameFile(const FilePath &filePath, const FilePath &target) const
 {
     if (disconnected())
-        return false;
+        return make_unexpected_disconnected();
 
-    return runInShellSuccess({"mv", {filePath.path(), target.path()}, OsType::OsTypeLinux});
+    auto result =  runInShell({"mv", {filePath.path(), target.path()}, OsType::OsTypeLinux});
+    if (result.exitCode != 0) {
+        return make_unexpected(Tr::tr("Failed to rename file \"%1\" to \"%2\": %3")
+                                   .arg(filePath.toUserOutput(),
+                                        target.toUserOutput(),
+                                        QString::fromUtf8(result.stdErr)));
+    }
+    return {};
 }
 
 FilePath UnixDeviceFileAccess::symLinkTarget(const FilePath &filePath) const

src/libs/utils/devicefileaccess.h

@@ -47,7 +47,7 @@ protected:
     virtual expected_str<void> copyFile(const FilePath &filePath, const FilePath &target) const;
     virtual expected_str<void> copyRecursively(const FilePath &filePath,
                                                const FilePath &target) const;
-    virtual bool renameFile(const FilePath &filePath, const FilePath &target) const;
+    virtual expected_str<void> renameFile(const FilePath &filePath, const FilePath &target) const;
 
     virtual FilePath symLinkTarget(const FilePath &filePath) const;
     virtual FilePathInfo filePathInfo(const FilePath &filePath) const;
@@ -104,7 +104,7 @@ protected:
     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;
+    expected_str<void> renameFile(const FilePath &filePath, const FilePath &target) const override;
 
     FilePath symLinkTarget(const FilePath &filePath) const override;
     FilePathInfo filePathInfo(const FilePath &filePath) const override;
@@ -162,7 +162,7 @@ protected:
     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;
+    expected_str<void> renameFile(const FilePath &filePath, const FilePath &target) const override;
 
     FilePathInfo filePathInfo(const FilePath &filePath) const override;
     FilePath symLinkTarget(const FilePath &filePath) const override;

src/libs/utils/filepath.cpp

@@ -2022,7 +2022,7 @@ expected_str<void> FilePath::copyFile(const FilePath &target) const
     return fileAccess()->copyFile(*this, target);
 }
 
-bool FilePath::renameFile(const FilePath &target) const
+expected_str<void> FilePath::renameFile(const FilePath &target) const
 {
     return fileAccess()->renameFile(*this, target);
 }

src/libs/utils/filepath.h

@@ -167,7 +167,7 @@ public:
     bool removeRecursively(QString *error = nullptr) const;
     expected_str<void> copyRecursively(const FilePath &target) const;
     expected_str<void> copyFile(const FilePath &target) const;
-    bool renameFile(const FilePath &target) const;
+    expected_str<void> renameFile(const FilePath &target) const;
     qint64 fileSize() const;
     qint64 bytesAvailable() const;
     bool createDir() const;

src/libs/utils/fsengine/fsenginehandler.cpp

@@ -216,13 +216,17 @@ bool FSEngineImpl::remove()
 bool FSEngineImpl::copy(const QString &newName)
 {
     expected_str<void> result = m_filePath.copyFile(FilePath::fromString(newName));
-    QTC_ASSERT_EXPECTED(result, return false);
+    if (!result)
-    return true;
+        setError(QFile::CopyError, result.error());
+    return result.has_value();
 }
 
 bool FSEngineImpl::rename(const QString &newName)
 {
-    return m_filePath.renameFile(FilePath::fromString(newName));
+    auto result = m_filePath.renameFile(FilePath::fromString(newName));
+    if (!result)
+        setError(QFile::RenameError, result.error());
+    return result.has_value();
 }
 
 bool FSEngineImpl::renameOverwrite(const QString &newName)

src/plugins/coreplugin/fileutils.cpp

@@ -187,7 +187,7 @@ bool FileUtils::renameFile(const FilePath &orgFilePath, const FilePath &newFileP
     if (vc && vc->supportsOperation(IVersionControl::MoveOperation))
         result = vc->vcsMove(orgFilePath, newFilePath);
     if (!result) // The moving via vcs failed or the vcs does not support moving, fall back
-        result = orgFilePath.renameFile(newFilePath);
+        result = orgFilePath.renameFile(newFilePath).has_value();
     if (result) {
         DocumentManager::renamedFile(orgFilePath, newFilePath);
         updateHeaderFileGuardIfApplicable(orgFilePath, newFilePath, handleGuards);

src/plugins/languageclient/languageclientutils.cpp

@@ -381,7 +381,7 @@ bool applyDocumentChange(const Client *client, const DocumentChange &change)
                 }
             }
         }
-        return oldPath.renameFile(newPath);
+        return oldPath.renameFile(newPath).has_value();
     } else if (const auto deleteOperation = std::get_if<DeleteFileOperation>(&change)) {
         const FilePath filePath = deleteOperation->uri().toFilePath(client->hostPathMapper());
         if (const std::optional<DeleteFileOptions> options = deleteOperation->options()) {