From d17d40293de158a73b5e9621e50e97519e80e582 Mon Sep 17 00:00:00 2001 From: Dan Čermák 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 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 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 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 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 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 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