Android: Simplify listVirtualDevices()

Introduce ParsedAvdList struct and return it from the parseAvdList().
This eliminates passing the errorPaths result by reference.
Simplify listVirtualDevices() implementation by removing some local
variables.
Adapt the tst_AvdManagerOutputParser test accordingly.

Change-Id: If4670bcc81ce36416c1fe35e8ee57e822f82516f
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>

4 files changed, 26 insertions, 24 deletions

diff --git a/src/plugins/android/androidavdmanager.cpp b/src/plugins/android/androidavdmanager.cpp
index fcc56980bc..fc9148d3fe 100644
--- a/src/plugins/android/androidavdmanager.cpp
+++ b/src/plugins/android/androidavdmanager.cpp
@@ -72,8 +72,6 @@ static void avdConfigEditManufacturerTag(const FilePath &avdPath, bool recoverMo
 
 static AndroidDeviceInfoList listVirtualDevices()
 {
-    QString output;
-    AndroidDeviceInfoList avdList;
     /*
         Currenly avdmanager tool fails to parse some AVDs because the correct
         device definitions at devices.xml does not have some of the newest devices.
@@ -83,26 +81,25 @@ static AndroidDeviceInfoList listVirtualDevices()
         aim to keep support for Qt Creator and Android Studio.
     */
     FilePaths allAvdErrorPaths;
-    FilePaths avdErrorPaths;
-
-    do {
+    while (true) {
+        QString output;
         if (!AndroidAvdManager::avdManagerCommand({"list", "avd"}, &output)) {
             qCDebug(avdManagerLog)
                 << "Avd list command failed" << output << androidConfig().sdkToolsVersion();
             return {};
         }
 
-        avdErrorPaths.clear();
-        avdList = parseAvdList(output, &avdErrorPaths);
-        allAvdErrorPaths << avdErrorPaths;
-        for (const FilePath &avdPath : std::as_const(avdErrorPaths))
+        const auto parsedAvdList = parseAvdList(output);
+        if (parsedAvdList.errorPaths.isEmpty()) {
+            for (const FilePath &avdPath : std::as_const(allAvdErrorPaths))
+                avdConfigEditManufacturerTag(avdPath, true); // re-add manufacturer tag
+            return parsedAvdList.avdList;
+        }
+        allAvdErrorPaths << parsedAvdList.errorPaths;
+        for (const FilePath &avdPath : parsedAvdList.errorPaths)
             avdConfigEditManufacturerTag(avdPath); // comment out manufacturer tag
-    } while (!avdErrorPaths.isEmpty());            // try again
-
-    for (const FilePath &avdPath : std::as_const(allAvdErrorPaths))
-        avdConfigEditManufacturerTag(avdPath, true); // re-add manufacturer tag
-
-    return avdList;
+    }
+    return {};
 }
 
 QFuture<AndroidDeviceInfoList> AndroidAvdManager::avdList() const
diff --git a/src/plugins/android/avdmanageroutputparser.cpp b/src/plugins/android/avdmanageroutputparser.cpp
index 2727680d08..7a91b99f19 100644
--- a/src/plugins/android/avdmanageroutputparser.cpp
+++ b/src/plugins/android/avdmanageroutputparser.cpp
@@ -86,10 +86,10 @@ static std::optional<AndroidDeviceInfo> parseAvd(const QStringList &deviceInfo)
     return {};
 }
 
-AndroidDeviceInfoList parseAvdList(const QString &output, Utils::FilePaths *avdErrorPaths)
+ParsedAvdList parseAvdList(const QString &output)
 {
-    QTC_CHECK(avdErrorPaths);
     AndroidDeviceInfoList avdList;
+    Utils::FilePaths errorPaths;
     QStringList avdInfo;
     using ErrorPath = Utils::FilePath;
     using AvdResult = std::variant<std::monostate, AndroidDeviceInfo, ErrorPath>;
@@ -120,14 +120,14 @@ AndroidDeviceInfoList parseAvdList(const QString &output, Utils::FilePaths *avdE
             if (auto info = std::get_if<AndroidDeviceInfo>(&result))
                 avdList << *info;
             else if (auto errorPath = std::get_if<ErrorPath>(&result))
-                *avdErrorPaths << *errorPath;
+                errorPaths << *errorPath;
             avdInfo.clear();
         } else {
             avdInfo << line;
         }
     }
 
-    return Utils::sorted(std::move(avdList));
+    return {Utils::sorted(std::move(avdList)), errorPaths};
 }
 
 int platformNameToApiLevel(const QString &platformName)
diff --git a/src/plugins/android/avdmanageroutputparser.h b/src/plugins/android/avdmanageroutputparser.h
index b3a74f8fc8..81cbe64eaa 100644
--- a/src/plugins/android/avdmanageroutputparser.h
+++ b/src/plugins/android/avdmanageroutputparser.h
@@ -10,7 +10,13 @@ namespace Internal {
 
 const char avdManufacturerError[] = "no longer exists as a device";
 
-AndroidDeviceInfoList parseAvdList(const QString &output, Utils::FilePaths *avdErrorPaths);
+struct ParsedAvdList
+{
+    AndroidDeviceInfoList avdList;
+    Utils::FilePaths errorPaths;
+};
+
+ParsedAvdList parseAvdList(const QString &output);
 int platformNameToApiLevel(const QString &platformName);
 QString convertNameToExtension(const QString &name);
 
diff --git a/tests/auto/android/tst_avdmanageroutputparser.cpp b/tests/auto/android/tst_avdmanageroutputparser.cpp
index a41022ec5d..e65a2bb15a 100644
--- a/tests/auto/android/tst_avdmanageroutputparser.cpp
+++ b/tests/auto/android/tst_avdmanageroutputparser.cpp
@@ -80,10 +80,9 @@ void tst_AvdManagerOutputParser::parse()
     QFETCH(AndroidDeviceInfoList, output);
     QFETCH(Utils::FilePaths, errorPaths);
 
-    Utils::FilePaths avdErrorPaths;
-    const auto result = parseAvdList(input, &avdErrorPaths);
-    QCOMPARE(result, output);
-    QCOMPARE(avdErrorPaths, errorPaths);
+    const auto result = parseAvdList(input);
+    QCOMPARE(result.avdList, output);
+    QCOMPARE(result.errorPaths, errorPaths);
 }
 
 QTEST_GUILESS_MAIN(tst_AvdManagerOutputParser)