Fix #309: Make some functions private

tomschr · tomschr · commit 42fd668df853 · 2020-11-08T20:49:40.000+01:00
Make private function in semver.version module:

* Rename comparator -&gt; _comparator, cmd -&gt; _cmd
* Remove ensure_str and integrate it into Version.parse
* Rework tests in test_typeerror-274.py
* Move _nat_cmp to Version and make it a class method
* Remove private functions from API documentation
diff --git a/changelog.d/309.trivial.rst b/changelog.d/309.trivial.rst
@@ -0,0 +1,17 @@
+Some (private) functions from the :mod:`semver.version`
+module has been changed.
+
+The following functions got renamed:
+
+* function ``semver.version.comparator`` got renamed to
+  :func:`semver.version._comparator` as it is only useful
+  inside the :class:`~semver.version.Version` class.
+* function ``semver.version.cmp`` got renamed to
+  :func:`semver.version._cmp` as it is only useful
+  inside the :class:`~semver.version.Version` class.
+
+The following functions got integrated into the
+:class:`~semver.version.Version` class:
+
+* function ``semver.version._nat_cmd`` as a classmethod
+* function ``semver.version.ensure_str``
diff --git a/docs/api.rst b/docs/api.rst
@@ -50,12 +50,6 @@ Version Handling :mod:`semver.version`
 
 .. automodule:: semver.version
 
-.. autofunction:: semver.version.cmp
-
-.. autofunction:: semver.version.ensure_str
-
-.. autofunction:: semver.version.comparator
-
 .. autoclass:: semver.version.VersionInfo
 
 .. autoclass:: semver.version.Version
diff --git a/src/semver/version.py b/src/semver/version.py
@@ -29,37 +29,7 @@
 Comparator = Callable[["Version", Comparable], bool]
 
 
-def cmp(a, b):  # TODO: type hints
-    """Return negative if a<b, zero if a==b, positive if a>b."""
-    return (a > b) - (a < b)
-
-
-def ensure_str(s: String, encoding="utf-8", errors="strict") -> str:
-    # Taken from six project
-    """
-    Coerce *s* to `str`.
-
-    * `str` -> `str`
-    * `bytes` -> decoded to `str`
-
-    :param s: the string to convert
-    :param encoding: the encoding to apply, defaults to "utf-8"
-    :param errors: set a different error handling scheme,
-       defaults to "strict".
-       Other possible values are `ignore`, `replace`, and
-       `xmlcharrefreplace` as well as any other name
-       registered with :func:`codecs.register_error`.
-    :raises TypeError: if ``s`` is not str or bytes type
-    :return: the converted string
-    """
-    if isinstance(s, bytes):
-        s = s.decode(encoding, errors)
-    elif not isinstance(s, String.__args__):  # type: ignore
-        raise TypeError("not expecting type '%s'" % type(s))
-    return s
-
-
-def comparator(operator: Comparator) -> Comparator:
+def _comparator(operator: Comparator) -> Comparator:
     """Wrap a Version binary op method in a type-check."""
 
     @wraps(operator)
@@ -78,31 +48,9 @@ def wrapper(self: "Version", other: Comparable) -> bool:
     return wrapper
 
 
-def _nat_cmp(a, b):  # TODO: type hints
-    def convert(text):
-        return int(text) if re.match("^[0-9]+$", text) else text
-
-    def split_key(key):
-        return [convert(c) for c in key.split(".")]
-
-    def cmp_prerelease_tag(a, b):
-        if isinstance(a, int) and isinstance(b, int):
-            return cmp(a, b)
-        elif isinstance(a, int):
-            return -1
-        elif isinstance(b, int):
-            return 1
-        else:
-            return cmp(a, b)
-
-    a, b = a or "", b or ""
-    a_parts, b_parts = split_key(a), split_key(b)
-    for sub_a, sub_b in zip(a_parts, b_parts):
-        cmp_result = cmp_prerelease_tag(sub_a, sub_b)
-        if cmp_result != 0:
-            return cmp_result
-    else:
-        return cmp(len(a), len(b))
+def _cmd(a, b):  # TODO: type hints
+    """Return negative if a<b, zero if a==b, positive if a>b."""
+    return (a > b) - (a < b)
 
 
 class Version:
