Add custom VersionNumber class

Qt's API is kind of painful, and we need some custom functionality
anyways. Wrap QVersionNumber with our own class instead of piling up
workarounds.

8 files changed, 188 insertions, 56 deletions

diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py
index 9031c9b96..04aa4ec49 100644
--- a/qutebrowser/config/configfiles.py
+++ b/qutebrowser/config/configfiles.py
@@ -128,22 +128,21 @@ class StateConfig(configparser.ConfigParser):
             # https://github.com/python/typeshed/issues/2093
             return  # type: ignore[unreachable]
 
-        old_version = utils.parse_version(old_qutebrowser_version)
-        new_version = utils.parse_version(qutebrowser.__version__)
-
-        if old_version.isNull():
+        try:
+            old_version = utils.VersionNumber.parse(old_qutebrowser_version)
+        except ValueError:
             log.init.warning(f"Unable to parse old version {old_qutebrowser_version}")
             return
 
-        assert not new_version.isNull(), qutebrowser.__version__
+        new_version = utils.VersionNumber.parse(qutebrowser.__version__)
 
         if old_version == new_version:
             self.qutebrowser_version_changed = VersionChange.equal
         elif new_version < old_version:
             self.qutebrowser_version_changed = VersionChange.downgrade
-        elif old_version.segments()[:2] == new_version.segments()[:2]:
+        elif old_version.segments[:2] == new_version.segments[:2]:
             self.qutebrowser_version_changed = VersionChange.patch
-        elif old_version.majorVersion() == new_version.majorVersion():
+        elif old_version.major == new_version.major:
             self.qutebrowser_version_changed = VersionChange.minor
         else:
             self.qutebrowser_version_changed = VersionChange.major
diff --git a/qutebrowser/misc/crashdialog.py b/qutebrowser/misc/crashdialog.py
index ac292dcdb..430553433 100644
--- a/qutebrowser/misc/crashdialog.py
+++ b/qutebrowser/misc/crashdialog.py
@@ -359,8 +359,8 @@ class _CrashDialog(QDialog):
         Args:
             newest: The newest version as a string.
         """
-        new_version = utils.parse_version(newest)
-        cur_version = utils.parse_version(qutebrowser.__version__)
+        new_version = utils.VersionNumber.parse(newest)
+        cur_version = utils.VersionNumber.parse(qutebrowser.__version__)
         lines = ['The report has been sent successfully. Thanks!']
         if new_version > cur_version:
             lines.append("<b>Note:</b> The newest available version is v{}, "
diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py
index f7c5a3ce0..01234a42b 100644
--- a/qutebrowser/utils/qtutils.py
+++ b/qutebrowser/utils/qtutils.py
@@ -98,15 +98,15 @@ def version_check(version: str,
     if compiled and exact:
         raise ValueError("Can't use compiled=True with exact=True!")
 
-    parsed = utils.parse_version(version)
+    parsed = utils.VersionNumber.parse(version)
     op = operator.eq if exact else operator.ge
-    result = op(utils.parse_version(qVersion()), parsed)
+    result = op(utils.VersionNumber.parse(qVersion()), parsed)
     if compiled and result:
         # qVersion() ==/>= parsed, now check if QT_VERSION_STR ==/>= parsed.
-        result = op(utils.parse_version(QT_VERSION_STR), parsed)
+        result = op(utils.VersionNumber.parse(QT_VERSION_STR), parsed)
     if compiled and result:
         # Finally, check PYQT_VERSION_STR as well.
-        result = op(utils.parse_version(PYQT_VERSION_STR), parsed)
+        result = op(utils.VersionNumber.parse(PYQT_VERSION_STR), parsed)
     return result
 
 
@@ -116,8 +116,8 @@ MAX_WORLD_ID = 256
 def is_new_qtwebkit() -> bool:
     """Check if the given version is a new QtWebKit."""
     assert qWebKitVersion is not None
-    return (utils.parse_version(qWebKitVersion()) >
-            utils.parse_version('538.1'))
+    return (utils.VersionNumber.parse(qWebKitVersion()) >
+            utils.VersionNumber.parse('538.1'))
 
 
 def is_single_process() -> bool:
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index 698a608ef..af9d5fad7 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -92,26 +92,74 @@ class Comparable(Protocol):
         ...
 
 
-if TYPE_CHECKING:
-    class VersionNumber(Comparable, QVersionNumber):
+class VersionNumber:
 
-        """WORKAROUND for incorrect PyQt stubs."""
-else:
-    class VersionNumber(QVersionNumber):
+    """A representation of a version number."""
 
-        """We can't inherit from Protocol and QVersionNumber at runtime."""
+    def __init__(self, *args: int) -> None:
+        self._ver = QVersionNumber(*args)
+        if self._ver.isNull():
+            raise ValueError("Can't construct a null version")
 
