From d17d40293de158a73b5e9621e50e97519e80e582 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:07:16 +0000
Subject: [PATCH 1/7] Add tests for Expr comparison and PrefixRelOp negate


---

diff --git a/tests/test_expressions.py b/tests/test_expressions.py
new file mode 100644
index 0000000..1d59340
--- /dev/null
+++ b/tests/test_expressions.py
@@ -0,0 +1,116 @@
+from opam2rpm.opamparser import (
+    TokenType,
+    Token,
+    BoolVal,
+    IntVal,
+    StrVal,
+    ListVal,
+    DictVal,
+    Identifier,
+    NotOp,
+    DefinedOp,
+    PrefixRelOp,
+    RelOp,
+    AndOp,
+    OrOp,
+    EnvOp,
+    Option,
+    Section,
+)
+
+
+EXPRESSIONS = [
+    None,
+    1,
+    2,
+    "asdf",
+    BoolVal(True),
+    BoolVal(False),
+    IntVal(1),
+    IntVal(-512),
+    StrVal(""),
+    StrVal("bar"),
+    ListVal([]),
+    ListVal([StrVal("baz")]),
+    DictVal({"a": 1}),
+    DictVal({}),
+    Identifier("foo"),
+    Identifier("with-doc"),
+    NotOp(StrVal("a")),
+    NotOp(IntVal(1)),
+    DefinedOp(StrVal("with-foo")),
+    DefinedOp(Option(StrVal("build"), IntVal(3))),
+    PrefixRelOp(Token(TokenType.GEQ, ">="), StrVal("libfoo")),
+    PrefixRelOp(Token(TokenType.NOT, "!"), BoolVal(True)),
+    RelOp(Token(TokenType.LT, "<"), IntVal(1), IntVal(2)),
+    RelOp(Token(TokenType.GT, ">"), IntVal(1), IntVal(4)),
+    AndOp(StrVal("foo"), IntVal(1)),
+    AndOp(
+        RelOp(Token(TokenType.NEQ, "!="), StrVal("3"), IntVal(-18)),
+        BoolVal(True),
+    ),
+    OrOp(StrVal("libfoo"), StrVal("libbar")),
+    OrOp(IntVal(1), StrVal("baz")),
+    EnvOp(Token(TokenType.OR, "|"), StrVal("bar"), IntVal(16)),
+    EnvOp(Token(TokenType.AND, "&"), IntVal(2), IntVal(42)),
+    Option(StrVal("dune"), Identifier("with-dependency")),
+    Option(BoolVal(True), IntVal(-42)),
+    Section(StrVal("build"), ListVal([StrVal("build")])),
+    Section(StrVal("clean"), IntVal(8)),
+]
+
+
+def test_cross_comparisons():
+    for i, expr_i in enumerate(EXPRESSIONS):
+        for j, expr_j in enumerate(EXPRESSIONS):
+            if i == j:
+                assert expr_i == expr_j
+            else:
+                assert expr_i != expr_j
+
+
+class TestPrefixRelOp:
+    def test_negate(self):
+        term = StrVal("irrelevant")
+        # = -> !=
+        assert PrefixRelOp(
+            Token(TokenType.EQ, "="), term
+        ).negate() == PrefixRelOp(Token(TokenType.NEQ, "="), term)
+        assert PrefixRelOp(
+            Token(TokenType.NEQ, "!="), term
+        ).negate() == PrefixRelOp(Token(TokenType.EQ, "!="), term)
+
+        # >= -> <
+        assert PrefixRelOp(
+            Token(TokenType.GEQ, ">="), term
+        ).negate() == PrefixRelOp(Token(TokenType.LT, ">="), term)
+        assert PrefixRelOp(
+            Token(TokenType.LT, "<"), term
+        ).negate() == PrefixRelOp(Token(TokenType.GEQ, "<"), term)
+
+        # <= -> >
+        assert PrefixRelOp(
+            Token(TokenType.LEQ, "<="), term
+        ).negate() == PrefixRelOp(Token(TokenType.GT, "<="), term)
+        assert PrefixRelOp(
+            Token(TokenType.GT, ">"), term
+        ).negate() == PrefixRelOp(Token(TokenType.LEQ, ">"), term)
+
+        # anything else:
+        assert PrefixRelOp(
+            Token(TokenType.NOT, "!"), term
+        ).negate() == PrefixRelOp(Token(TokenType.NOT, "!"), term)
+
+    def test_version(self):
+        assert (
+            PrefixRelOp(
+                Token(TokenType.EQ, "="), StrVal("16.1")
+            ).verdependency("foo")
+            == "foo = 16.1"
+        )
+        assert (
+            PrefixRelOp(
+                Token(TokenType.LT, "<"), StrVal("asdf")
+            ).verdependency("libinvalid")
+            == "libinvalid < 0"
+        )

