Fix SonarCloud issue

- FolderDefinition::load can never fail, so just return the newly
  loaded FolderDefinition instead of passing it in by reference.
  This also removes an if statement, reducing the complexity.
- FolderMan::addFolderInternal can never return a nullptr, so
  replace the if check in FolderMan::addFolder by an assert.

src/gui/folder.cpp

@@ -1337,54 +1337,55 @@ void FolderDefinition::save(QSettings &settings, const FolderDefinition &folder)
     }
 }
 
-bool FolderDefinition::load(const QSettings &settings, const QString &alias, FolderDefinition *folder)
+FolderDefinition FolderDefinition::load(const QSettings &settings, const QString &alias)
 {
-    folder->alias = FolderMan::unescapeAlias(alias);
-    folder->localPath = settings.value(localPathC).toString();
-    folder->journalPath = settings.value(journalPathC).toString();
-    folder->targetPath = settings.value(targetPathC).toString();
-    folder->paused = settings.value(pausedC).toBool();
-    folder->ignoreHiddenFiles = settings.value(ignoreHiddenFilesC, QVariant(true)).toBool();
-    folder->isInNavigationPane = settings.value(isInNavigationPaneC, QVariant(true)).toBool();
+    FolderDefinition folder;
+    folder.alias = FolderMan::unescapeAlias(alias);
+    folder.localPath = settings.value(localPathC).toString();
+    folder.journalPath = settings.value(journalPathC).toString();
+    folder.targetPath = settings.value(targetPathC).toString();
+    folder.paused = settings.value(pausedC).toBool();
+    folder.ignoreHiddenFiles = settings.value(ignoreHiddenFilesC, QVariant(true)).toBool();
+    folder.isInNavigationPane = settings.value(isInNavigationPaneC, QVariant(true)).toBool();
 
     if (settings.contains(uuidC)) {
-        folder->uuid = settings.value(uuidC).toUuid();
+        folder.uuid = settings.value(uuidC).toUuid();
     } else if (settings.contains(navigationPaneClsidC)) {
         // For backwards compatibility, will be changed when saved:
-        folder->uuid = settings.value(navigationPaneClsidC).toUuid();
-        if (!folder->isInNavigationPane) {
-            folder->isInNavigationPane = true;
+        folder.uuid = settings.value(navigationPaneClsidC).toUuid();
+        if (!folder.isInNavigationPane) {
+            folder.isInNavigationPane = true;
         }
     }
 
     // Sanity check:
-    if (folder->uuid.isNull()) {
-        folder->uuid = QUuid::createUuid();
+    if (folder.uuid.isNull()) {
+        folder.uuid = QUuid::createUuid();
     }
 
-    folder->virtualFilesMode = Vfs::Off;
+    folder.virtualFilesMode = Vfs::Off;
     QString vfsModeString = settings.value(virtualFilesModeC).toString();
     if (!vfsModeString.isEmpty()) {
         if (auto mode = Vfs::modeFromString(vfsModeString)) {
-            folder->virtualFilesMode = *mode;
+            folder.virtualFilesMode = *mode;
         } else {
             qCWarning(lcFolder) << "Unknown virtualFilesMode:" << vfsModeString << "assuming 'off'";
         }
     } else {
         if (settings.value(usePlaceholdersC).toBool()) {
-            folder->virtualFilesMode = Vfs::WithSuffix;
-            folder->upgradeVfsMode = true; // maybe winvfs is available?
+            folder.virtualFilesMode = Vfs::WithSuffix;
+            folder.upgradeVfsMode = true; // maybe winvfs is available?
         }
     }
 
     // Old settings can contain paths with native separators. In the rest of the
     // code we assume /, so clean it up now.
-    folder->localPath = prepareLocalPath(folder->localPath);
+    folder.localPath = prepareLocalPath(folder.localPath);
 
     // Target paths also have a convention
-    folder->targetPath = prepareTargetPath(folder->targetPath);
+    folder.targetPath = prepareTargetPath(folder.targetPath);
 
-    return true;
+    return folder;
 }
 
 QString FolderDefinition::prepareLocalPath(const QString &path)

src/gui/folder.h

@@ -85,8 +85,7 @@ class FolderDefinition
     static void save(QSettings &settings, const FolderDefinition &folder);
 
     /// Reads a folder definition from the current settings group.
-    static bool load(const QSettings &settings, const QString &alias,
-        FolderDefinition *folder);
+    static FolderDefinition load(const QSettings &settings, const QString &alias);
 
     /** The highest version in the settings that load() can read
      *

src/gui/folderman.cpp

@@ -306,78 +306,79 @@ void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account,
         }
         settings.endGroup();
 
-        FolderDefinition folderDefinition;
         settings.beginGroup(folderAlias);
-        if (FolderDefinition::load(settings, folderAlias, &folderDefinition)) {
-            const auto defaultJournalPath = [&account, folderDefinition] {
-                // if we would have booth the 2.9.0 file name and the lagacy file
-                // with the md5 infix we prefer the 2.9.0 version
-                const QDir info(folderDefinition.localPath);
-                const QString defaultPath = SyncJournalDb::makeDbName(folderDefinition.localPath);
-                if (info.exists(defaultPath)) {
-                    return defaultPath;
-                }
-                // 2.6
-                QString legacyPath = makeLegacyDbName(folderDefinition, account->account());
-                if (info.exists(legacyPath)) {
-                    return legacyPath;
-                }
-                // pre 2.6
-                legacyPath.replace(QLatin1String(".sync_"), QLatin1String("._sync_"));
-                if (info.exists(legacyPath)) {
-                    return legacyPath;
-                }
+        FolderDefinition folderDefinition = FolderDefinition::load(settings, folderAlias);
+        const auto defaultJournalPath = [&account, folderDefinition] {
+            // if we would have both the 2.9.0 file name and the lagacy file
+            // with the md5 infix we prefer the 2.9.0 version
+            const QDir info(folderDefinition.localPath);
+            const QString defaultPath = SyncJournalDb::makeDbName(folderDefinition.localPath);
+            if (info.exists(defaultPath)) {
                 return defaultPath;
-            }();
-
-            // Migration: Old settings don't have journalPath
-            if (folderDefinition.journalPath.isEmpty()) {
-                folderDefinition.journalPath = defaultJournalPath;
             }
-
-            // Migration: ._ files sometimes can't be created.
-            // So if the configured journalPath has a dot-underscore ("._sync_*.db")
-            // but the current default doesn't have the underscore, switch to the
-            // new default if no db exists yet.
-            if (folderDefinition.journalPath.startsWith("._sync_")
-                && defaultJournalPath.startsWith(".sync_")
-                && !QFile::exists(folderDefinition.absoluteJournalPath())) {
-                folderDefinition.journalPath = defaultJournalPath;
+            // 2.6
+            QString legacyPath = makeLegacyDbName(folderDefinition, account->account());
+            if (info.exists(legacyPath)) {
+                return legacyPath;
             }
-
-            // Migration: If an old .csync_journal.db is found, move it to the new name.
-            if (backwardsCompatible) {
-                SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath());
+            // pre 2.6
+            legacyPath.replace(QLatin1String(".sync_"), QLatin1String("._sync_"));
+            if (info.exists(legacyPath)) {
+                return legacyPath;
             }
+            return defaultPath;
+        }();
 
-            auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode);
-            if (!vfs) {
-                // TODO: Must do better error handling
-                qFatal("Could not load plugin");
-            }
+        // Migration: Old settings don't have journalPath
+        if (folderDefinition.journalPath.isEmpty()) {
+            folderDefinition.journalPath = defaultJournalPath;
+        }
 
-            Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs));
-            if (f) {
-                // Migrate the old "usePlaceholders" setting to the root folder pin state
-                if (settings.value(versionC, 1).toInt() == 1
-                    && settings.value(QLatin1String("usePlaceholders"), false).toBool()) {
-                    qCInfo(lcFolderMan) << "Migrate: From usePlaceholders to PinState::OnlineOnly";
-                    f->setRootPinState(PinState::OnlineOnly);
-                }
+        // Migration: ._ files sometimes can't be created.
+        // So if the configured journalPath has a dot-underscore ("._sync_*.db")
+        // but the current default doesn't have the underscore, switch to the
+        // new default if no db exists yet.
+        if (folderDefinition.journalPath.startsWith("._sync_")
+            && defaultJournalPath.startsWith(".sync_")
+            && !QFile::exists(folderDefinition.absoluteJournalPath())) {
+            folderDefinition.journalPath = defaultJournalPath;
+        }
 
-                // Migration: Mark folders that shall be saved in a backwards-compatible way
-                if (backwardsCompatible)
-                    f->setSaveBackwardsCompatible(true);
-                if (foldersWithPlaceholders)
-                    f->setSaveInFoldersWithPlaceholders();
+        // Migration: If an old .csync_journal.db is found, move it to the new name.
+        if (backwardsCompatible) {
+            SyncJournalDb::maybeMigrateDb(folderDefinition.localPath, folderDefinition.absoluteJournalPath());
+        }
 
-                // save possible changes from the migration
-                f->saveToSettings();
+        auto vfs = createVfsFromPlugin(folderDefinition.virtualFilesMode);
+        if (!vfs) {
+            // TODO: Must do better error handling
+            qFatal("Could not load plugin");
+        }
 
-                scheduleFolder(f);
-                emit folderSyncStateChange(f);
-            }
+        Folder *f = addFolderInternal(std::move(folderDefinition), account.data(), std::move(vfs));
+        Q_ASSERT(f);
+
+        // Migrate the old "usePlaceholders" setting to the root folder pin state
+        if (settings.value(versionC, 1).toInt() == 1
+            && settings.value(QLatin1String("usePlaceholders"), false).toBool()) {
+            qCInfo(lcFolderMan) << "Migrate: From usePlaceholders to PinState::OnlineOnly";
+            f->setRootPinState(PinState::OnlineOnly);
+        }
+
+        // Migration: Mark folders that shall be saved in a backwards-compatible way
+        if (backwardsCompatible) {
+            f->setSaveBackwardsCompatible(true);
+        }
+        if (foldersWithPlaceholders) {
+            f->setSaveInFoldersWithPlaceholders();
         }
+
+        // save possible changes from the migration
+        f->saveToSettings();
+
+        scheduleFolder(f);
+        emit folderSyncStateChange(f);
+
         settings.endGroup();
     }
 }

src/gui/folderman.h

@@ -329,7 +329,9 @@ private slots:
 private:
     /** Adds a new folder, does not add it to the account settings and
      *  does not set an account on the new folder.
-      */
+     *
+     *  \returns the newly created Folder, which can never be a nullptr.
+     */
     Folder *addFolderInternal(FolderDefinition folderDefinition,
         AccountState *accountState, std::unique_ptr<Vfs> vfs);