-        def __init__(self, *args, **kwargs):
-            super().__init__(*args, **kwargs)
-            normalized = self.normalized()
-            if normalized != self:
-                raise ValueError(
-                    f"Refusing to construct non-normalized version from {args} "
-                    f"(normalized: {tuple(normalized.segments())}).")
+        normalized = self._ver.normalized()
+        if normalized != self._ver:
+            raise ValueError(
+                f"Refusing to construct non-normalized version from {args} "
+                f"(normalized: {tuple(normalized.segments())}).")
 
-        def __repr__(self):
-            args = ", ".join(str(s) for s in self.segments())
-            return f'VersionNumber({args})'
+        self.major = self._ver.majorVersion()
+        self.minor = self._ver.minorVersion()
+        self.patch = self._ver.microVersion()
+        self.segments = self._ver.segments()
+
+        assert len(self.segments) <= 3, self.segments
+
+    def __str__(self) -> str:
+        return ".".join(str(s) for s in self.segments)
+
+    def __repr__(self) -> str:
+        args = ", ".join(str(s) for s in self.segments)
+        return f'VersionNumber({args})'
+
+    def strip_patch(self) -> 'VersionNumber':
+        """Get a new VersionNumber with the patch version removed."""
+        return VersionNumber(*self.segments[:2])
+
+    @classmethod
+    def parse(cls, s: str) -> 'VersionNumber':
+        """Parse a version number from a string."""
+        ver, _suffix = QVersionNumber.fromString(s)
+        # FIXME: Should we support a suffix?
+
+        if ver.isNull():
+            raise ValueError(f"Failed to parse {s}")
+
+        return cls(*ver.normalized().segments())
+
+    def __hash__(self) -> int:
+        return hash(self._ver)
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, VersionNumber):
+            return NotImplemented
+        return self._ver == other._ver
+
+    def __ne__(self, other: object) -> bool:
+        if not isinstance(other, VersionNumber):
+            return NotImplemented
+        return self._ver != other._ver
+
+    def __ge__(self, other: 'VersionNumber') -> bool:
+        return self._ver >= other._ver  # type: ignore[operator]
+
+    def __gt__(self, other: 'VersionNumber') -> bool:
+        return self._ver > other._ver  # type: ignore[operator]
+
+    def __le__(self, other: 'VersionNumber') -> bool:
+        return self._ver <= other._ver  # type: ignore[operator]
+
+    def __lt__(self, other: 'VersionNumber') -> bool:
+        return self._ver < other._ver  # type: ignore[operator]
 
 
 class Unreachable(Exception):
@@ -294,12 +342,6 @@ def read_file_binary(filename: str) -> bytes:
         return path.read_bytes()
 
 
-def parse_version(version: str) -> VersionNumber:
-    """Parse a version string."""
-    ver, _suffix = QVersionNumber.fromString(version)
-    return VersionNumber(ver.normalized())
-
-
 def format_seconds(total_seconds: int) -> str:
     """Format a count of seconds to get a [H:]M:SS string."""
     prefix = '-' if total_seconds < 0 else ''
diff --git a/qutebrowser/utils/version.py b/qutebrowser/utils/version.py
index a10f50ffc..4a4455c99 100644
--- a/qutebrowser/utils/version.py
+++ b/qutebrowser/utils/version.py
@@ -160,7 +160,7 @@ def distribution() -> Optional[DistributionInfo]:
     dist_version: Optional[utils.VersionNumber] = None
     for version_key in ['VERSION', 'VERSION_ID']:
         if version_key in info:
-            dist_version = utils.parse_version(info[version_key])
+            dist_version = utils.VersionNumber.parse(info[version_key])
             break
 
     dist_id = info.get('ID', None)
@@ -568,7 +568,7 @@ class WebEngineVersions:
     }
 
     def __str__(self) -> str:
-        s = f'QtWebEngine {self.webengine.toString()}'
+        s = f'QtWebEngine {self.webengine}'
         if self.chromium is not None:
             s += f', Chromium {self.chromium}'
         if self.source != 'UA':