From d07211965909bdce59c84d4ad282455a92899768 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:14:52 +0000
Subject: [PATCH 2/7] Add comparison & negate() to Token


---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index 1cc9c00..1e2e3c2 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -268,6 +268,7 @@ class OpamString(pp.Token):
 
         return self.strRepr
 
+
 class Token:
     """A token in an opam file.
 
@@ -278,24 +279,45 @@ class Token:
 
     __slots__ = ['tok_type', 'value']
 
-    def __init__(self, tok_type, value):
+    _NEGATED_TYPE: Dict[TokenType, TokenType] = {
+        TokenType.EQ: TokenType.NEQ,
+        TokenType.NEQ: TokenType.EQ,
+
+        TokenType.GEQ: TokenType.LT,
+        TokenType.LT: TokenType.GEQ,
+
+        TokenType.GT: TokenType.LEQ,
+        TokenType.LEQ: TokenType.GT
+    }
+
+    def __init__(self, tok_type: TokenType, value) -> None:
         """Initialize an opam token with its type and value."""
-        self.tok_type = tok_type
+        self.tok_type: TokenType = tok_type
         self.value = value
 
-    def __str__(self):
+    def __eq__(self, other: object) -> bool:
+        return (
+            (self.tok_type == other.tok_type) and (self.value == other.value)
+        ) if isinstance(other, Token) else False
+
+    def __str__(self) -> str:
         """Produce a string representation of an opam token."""
         return 'Token(' + self.tok_type.name + ', ' + str(self.value) + ')'
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         """Produce a string representation of an opam token."""
         return 'Token(' + self.tok_type.name + ', ' + repr(self.value) + ')'
 
-    def simplify(self, values, internal, seen):
+    def simplify(self, values, internal, seen) -> Token:
         """Simplify a token."""
         return self
 
-def set_bool_true():
+    def negate(self) -> Token:
+        return self if self.tok_type not in Token._NEGATED_TYPE else \
+            Token(Token._NEGATED_TYPE[self.tok_type], self.value)
+
+
+def set_bool_true() -> Token:
     """Create a boolean token with the value of true."""
     return Token(TokenType.BOOL, True)
 

From c3bda106f8e6b2800b4898004422a661dab295e0 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:25:41 +0000
Subject: [PATCH 3/7] Allow Expr child classes __eq__ to handle instances of other classes


---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index 1e2e3c2..daf5db1 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -441,9 +441,9 @@ class BoolVal(Expr):
         """Initialize a boolean value."""
         self.val = val
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two boolean values for equality."""
-        return self.val == other.val
+        return self.val == other.val if isinstance(other, BoolVal) else False
 
     def __hash__(self):
         """Compute a hash code for a boolean value."""
@@ -479,7 +479,7 @@ class IntVal(Expr):
 
     def __eq__(self, other):
         """Compare two integer values for equality."""
-        return self.val == other.val
+        return self.val == other.val if isinstance(other, IntVal) else False
 
     def __hash__(self):
         """Compute a hash code for a integer value."""
@@ -534,9 +534,9 @@ class StrVal(Expr):
         self.val = val
         self.matcher = re.compile('%{(.*)}%')
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two string values for equality."""
-        return other is not None and self.val == other.val
+        return self.val == other.val if isinstance(other, StrVal) else False
 
     def __hash__(self):
         """Compute a hash code for a string value."""
@@ -608,9 +608,9 @@ class ListVal(Expr):
         """Initialize a list value."""
         self.val = val
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two list values for equality."""
-        return self.val == other.val
+        return self.val == other.val if isinstance(other, ListVal) else False
 
     def __str__(self):
         """Produce a string representation of a list value."""
@@ -673,9 +673,9 @@ class DictVal(Expr):
         """Initialize a dictionary value."""
         self.val = val
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two dictionary values for equality."""
-        return self.val == other.val
+        return self.val == other.val if isinstance(other, DictVal) else False
 
     def __str__(self):
         """Produce a string representation of a dictionary value."""
@@ -703,9 +703,9 @@ class Identifier(Expr):
         """Initialize an identifier."""
         self.name = name
 
-    def __eq__(self, other):
+    def __eq__(self, other) -> bool:
         """Compare two opam identifiers for equality."""
-        return self.name == other.name
+        return self.name == other.name if isinstance(other, Identifier) else False
 
     def __hash__(self):
         """Compute a hash code for an opam identifier."""
@@ -741,9 +741,10 @@ class NotOp(Expr):
         """Initialize a NOT operator."""
         self.term = term
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two NOT operators for equality."""
-        return self.term == other.term
+        return self.term == other.term if isinstance(other, NotOp) \
+            else False
 
     def __hash__(self):
         """Compute a hash code for a NOT operator."""
