Merge pull request #751 from Edgars-Cirulis/develop

launcher/Version.cpp

@@ -1,68 +1,56 @@
 #include "Version.h"
 
-#include <QStringList>
+#include <QDebug>
-#include <QUrl>
 #include <QRegularExpression>
 #include <QRegularExpressionMatch>
+#include <QUrl>
 
-Version::Version(const QString &str) : m_string(str)
+Version::Version(QString str) : m_string(std::move(str))
 {
     parse();
 }
 
+#define VERSION_OPERATOR(return_on_different)                                               \
+    bool exclude_our_sections = false;                                                      \
+    bool exclude_their_sections = false;                                                    \
+                                                                                            \
+    const auto size = qMax(m_sections.size(), other.m_sections.size());                     \
+    for (int i = 0; i < size; ++i) {                                                        \
+        Section sec1 = (i >= m_sections.size()) ? Section() : m_sections.at(i);             \
+        Section sec2 = (i >= other.m_sections.size()) ? Section() : other.m_sections.at(i); \
+                                                                                            \
+        { /* Don't include appendixes in the comparison */                                  \
+            if (sec1.isAppendix())                                                          \
+                exclude_our_sections = true;                                                \
+            if (sec2.isAppendix())                                                          \
+                exclude_their_sections = true;                                              \
+                                                                                            \
+            if (exclude_our_sections) {                                                     \
+                sec1 = Section();                                                           \
+                if (sec2.m_isNull)                                                          \
+                    break;                                                                  \
+            }                                                                               \
+                                                                                            \
+            if (exclude_their_sections) {                                                   \
+                sec2 = Section();                                                           \
+                if (sec1.m_isNull)                                                          \
+                    break;                                                                  \
+            }                                                                               \
+        }                                                                                   \
+                                                                                            \
+        if (sec1 != sec2)                                                                   \
+            return return_on_different;                                                     \
+    }
+
 bool Version::operator<(const Version& other) const
 {
-    const int size = qMax(m_sections.size(), other.m_sections.size());
+    VERSION_OPERATOR(sec1 < sec2)
-    for (int i = 0; i < size; ++i)
-    {
-        const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i);
-        const Section sec2 =
-            (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i);
-        if (sec1 != sec2)
-        {
-            return sec1 < sec2;
-        }
-    }
 
     return false;
 }
-bool Version::operator<=(const Version &other) const
-{
-    return *this < other || *this == other;
-}
-bool Version::operator>(const Version &other) const
-{
-    const int size = qMax(m_sections.size(), other.m_sections.size());
-    for (int i = 0; i < size; ++i)
-    {
-        const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i);
-        const Section sec2 =
-            (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i);
-        if (sec1 != sec2)
-        {
-            return sec1 > sec2;
-        }
-    }
-
-    return false;
-}
-bool Version::operator>=(const Version &other) const
-{
-    return *this > other || *this == other;
-}
 bool Version::operator==(const Version& other) const
 {
-    const int size = qMax(m_sections.size(), other.m_sections.size());
+    VERSION_OPERATOR(false)
-    for (int i = 0; i < size; ++i)
-    {
-        const Section sec1 = (i >= m_sections.size()) ? Section("0") : m_sections.at(i);
-        const Section sec2 =
-            (i >= other.m_sections.size()) ? Section("0") : other.m_sections.at(i);
-        if (sec1 != sec2)
-        {
-            return false;
-        }
-    }
 
     return true;
 }
@@ -70,16 +58,71 @@ bool Version::operator!=(const Version &other) const
 {
     return !operator==(other);
 }
