Repository: MarketSquare/robotframework-robocop
Pull request #1024 (2023-12-27 09:38:26)

Handle RF development version as final version
With previous implementation there was problem where some of our tests didn't run at all. For example if we used ">=7" version specifier, RF "7.0a1" didn't match it. I have decided to just drop support for development versions in version specifier and handle those as 'final' versions. Parsed version (the one from RF) can have development suffix (dev1, rc1..) but it will be ignored.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1024?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`bb84188`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/commit/bb841889b758e8619ad55886ff09dd4cffe41b66?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 96.40% compared to head [(`4f5f39a`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1024?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 94.83%.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1024      +/-   ##
==========================================
- Coverage   96.40%   94.83%   -1.57%     
==========================================
  Files          37       37              
  Lines        4565     4515      -50     
==========================================
- Hits         4401     4282     -119     
- Misses        164      233      +69     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1024?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).


Patch:
diff --git a/docs/releasenotes/unreleased/other.2.rst b/docs/releasenotes/unreleased/other.2.rst
new file mode 100644
index 0000000..b2f213c
--- /dev/null
+++ b/docs/releasenotes/unreleased/other.2.rst
@@ -0,0 +1,19 @@
+Consider Robot Framework development version as final version in rule version matching
+---------------------------------------------------------------------------------------
+
+Robot Framework development version (for example 7.0rc1) is considered as final version (for example 7.0) in our
+version matcher. Thanks to this change it is easier to test rules with not released version - it's not required
+to define exact version specifier anymore (for example '==7.0rc1').
+
+It's internal change but can have effect on your custom rules if you have rule version specifier using development
+version:
+
+```
+"XYZ": Rule(
+    rule_id="XYZ",
+    name="custom-rule",
+    msg="custom-message",
+    severity=RuleSeverity.WARNING,
+    version=">=4.0alpha",  # it is not allowed anymore, use >=4.0 instead
+    )
+```
diff --git a/robocop/utils/version_matching.py b/robocop/utils/version_matching.py
index 3718bc7..e5ddb53 100644
--- a/robocop/utils/version_matching.py
+++ b/robocop/utils/version_matching.py
@@ -25,77 +25,14 @@ VERSION_PATTERN = r"""
 """
 
 
-class InfinityType:
-    def __lt__(self, other: object) -> bool:
-        return False
-
-    def __le__(self, other: object) -> bool:
-        return False
-
-    def __eq__(self, other: object) -> bool:
-        return isinstance(other, self.__class__)
-
-    def __gt__(self, other: object) -> bool:
-        return True
-
-    def __ge__(self, other: object) -> bool:
-        return True
-
-    def __neg__(self: object) -> "NegativeInfinityType":
-        return NegativeInfinity
-
-
-class NegativeInfinityType:
-    def __lt__(self, other: object) -> bool:
-        return True
-
-    def __le__(self, other: object) -> bool:
-        return True
-
-    def __eq__(self, other: object) -> bool:
-        return isinstance(other, self.__class__)
-
-    def __gt__(self, other: object) -> bool:
-        return False
-
-    def __ge__(self, other: object) -> bool:
-        return False
-
-    def __neg__(self: object) -> InfinityType:
-        return Infinity
-
-
-Infinity = InfinityType()
-NegativeInfinity = NegativeInfinityType()
-
-
-def _get_comparison_key(
-    release: Tuple[int, ...],
-    pre: Optional[Tuple[str, int]],
-    dev: Optional[Tuple[str, int]],
-):
+def _get_comparison_key(release: Tuple[int, ...]):
     # When we compare a release version, we want to compare it with all the
     # trailing zeros removed. So we'll use a reverse the list, drop all the now
     # leading zeros until we come to something non zero, then take the rest
     # re-reverse it back into the correct order and make it a tuple and use
     # that for our sorting key.
     _release = tuple(reversed(list(itertools.dropwhile(lambda x: x == 0, reversed(release)))))
-
-    # We need to "trick" the sorting algorithm to put 1.0.dev0 before 1.0a0.
-    # We'll do this by abusing the pre segment, but we _only_ want to do this
-    # if there is not a pre segment. Otherwise, the normal sorting rules will handle this case correctly.
-    if pre is None and dev is not None:
-        _pre = NegativeInfinity
-    # Versions without a pre-release (except as noted above) should sort after
-    # those with one.
-    elif pre is None:
-        _pre = Infinity
-    else:
-        _pre = pre
-
-    # Versions without a development segment should sort after those with one.
-    _dev = Infinity if dev is None else dev
-    return _release, _pre, _dev
+    return _release
 
 
 def _parse_letter_version(letter: str, number: Union[str, bytes, SupportsInt]) -> Optional[Tuple[str, int]]:
@@ -128,11 +65,7 @@ class Version:
         self.release = tuple(int(i) for i in match.group("release").split("."))
         self.pre = _parse_letter_version(match.group("pre_l"), match.group("pre_n"))
         self._dev = _parse_letter_version(match.group("dev_l"), match.group("dev_n"))
-        self._key = _get_comparison_key(
-            self.release,
-            self.pre,
-            self._dev,
-        )
+        self._key = _get_comparison_key(self.release)
 
     def __lt__(self, other) -> bool:
         return self._key < other._key
@@ -141,18 +74,7 @@ class Version:
         return self._key == other._key
 
     def __str__(self) -> str:
-        parts = [".".join(str(x) for x in self.release)]
-        if self.pre is not None:
-            parts.append("".join(str(x) for x in self.pre))
-
-        if self.dev is not None:
-            parts.append(f".dev{self.dev}")
-
-        return "".join(parts)
-
-    @property
-    def dev(self) -> Optional[int]:
-        return self._dev[1] if self._dev else None
+        return ".".join(str(x) for x in self.release)
 
     @property
     def public(self) -> str:
@@ -163,10 +85,6 @@ class Version:
         parts = [".".join(str(x) for x in self.release)]
         return "".join(parts)
 
-    @property
-    def is_prerelease(self) -> bool:
-        return self.dev is not None or self.pre is not None
-
     @property
     def major(self) -> int:
         return self.release[0] if len(self.release) >= 1 else 0
@@ -229,18 +147,9 @@ class VersionSpecifier:
                 \s*
                 v?
                 [0-9]+(?:\.[0-9]+)*   # release
-                (?:                   # pre release
-                    [-_\.]?
-                    (a|b|c|rc|alpha|beta|pre|preview)
-                    [-_\.]?
-                    [0-9]*
-                )?
                 # You cannot use a wild card and a dev or local version
                 # together so group them with a | and make them optional.
                 (?:
-                    (?:[-_\.]?dev[-_\.]?[0-9]*)?         # dev release
-                    (?:\+[a-z0-9]+(?:[-_\.][a-z0-9]+)*)? # local
-                    |
                     \.\*  # Wild card syntax of .*
                 )?
             )