@@ -776,7 +777,8 @@ class DefinedOp(Expr):
 
     def __eq__(self, other):
         """Compare two defined operators for equality."""
-        return self.term == other.term
+        return self.term == other.term if isinstance(other, DefinedOp) \
+            else False
 
     def __hash__(self):
         """Compute a hash code for a defined operator."""
@@ -808,9 +810,10 @@ class PrefixRelOp(Expr):
         self.oper = oper
         self.term = term
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two prefix relational operators for equality."""
-        return self.oper == other.oper and self.term == other.term
+        return self.oper == other.oper and self.term == other.term if \
+            isinstance(other, PrefixRelOp) else False
 
     def __hash__(self):
         """Compute a hash code for a prefix relational operator."""
@@ -865,9 +868,11 @@ class RelOp(Expr):
 
     def __eq__(self, other):
         """Compare two relational operators for equality."""
-        return self.oper == other.oper \
-            and self.left == other.left \
+        return (
+            self.oper == other.oper
+            and self.left == other.left
             and self.right == other.right
+        ) if isinstance(other, RelOp) else False
 
     def __hash__(self):
         """Compute a hash code for a relational operator."""
@@ -925,9 +930,10 @@ class AndOp(Expr):
         self.left = left
         self.right = right
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two AND operators for equality."""
-        return self.left == other.left and self.right == other.right
+        return self.left == other.left and self.right == other.right if \
+            isinstance(other, AndOp) else False
 
     def __hash__(self):
         """Compute a hash code for an AND operator."""
@@ -987,9 +993,10 @@ class OrOp(Expr):
         self.left = left
         self.right = right
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two OR operators for equality."""
-        return self.left == other.left and self.right == other.right
+        return (self.left == other.left and self.right == other.right) \
+            if isinstance(other, OrOp) else False
 
     def __hash__(self):
         """Compute a hash code for an OR operator."""
@@ -1050,11 +1057,13 @@ class EnvOp(Expr):
         self.var = var
         self.value = value
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two environment update operators for equality."""
-        return self.oper == other.oper \
-            and self.var == other.var \
+        return (
+            self.oper == other.oper
+            and self.var == other.var
             and self.value == other.value
+        ) if isinstance(other, EnvOp) else False
 
     def __hash__(self):
         """Compute a hash code for an OR operator."""