+bool Version::operator<=(const Version& other) const
+{
+    return *this < other || *this == other;
+}
+bool Version::operator>(const Version& other) const
+{
+    return !(*this <= other);
+}
+bool Version::operator>=(const Version& other) const
+{
+    return !(*this < other);
+}
 
 void Version::parse()
 {
     m_sections.clear();
+    QString currentSection;
 
-    // FIXME: this is bad. versions can contain a lot more separators...
+    if (m_string.isEmpty())
-    QStringList parts = m_string.split('.');
+        return;
 
-    for (const auto& part : parts)
+    auto classChange = [&](QChar lastChar, QChar currentChar) {
+        if (lastChar.isNull())
+            return false;
+        if (lastChar.isDigit() != currentChar.isDigit())
+            return true;
+
+        const QList<QChar> s_separators{ '.', '-', '+' };
+        if (s_separators.contains(currentChar) && currentSection.at(0) != currentChar)
+            return true;
+
+        return false;
+    };
+
+    currentSection += m_string.at(0);
+    for (int i = 1; i < m_string.size(); ++i) {
+        const auto& current_char = m_string.at(i);
+        if (classChange(m_string.at(i - 1), current_char)) {
+            if (!currentSection.isEmpty())
+                m_sections.append(Section(currentSection));
+            currentSection = "";
+        }
+
+        currentSection += current_char;
+    }
+
+    if (!currentSection.isEmpty())
+        m_sections.append(Section(currentSection));
+}
+
+/// qDebug print support for the Version class
+QDebug operator<<(QDebug debug, const Version& v)
 {
-        m_sections.append(Section(part));
+    QDebugStateSaver saver(debug);
+
+    debug.nospace() << "Version{ string: " << v.toString() << ", sections: [ ";
+
+    bool first = true;
+    for (auto s : v.m_sections) {
+        if (!first) debug.nospace() << ", ";
+        debug.nospace() << s.m_fullString;
+        first = false;
     }
+                    
+    debug.nospace() << " ]" << " }";
+
+    return debug;
 }