@@ -253,13 +162,6 @@ class VersionSpecifier:
                 \s*
                 v?
                 [0-9]+(?:\.[0-9]+)+   # release  (We have a + instead of a *)
-                (?:                   # pre release
-                    [-_\.]?
-                    (a|b|c|rc|alpha|beta|pre|preview)
-                    [-_\.]?
-                    [0-9]*
-                )?
-                (?:[-_\.]?dev[-_\.]?[0-9]*)?          # dev release
             )
             |
             (?:
@@ -274,13 +176,6 @@ class VersionSpecifier:
                 \s*
                 v?
                 [0-9]+(?:\.[0-9]+)*   # release
-                (?:                   # pre release
-                    [-_\.]?
-                    (a|b|c|rc|alpha|beta|pre|preview)
-                    [-_\.]?
-                    [0-9]*
-                )?
-                (?:[-_\.]?dev[-_\.]?[0-9]*)?          # dev release
             )
         )
         """
@@ -384,12 +279,7 @@ class VersionSpecifier:
         return Version(prospective.public) >= Version(spec)
 
     def _compare_less_than(self, prospective, spec_str: str) -> bool:
-        spec = Version(spec_str)
-        if not prospective < spec:
-            return False
-        if not spec.is_prerelease and prospective.is_prerelease:
-            return Version(prospective.base_version) != Version(spec.base_version)
-        return True
+        return prospective < Version(spec_str)
 
     def _compare_greater_than(self, prospective, spec_str: str) -> bool:
         return prospective <= Version(spec_str)


Test patch:
diff --git a/tests/atest/rules/errors/parsing_error/expected_output_rf7.txt b/tests/atest/rules/errors/parsing_error/expected_output_rf7.txt
index 7c52e2e..9201125 100644
--- a/tests/atest/rules/errors/parsing_error/expected_output_rf7.txt
+++ b/tests/atest/rules/errors/parsing_error/expected_output_rf7.txt
@@ -14,8 +14,8 @@ invalid_try_except.robot:36:1 [E] 0401 Robot Framework syntax error: TRY does no
 invalid_try_except.robot:48:1 [E] 0401 Robot Framework syntax error: EXCEPT branch cannot be empty.
 invalid_try_except.robot:57:1 [E] 0401 Robot Framework syntax error: EXCEPT without patterns must be last.
 invalid_try_except.robot:69:1 [E] 0401 Robot Framework syntax error: Only one EXCEPT without patterns allowed.
-invalid_try_except.robot:83:1 [E] 0401 Robot Framework syntax error: EXCEPT AS requires variable.
-invalid_try_except.robot:91:1 [E] 0401 Robot Framework syntax error: EXCEPT AS accepts only one variable.
+invalid_try_except.robot:83:1 [E] 0401 Robot Framework syntax error: EXCEPT AS requires a value.
+invalid_try_except.robot:91:1 [E] 0401 Robot Framework syntax error: EXCEPT AS accepts only one value.
 invalid_try_except.robot:99:1 [E] 0401 Robot Framework syntax error: EXCEPT AS variable 'foo' is invalid.
 invalid_try_except.robot:109:1 [E] 0401 Robot Framework syntax error: ELSE does not accept arguments, got 'I should not be here'.
 invalid_try_except.robot:121:1 [E] 0401 Robot Framework syntax error: ELSE branch cannot be empty.
diff --git a/tests/atest/rules/errors/parsing_error/test_rule.py b/tests/atest/rules/errors/parsing_error/test_rule.py
index a4c25e9..2666991 100644
--- a/tests/atest/rules/errors/parsing_error/test_rule.py
+++ b/tests/atest/rules/errors/parsing_error/test_rule.py
@@ -3,7 +3,7 @@ from tests.atest.utils import RuleAcceptance
 
 class TestRule(RuleAcceptance):
     def test_rule(self):
-        self.check_rule(expected_file="expected_output.txt", target_version=">=7")
+        self.check_rule(expected_file="expected_output_rf7.txt", target_version=">=7")
 
     def test_rule_rf61(self):
         self.check_rule(expected_file="expected_output.txt", target_version="==6.1.*")
diff --git a/tests/utest/test_utils.py b/tests/utest/test_utils.py
index f57f4df..a43bd7b 100644
--- a/tests/utest/test_utils.py
+++ b/tests/utest/test_utils.py
@@ -219,17 +219,14 @@ class TestMisc:
     [
         ("6.0.2", ">=6", True),
         ("6.1a2.dev1", ">=6", True),
-        ("6.0rc2.dev1", ">=6", False),
-        ("6.0pre2.dev1", ">=6", False),
+        ("6.0rc2.dev1", ">=6", True),
+        ("6.0pre2.dev1", ">=6", True),
         ("6.0.2", "<5", False),
         ("6.1a2.dev1", "<=5", False),
         ("6.1a2.dev1", "~=5.0", False),
         ("6.0rc2.dev1", "<=6", True),
         ("6.0rc2.dev", "<=6", True),
-        ("5.0", "<=5dev1", False),
-        ("5.0", ">5dev1", False),
-        ("5.0", ">5.0.dev1", False),
-        ("6.0dev0", ">=6", False),
+        ("6.0dev0", ">=6", True),
         ("6.0", ">=6", True),
         ("6", ">=6", True),
         ("5.0.2", ">=6", False),
@@ -261,12 +258,8 @@ def test_version_parsing_and_comparison():
     lower_version = Version("3")
     for version in versions:
         assert (version.major, version.minor, version.micro) == (4, 0, 0)
-        if version.dev is None:
-            assert version == equal_version
-            assert version >= equal_version
-        else:
-            assert version != equal_version
-            assert not version >= equal_version
+        assert version == equal_version
+        assert version >= equal_version
         assert version <= equal_version
         assert version < higher_version
         assert not version > higher_version