@@ -1084,9 +1093,10 @@ class Option(Expr):
         self.name = name
         self.body = body
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two opam options for equality."""
-        return self.name == other.name and self.body == other.body
+        return self.name == other.name and self.body == other.body if \
+            isinstance(other, Option) else False
 
     def __hash__(self):
         """Compute a hash code for an opam option."""
@@ -1140,9 +1150,10 @@ class Section(Expr):
         self.name = name
         self.body = body
 
-    def __eq__(self, other):
+    def __eq__(self, other: object) -> bool:
         """Compare two opam file sections for equality."""
-        return self.name == other.name and self.body == other.body
+        return (self.name == other.name and self.body == other.body) \
+            if isinstance(other, Section) else False
 
     def __hash__(self):
         """Compute a hash code for an opam file section."""

From ff19971892fc94dd8f0bb1364c5bb29a277e60e9 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:28:44 +0000
Subject: [PATCH 4/7] Pass the full token into the PrefixRelOp constructor


If PrefixRelOp does not know the full token, then it cannot properly implement
negate()

---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index daf5db1..279f960 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -1248,7 +1248,7 @@ class OpamParser:
         elif tok.tok_type == TokenType.DEFINED:
             value = DefinedOp(self.parse_option_value(toks))
         elif is_rel_op(tok.tok_type):
-            value = PrefixRelOp(tok.value, self.parse_option_value(toks))
+            value = PrefixRelOp(tok, self.parse_option_value(toks))
         elif is_atom(tok):
             if tok.tok_type == TokenType.BOOL:
                 tok = get_bool(tok.value)

From 2bbc19ed364906e1671ba54a74cc210d4f38d187 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:30:20 +0000
Subject: [PATCH 5/7] Fix PrefixRelOp for receiving the token as oper to __init__


---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index 279f960..58b09f8 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -805,9 +805,9 @@ class PrefixRelOp(Expr):
 
     __slots__ = ['oper', 'term']
 
-    def __init__(self, oper, term):
+    def __init__(self, operator: Token, term: Expr) -> None:
         """Initialize a prefix relational operator."""
-        self.oper = oper
+        self.oper = operator
         self.term = term
 
     def __eq__(self, other: object) -> bool:
@@ -821,34 +821,20 @@ class PrefixRelOp(Expr):
 
     def __str__(self):
         """Produce a string representation of a prefix relational operator."""
-        return self.oper + ' ' + str(self.term)
+        return self.oper.value + ' ' + str(self.term)
 
     def __repr__(self):
         """Produce a string representation of a prefix relational operator."""
-        return 'PrefixRelOp(' + self.oper + ', ' + repr(self.term) + ')'
+        return 'PrefixRelOp(' + self.oper.value + ', ' + repr(self.term) + ')'
 
-    def verdependency(self, name):
+    def verdependency(self, name: str) -> str:
         """Convert a prefix relational operator to an RPM dependency."""
-        return name + ' ' + self.oper + ' ' \
+        return name + ' ' + self.oper.value + ' ' \
             + version.Version(str(self.term)).to_fedora()[0]
 
-    def negate(self):
+    def negate(self) -> PrefixRelOp:
         """Negate a prefix relational operator."""
-        if self.oper.tok_type == TokenType.EQ:
-            neg = PrefixRelOp(TokenType.NEQ, self.term)
-        elif self.oper.tok_type == TokenType.NEQ:
-            neg = PrefixRelOp(TokenType.EQ, self.term)
-        elif self.oper.tok_type == TokenType.GEQ:
-            neg = PrefixRelOp(TokenType.LT, self.term)
-        elif self.oper.tok_type == TokenType.GT:
-            neg = PrefixRelOp(TokenType.LEQ, self.term)
-        elif self.oper.tok_type == TokenType.LEQ:
-            neg = PrefixRelOp(TokenType.GT, self.term)
-        elif self.oper.tok_type == TokenType.LT:
-            neg = PrefixRelOp(TokenType.GEQ, self.term)
-        else:
-            neg = self
-        return neg
+        return PrefixRelOp(self.oper.negate(), self.term)
 
     def simplify(self, values, internal, seen):
         """Simplify a prefix relational operator."""

From c03663f65a0c3add6ff605f9bc83043fd9700f87 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:37:29 +0000
Subject: [PATCH 6/7] PrefixRelOp.simplify(): handle term.simplify() returning None


---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index 58b09f8..5c4f13b 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -836,10 +836,12 @@ class PrefixRelOp(Expr):
         """Negate a prefix relational operator."""
         return PrefixRelOp(self.oper.negate(), self.term)
 
-    def simplify(self, values, internal, seen):
+    def simplify(self, values, internal, seen) -> Optional[PrefixRelOp]:
         """Simplify a prefix relational operator."""
-        return PrefixRelOp(self.oper,
-                           self.term.simplify(values, internal, seen))
+        simplified_term = self.term.simplify(values, internal, seen)
+        return PrefixRelOp(self.oper, simplified_term) \
+            if simplified_term is not None else None
+
 
 class RelOp(Expr):
     """Class representing a relational operator."""

From b09389c2e6d10f2a743b2a938752d10d009aec83 Mon Sep 17 00:00:00 2001
From: Dan Čermák <dan.cermak@cgc-instruments.com>
Date: Dec 20 2020 21:45:13 +0000
Subject: [PATCH 7/7] Allow ListVal to behave like a list


It is accessed like a list in line 1322 in opamparser.py by being querried for
its length and getting its first element. This fails without implementing
__len__, __iter__ and __getitem__.

---

diff --git a/opam2rpm/opamparser.py b/opam2rpm/opamparser.py
index 5c4f13b..a4588f4 100644
--- a/opam2rpm/opamparser.py
+++ b/opam2rpm/opamparser.py
@@ -608,6 +608,16 @@ class ListVal(Expr):
         """Initialize a list value."""
         self.val = val
 
+    def __len__(self) -> int:
+        return len(self.val)
+
+    def __iter__(self) -> Iterator[Expr]:
+        for elem in self.val:
+            yield elem
+
+    def __getitem__(self, index: int) -> Expr:
+        return self.val[index]
+
     def __eq__(self, other: object) -> bool:
         """Compare two list values for equality."""
         return self.val == other.val if isinstance(other, ListVal) else False