@@ -585,7 +585,7 @@ class WebEngineVersions:
         """
         assert ua.qt_version is not None, ua
         return cls(
-            webengine=utils.parse_version(ua.qt_version),
+            webengine=utils.VersionNumber.parse(ua.qt_version),
             chromium=ua.upstream_browser_version,
             source='UA',
         )
@@ -602,7 +602,7 @@ class WebEngineVersions:
         (though hackish) way to get a more accurate result.
         """
         return cls(
-            webengine=utils.parse_version(versions.webengine),
+            webengine=utils.VersionNumber.parse(versions.webengine),
             chromium=versions.chromium,
             source='ELF',
         )
@@ -624,11 +624,7 @@ class WebEngineVersions:
             minor_version = v5_15_3
         else:
             # e.g. 5.14.2 -> 5.14
-            segments = pyqt_webengine_version.segments()[:2]
-            if segments[-1] == 0:
-                del segments[-1]  # pragma: no cover
-
-            minor_version = utils.VersionNumber(*segments)
+            minor_version = pyqt_webengine_version.strip_patch()
 
         return cls._CHROMIUM_VERSIONS.get(minor_version)
 
@@ -651,7 +647,7 @@ class WebEngineVersions:
         Note that we only can get the PyQtWebEngine version with PyQt 5.13 or newer.
         With Qt 5.12, we instead rely on qVersion().
         """
-        parsed = utils.parse_version(pyqt_webengine_version)
+        parsed = utils.VersionNumber.parse(pyqt_webengine_version)
         return cls(
             webengine=parsed,
             chromium=cls._infer_chromium_version(parsed),
diff --git a/tests/end2end/test_invocations.py b/tests/end2end/test_invocations.py
index f3d74d1f0..38e40f9b7 100644
--- a/tests/end2end/test_invocations.py
+++ b/tests/end2end/test_invocations.py
@@ -657,7 +657,7 @@ def test_dark_mode(webengine_versions, quteproc_new, request,
     quteproc_new.start(args)
 
     ver = webengine_versions.webengine
-    minor_version = f'{ver.majorVersion()}.{ver.minorVersion()}'
+    minor_version = str(ver.strip_patch())
     expected = colors.get(minor_version, colors[None])
 
     quteproc_new.open_path(f'data/darkmode/{filename}.html')
diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py
index 4cf60943c..a64622dee 100644
--- a/tests/unit/utils/test_utils.py
+++ b/tests/unit/utils/test_utils.py
@@ -29,6 +29,7 @@ import re
 import shlex
 import math
 import zipfile
+import operator
 
 from PyQt5.QtCore import QUrl, QRect
 from PyQt5.QtGui import QClipboard
@@ -44,19 +45,113 @@ from qutebrowser.utils import utils, version, usertypes
 
 class TestVersionNumber:
 
-    @pytest.mark.parametrize('args, expected', [
-        ([5, 15, 2], 'VersionNumber(5, 15, 2)'),
-        ([5, 15], 'VersionNumber(5, 15)'),
-        ([5], 'VersionNumber(5)'),
+    @pytest.mark.parametrize('num, expected', [
+        (utils.VersionNumber(5, 15, 2), 'VersionNumber(5, 15, 2)'),
+        (utils.VersionNumber(5, 15), 'VersionNumber(5, 15)'),
+        (utils.VersionNumber(5), 'VersionNumber(5)'),
     ])
-    def test_repr(self, args, expected):
-        num = utils.VersionNumber(*args)
+    def test_repr(self, num, expected):
         assert repr(num) == expected
 
+    @pytest.mark.parametrize('num, expected', [
+        (utils.VersionNumber(5, 15, 2), '5.15.2'),
+        (utils.VersionNumber(5, 15), '5.15'),
+        (utils.VersionNumber(5), '5'),
+    ])
+    def test_str(self, num, expected):
+        assert str(num) == expected
+
     def test_not_normalized(self):
         with pytest.raises(ValueError, match='Refusing to construct'):
             utils.VersionNumber(5, 15, 0)
 
+    @pytest.mark.parametrize('num, expected', [
+        (utils.VersionNumber(5, 15, 2), utils.VersionNumber(5, 15)),
+        (utils.VersionNumber(5, 15), utils.VersionNumber(5, 15)),
+        (utils.VersionNumber(6), utils.VersionNumber(6)),
+    ])
+    def test_strip_patch(self, num, expected):
+        assert num.strip_patch() == expected
+
+    @pytest.mark.parametrize('s, expected', [
+        ('1x6.2', utils.VersionNumber(1)),
+        ('6', utils.VersionNumber(6)),
+        ('5.15', utils.VersionNumber(5, 15)),
+        ('5.15.3', utils.VersionNumber(5, 15, 3)),
+        ('5.15.3.dev1234', utils.VersionNumber(5, 15, 3)),
+    ])
+    def test_parse_valid(self, s, expected):
+        assert utils.VersionNumber.parse(s) == expected
+
+    @pytest.mark.parametrize('s, message', [
+        ('foo6', "Failed to parse foo6"),
+        ('.6', "Failed to parse .6"),
+        ('0x6.2', "Can't construct a null version"),
+    ])
+    def test_parse_invalid(self, s, message):
+        with pytest.raises(ValueError, match=message):
+            utils.VersionNumber.parse(s)
+
+    @pytest.mark.parametrize('lhs, op, rhs, outcome', [
+        # ==
+        (utils.VersionNumber(6), operator.eq, utils.VersionNumber(6), True),
+        (utils.VersionNumber(6), operator.eq, object(), False),
+
+        # !=
+        (utils.VersionNumber(6), operator.ne, utils.VersionNumber(5), True),
+        (utils.VersionNumber(6), operator.ne, object(), True),
+
+        # >=
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 13, 5), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 14, 2), False),
+        (utils.VersionNumber(5, 14, 3), operator.ge, utils.VersionNumber(5, 14, 2), True),
+        (utils.VersionNumber(5, 14, 3), operator.ge, utils.VersionNumber(5, 14, 3), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 13), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 14), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5, 15), False),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(4), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(5), True),
+        (utils.VersionNumber(5, 14), operator.ge, utils.VersionNumber(6), False),
+
+        # >
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 13, 5), True),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 14, 2), False),
+        (utils.VersionNumber(5, 14, 3), operator.gt, utils.VersionNumber(5, 14, 2), True),
+        (utils.VersionNumber(5, 14, 3), operator.gt, utils.VersionNumber(5, 14, 3), False),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 13), True),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 14), False),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5, 15), False),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(4), True),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(5), True),
+        (utils.VersionNumber(5, 14), operator.gt, utils.VersionNumber(6), False),
+
+        # <=
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 13, 5), False),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 14, 2), True),
+        (utils.VersionNumber(5, 14, 3), operator.le, utils.VersionNumber(5, 14, 2), False),
+        (utils.VersionNumber(5, 14, 3), operator.le, utils.VersionNumber(5, 14, 3), True),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 13), False),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 14), True),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5, 15), True),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(4), False),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(5), False),
+        (utils.VersionNumber(5, 14), operator.le, utils.VersionNumber(6), True),
+
+        # <
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 13, 5), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 14, 2), True),
+        (utils.VersionNumber(5, 14, 3), operator.lt, utils.VersionNumber(5, 14, 2), False),
+        (utils.VersionNumber(5, 14, 3), operator.lt, utils.VersionNumber(5, 14, 3), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 13), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 14), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5, 15), True),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(4), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(5), False),
+        (utils.VersionNumber(5, 14), operator.lt, utils.VersionNumber(6), True),
+    ])
+    def test_comparisons(self, lhs, op, rhs, outcome):
+        assert op(lhs, rhs) == outcome
+
 
 ELLIPSIS = '\u2026'
 
@@ -784,7 +879,7 @@ class TestOpenFile:
         info = version.DistributionInfo(
             id='org.kde.Platform',
             parsed=version.Distribution.kde_flatpak,
-            version=utils.parse_version('5.12'),
+            version=utils.VersionNumber.parse('5.12'),
             pretty='Unknown')
         monkeypatch.setattr(version, 'distribution',
                             lambda: info)
diff --git a/tests/unit/utils/test_version.py b/tests/unit/utils/test_version.py
index b7da60011..39b898d59 100644
--- a/tests/unit/utils/test_version.py
+++ b/tests/unit/utils/test_version.py
@@ -962,7 +962,7 @@ class TestWebEngineVersions:
     ])
     def test_from_pyqt(self, qt_version, chromium_version):
         expected = version.WebEngineVersions(
-            webengine=utils.parse_version(qt_version),
+            webengine=utils.VersionNumber.parse(qt_version),
             chromium=chromium_version,
             source='PyQt',
         )