launcher/Version.h

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-only
 /*
  *  PolyMC - Minecraft Launcher
+ *  Copyright (C) 2023 flowln <flowlnlnln@gmail.com>
  *  Copyright (C) 2022 Sefa Eyeoglu <contact@scrumplex.net>
  *
  *  This program is free software: you can redistribute it and/or modify
@@ -35,17 +36,17 @@
 
 #pragma once
 
+#include <QDebug>
+#include <QList>
 #include <QString>
 #include <QStringView>
-#include <QList>
 
 class QUrl;
 
-class Version
+class Version {
-{
    public:
-    Version(const QString &str);
+    Version(QString str);
-    Version() {}
+    Version() = default;
 
     bool operator<(const Version &other) const;
     bool operator<=(const Version &other) const;
@@ -54,96 +55,116 @@ public:
     bool operator==(const Version &other) const;
     bool operator!=(const Version &other) const;
 
-    QString toString() const
+    QString toString() const { return m_string; }
-    {
+
-        return m_string;
+    friend QDebug operator<<(QDebug debug, const Version& v);
-    }
 
    private:
-    QString m_string;
+    struct Section {
-    struct Section
+        explicit Section(QString fullString) : m_fullString(std::move(fullString))
         {
-        explicit Section(const QString &fullString)
-        {
-            m_fullString = fullString;
             int cutoff = m_fullString.size();
-            for(int i = 0; i < m_fullString.size(); i++)
+            for (int i = 0; i < m_fullString.size(); i++) {
-            {
+                if (!m_fullString[i].isDigit()) {
-                if(!m_fullString[i].isDigit())
-                {
                     cutoff = i;
                     break;
                 }
             }
+
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
             auto numPart = QStringView{m_fullString}.left(cutoff);
 #else
             auto numPart = m_fullString.leftRef(cutoff);
 #endif
-            if(numPart.size())
+
-            {
+            if (!numPart.isEmpty()) {
-                numValid = true;
+                m_isNull = false;
                 m_numPart = numPart.toInt();
             }
+
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
             auto stringPart = QStringView{m_fullString}.mid(cutoff);
 #else
             auto stringPart = m_fullString.midRef(cutoff);
 #endif
-            if(stringPart.size())
+
-            {
+            if (!stringPart.isEmpty()) {
+                m_isNull = false;
                 m_stringPart = stringPart.toString();
             }
         }
-        explicit Section() {}
+
-        bool numValid = false;
+        explicit Section() = default;
+
+        bool m_isNull = true;
+
         int m_numPart = 0;
         QString m_stringPart;
+
         QString m_fullString;
 
-        inline bool operator!=(const Section &other) const
+        [[nodiscard]] inline bool isAppendix() const { return m_stringPart.startsWith('+'); }
+        [[nodiscard]] inline bool isPreRelease() const { return m_stringPart.startsWith('-') && m_stringPart.length() > 1; }
+
+        inline bool operator==(const Section& other) const
         {
-            if(numValid && other.numValid)
+            if (m_isNull && !other.m_isNull)
-            {
+                return false;
-                return m_numPart != other.m_numPart || m_stringPart != other.m_stringPart;
+            if (!m_isNull && other.m_isNull)
-            }
+                return false;
-            else
+
-            {
+            if (!m_isNull && !other.m_isNull) {
-                return m_fullString != other.m_fullString;
+                return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart);
             }
+
+            return true;
         }
+
         inline bool operator<(const Section& other) const
         {   
-            if(numValid && other.numValid)
+            static auto unequal_is_less = [](Section const& non_null) -> bool {
-            {
+                if (non_null.m_stringPart.isEmpty())
+                    return non_null.m_numPart == 0;
+                return (non_null.m_stringPart != QLatin1Char('.')) && non_null.isPreRelease();
+            };
+
+            if (!m_isNull && other.m_isNull)
+                return unequal_is_less(*this);
+            if (m_isNull && !other.m_isNull)
+                return !unequal_is_less(other);
+
+            if (!m_isNull && !other.m_isNull) {
                 if (m_numPart < other.m_numPart)
                     return true;
                 if (m_numPart == other.m_numPart && m_stringPart < other.m_stringPart)
                     return true;
+
+                if (!m_stringPart.isEmpty() && other.m_stringPart.isEmpty())
+                    return false;
+                if (m_stringPart.isEmpty() && !other.m_stringPart.isEmpty())
+                    return true;
+
                 return false;
             }
-            else
+
-            {
             return m_fullString < other.m_fullString;
         }
+
+        inline bool operator!=(const Section& other) const
+        {
+            return !(*this == other);
         }
         inline bool operator>(const Section &other) const
         {
-            if(numValid && other.numValid)
+            return !(*this < other || *this == other);
-            {
-                if(m_numPart > other.m_numPart)
-                    return true;
-                if(m_numPart == other.m_numPart && m_stringPart > other.m_stringPart)
-                    return true;
-                return false;
-            }
-            else
-            {
-                return m_fullString > other.m_fullString;
-            }
         }
     };
+
+   private:
+    QString m_string;
     QList<Section> m_sections;
 
     void parse();
 };
+
+

tests/CMakeLists.txt

@@ -56,3 +56,6 @@ ecm_add_test(Packwiz_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR
 
 ecm_add_test(Index_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test
     TEST_NAME Index)
+
+ecm_add_test(Version_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test
+    TEST_NAME Version)

tests/Version_test.cpp

@@ -15,55 +15,51 @@
 
 #include <QTest>
 
-#include <TestUtil.h>
 #include <Version.h>
 
-class ModUtilsTest : public QObject
+class VersionTest : public QObject {
-{
     Q_OBJECT
-    void setupVersions()
+
+    void addDataColumns()
     {
         QTest::addColumn<QString>("first");
         QTest::addColumn<QString>("second");
         QTest::addColumn<bool>("lessThan");
         QTest::addColumn<bool>("equal");
+    }
+
+    void setupVersions()
+    {
+        addDataColumns();
 
         QTest::newRow("equal, explicit") << "1.2.0" << "1.2.0" << false << true;
-        QTest::newRow("equal, implicit 1") << "1.2" << "1.2.0" << false << true;
-        QTest::newRow("equal, implicit 2") << "1.2.0" << "1.2" << false << true;
         QTest::newRow("equal, two-digit") << "1.42" << "1.42" << false << true;
 
         QTest::newRow("lessThan, explicit 1") << "1.2.0" << "1.2.1" << true << false;
         QTest::newRow("lessThan, explicit 2") << "1.2.0" << "1.3.0" << true << false;
         QTest::newRow("lessThan, explicit 3") << "1.2.0" << "2.2.0" << true << false;
-        QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.1" << true << false;
+        QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.0" << true << false;
-        QTest::newRow("lessThan, implicit 2") << "1.2" << "1.3.0" << true << false;
+        QTest::newRow("lessThan, implicit 2") << "1.2" << "1.2.1" << true << false;
-        QTest::newRow("lessThan, implicit 3") << "1.2" << "2.2.0" << true << false;
+        QTest::newRow("lessThan, implicit 3") << "1.2" << "1.3.0" << true << false;
+        QTest::newRow("lessThan, implicit 4") << "1.2" << "2.2.0" << true << false;
         QTest::newRow("lessThan, two-digit") << "1.41" << "1.42" << true << false;
 
         QTest::newRow("greaterThan, explicit 1") << "1.2.1" << "1.2.0" << false << false;
         QTest::newRow("greaterThan, explicit 2") << "1.3.0" << "1.2.0" << false << false;
         QTest::newRow("greaterThan, explicit 3") << "2.2.0" << "1.2.0" << false << false;
-        QTest::newRow("greaterThan, implicit 1") << "1.2.1" << "1.2" << false << false;
+        QTest::newRow("greaterThan, implicit 1") << "1.2.0" << "1.2" << false << false;
-        QTest::newRow("greaterThan, implicit 2") << "1.3.0" << "1.2" << false << false;
+        QTest::newRow("greaterThan, implicit 2") << "1.2.1" << "1.2" << false << false;
-        QTest::newRow("greaterThan, implicit 3") << "2.2.0" << "1.2" << false << false;
+        QTest::newRow("greaterThan, implicit 3") << "1.3.0" << "1.2" << false << false;
+        QTest::newRow("greaterThan, implicit 4") << "2.2.0" << "1.2" << false << false;
         QTest::newRow("greaterThan, two-digit") << "1.42" << "1.41" << false << false;
     }
 
    private slots:
-    void initTestCase()
-    {
-
-    }
-    void cleanupTestCase()
-    {
-
-    }
-
     void test_versionCompare_data()
     {
         setupVersions();
     }
+
     void test_versionCompare()
     {
         QFETCH(QString, first);
@@ -74,12 +70,92 @@ private slots:
         const auto v1 = Version(first);
         const auto v2 = Version(second);
 
+        qDebug() << v1 << "vs" << v2;
+
+        QCOMPARE(v1 < v2, lessThan);
+        QCOMPARE(v1 > v2, !lessThan && !equal);
+        QCOMPARE(v1 == v2, equal);
+    }
+
+    void test_flexVerTestVector_data()
+    {
+        addDataColumns();
+
+        QDir test_vector_dir(QFINDTESTDATA("testdata/Version"));
+
+        QFile vector_file{test_vector_dir.absoluteFilePath("test_vectors.txt")};
+
+        vector_file.open(QFile::OpenModeFlag::ReadOnly);
+
+        int test_number = 0;
+        const QString test_name_template { "FlexVer test #%1 (%2)" };
+        for (auto line = vector_file.readLine(); !vector_file.atEnd(); line = vector_file.readLine()) {
+            line = line.simplified();
+            if (line.startsWith('#') || line.isEmpty())
+                continue;
+
+            test_number += 1;
+
+            auto split_line = line.split('<');
+            if (split_line.size() == 2) {
+                QString first{split_line.first().simplified()};
+                QString second{split_line.last().simplified()};
+
+                auto new_test_name = test_name_template.arg(QString::number(test_number), "lessThan").toLatin1().data();
+                QTest::newRow(new_test_name) << first << second << true << false;
+
+                continue;
+            }
+
+            split_line = line.split('=');
+            if (split_line.size() == 2) {
+                QString first{split_line.first().simplified()};
+                QString second{split_line.last().simplified()};
+
+                auto new_test_name = test_name_template.arg(QString::number(test_number), "equals").toLatin1().data();
+                QTest::newRow(new_test_name) << first << second << false << true;
+
+                continue;
+            }
+
+            split_line = line.split('>');
+            if (split_line.size() == 2) {
+                QString first{split_line.first().simplified()};
+                QString second{split_line.last().simplified()};
+
+                auto new_test_name = test_name_template.arg(QString::number(test_number), "greaterThan").toLatin1().data();
+                QTest::newRow(new_test_name) << first << second << false << false;
+
+                continue;
+            }
+
+            qCritical() << "Unexpected separator in the test vector: ";
+            qCritical() << line;
+
+            QVERIFY(0 != 0);
+        }
+
+        vector_file.close();
+    }
+
+    void test_flexVerTestVector()
+    {
+        QFETCH(QString, first);
+        QFETCH(QString, second);
+        QFETCH(bool, lessThan);
+        QFETCH(bool, equal);
+
+        const auto v1 = Version(first);
+        const auto v2 = Version(second);
+
+        qDebug() << v1 << "vs" << v2;
+
         QCOMPARE(v1 < v2, lessThan);
         QCOMPARE(v1 > v2, !lessThan && !equal);
         QCOMPARE(v1 == v2, equal);
     }
 };
 
-QTEST_GUILESS_MAIN(ModUtilsTest)
+QTEST_GUILESS_MAIN(VersionTest)
 
 #include "Version_test.moc"