@@ -165,6 +113,29 @@ def __init__(
         self._prerelease = None if prerelease is None else str(prerelease)
         self._build = None if build is None else str(build)
 
+    @classmethod
+    def _nat_cmp(cls, a, b):  # TODO: type hints
+        def cmp_prerelease_tag(a, b):
+            if isinstance(a, int) and isinstance(b, int):
+                return _cmd(a, b)
+            elif isinstance(a, int):
+                return -1
+            elif isinstance(b, int):
+                return 1
+            else:
+                return _cmd(a, b)
+
+        a, b = a or "", b or ""
+        a_parts, b_parts = a.split("."), b.split(".")
+        a_parts = [int(x) if re.match(r"^\d+$", x) else x for x in a_parts]
+        b_parts = [int(x) if re.match(r"^\d+$", x) else x for x in b_parts]
+        for sub_a, sub_b in zip(a_parts, b_parts):
+            cmp_result = cmp_prerelease_tag(sub_a, sub_b)
+            if cmp_result != 0:
+                return cmp_result
+        else:
+            return _cmd(len(a), len(b))
+
     @property
     def major(self) -> int:
         """The major part of a version (read-only)."""
@@ -381,12 +352,12 @@ def compare(self, other: Comparable) -> int:
 
         v1 = self.to_tuple()[:3]
         v2 = other.to_tuple()[:3]
-        x = cmp(v1, v2)
+        x = _cmd(v1, v2)
         if x:
             return x
 
         rc1, rc2 = self.prerelease, other.prerelease
-        rccmp = _nat_cmp(rc1, rc2)
+        rccmp = self._nat_cmp(rc1, rc2)
 
         if not rccmp:
             return 0
@@ -444,27 +415,27 @@ def next_version(self, part: str, prerelease_token: str = "rc") -> "Version":
             version = version.bump_patch()
         return version.bump_prerelease(prerelease_token)
 
-    @comparator
+    @_comparator
     def __eq__(self, other: Comparable) -> bool:  # type: ignore
         return self.compare(other) == 0
 
-    @comparator
+    @_comparator
     def __ne__(self, other: Comparable) -> bool:  # type: ignore
         return self.compare(other) != 0
 
-    @comparator
+    @_comparator
     def __lt__(self, other: Comparable) -> bool:
         return self.compare(other) < 0
 
-    @comparator
+    @_comparator
     def __le__(self, other: Comparable) -> bool:
         return self.compare(other) <= 0
 
-    @comparator
+    @_comparator
     def __gt__(self, other: Comparable) -> bool:
         return self.compare(other) > 0
 
-    @comparator
+    @_comparator
     def __ge__(self, other: Comparable) -> bool:
         return self.compare(other) >= 0
 
@@ -593,15 +564,20 @@ def parse(cls, version: String) -> "Version":
         :param version: version string
         :return: a new :class:`Version` instance
         :raises ValueError: if version is invalid
+        :raises TypeError: if version contains the wrong type
 
         >>> semver.Version.parse('3.4.5-pre.2+build.4')
         Version(major=3, minor=4, patch=5, \
 prerelease='pre.2', build='build.4')
         """
-        version_str = ensure_str(version)
-        match = cls._REGEX.match(version_str)
+        if isinstance(version, bytes):
+            version = version.decode("UTF-8")
+        elif not isinstance(version, String.__args__):  # type: ignore
+            raise TypeError("not expecting type '%s'" % type(version))
+
+        match = cls._REGEX.match(version)
         if match is None:
-            raise ValueError(f"{version_str} is not valid SemVer string")
+            raise ValueError(f"{version} is not valid SemVer string")
 
         matched_version_parts: Dict[str, Any] = match.groupdict()
 
diff --git a/tests/test_typeerror-274.py b/tests/test_typeerror-274.py
@@ -1,95 +1,14 @@
-import sys
-
 import pytest
-
 import semver
-import semver.version
-
-PY2 = sys.version_info[0] == 2
-PY3 = sys.version_info[0] == 3
-
-
-def ensure_binary(s, encoding="utf-8", errors="strict"):
-    """
-    Coerce ``s`` to bytes.
-
-    * `str` -> encoded to `bytes`
-    * `bytes` -> `bytes`
-
-    :param s: the string to convert
-    :type s: str | bytes
-    :param encoding: the encoding to apply, defaults to "utf-8"
-    :type encoding: str
-    :param errors: set a different error handling scheme;
-      other possible values are `ignore`, `replace`, and
-      `xmlcharrefreplace` as well as any other name
-      registered with :func:`codecs.register_error`.
-      Defaults to "strict".
-    :type errors: str
-    :raises TypeError: if ``s`` is not str or bytes type
-    :return: the converted string
-    :rtype: str
-    """
-    if isinstance(s, str):
-        return s.encode(encoding, errors)
-    elif isinstance(s, bytes):
-        return s
-    else:
-        raise TypeError("not expecting type '%s'" % type(s))
 
 
-def test_should_work_with_string_and_unicode():
+def test_should_work_with_string_and_bytes():
     result = semver.compare("1.1.0", b"1.2.2")
     assert result == -1
     result = semver.compare(b"1.1.0", "1.2.2")
     assert result == -1
 
 
-class TestEnsure:
-    # From six project
-    # grinning face emoji
-    UNICODE_EMOJI = "\U0001F600"
-    BINARY_EMOJI = b"\xf0\x9f\x98\x80"
-
-    def test_ensure_binary_raise_type_error(self):
-        with pytest.raises(TypeError):
-            semver.version.ensure_str(8)
-
-    def test_errors_and_encoding(self):
-        ensure_binary(self.UNICODE_EMOJI, encoding="latin-1", errors="ignore")
-        with pytest.raises(UnicodeEncodeError):
-            ensure_binary(self.UNICODE_EMOJI, encoding="latin-1", errors="strict")
-
-    def test_ensure_binary_raise(self):
-        converted_unicode = ensure_binary(
-            self.UNICODE_EMOJI, encoding="utf-8", errors="strict"
-        )
-        converted_binary = ensure_binary(
-            self.BINARY_EMOJI, encoding="utf-8", errors="strict"
-        )
-
-        # PY3: str -> bytes
-        assert converted_unicode == self.BINARY_EMOJI and isinstance(
-            converted_unicode, bytes
-        )
-        # PY3: bytes -> bytes
-        assert converted_binary == self.BINARY_EMOJI and isinstance(
-            converted_binary, bytes
-        )
-
-    def test_ensure_str(self):
-        converted_unicode = semver.version.ensure_str(
-            self.UNICODE_EMOJI, encoding="utf-8", errors="strict"
-        )
-        converted_binary = semver.version.ensure_str(
-            self.BINARY_EMOJI, encoding="utf-8", errors="strict"
-        )
-
-        # PY3: str -> str
-        assert converted_unicode == self.UNICODE_EMOJI and isinstance(
-            converted_unicode, str
-        )
-        # PY3: bytes -> str
-        assert converted_binary == self.UNICODE_EMOJI and isinstance(
-            converted_unicode, str
-        )
+def test_should_not_work_with_invalid_args():
+    with pytest.raises(TypeError):
+        semver.version.Version.parse(8)
