Skip to content

Commit 3ec48c6

Browse files
authored
fix: make SwitchInfo hashable again so requires=/excludes= work (#816) (#818)
Converting SwitchInfo from a namedtuple to a value-comparing dataclass set __hash__ to None, so any app using requires= or excludes= crashed with 'TypeError: unhashable type: SwitchInfo' while validating switches (they are collected into a set). SwitchInfo has mutable list fields and so cannot be value-hashed; it is used in those sets purely as a unique descriptor, so eq=False restores the correct identity-based __hash__ and __eq__. Adds regression tests for requires=/excludes= (which had no coverage).
1 parent 39a01e9 commit 3ec48c6

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

plumbum/cli/switches.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ class SubcommandError(SwitchError):
6767
# ===================================================================================================
6868
# The switch decorator
6969
# ===================================================================================================
70-
@dataclasses.dataclass
70+
# ``eq=False`` keeps the default identity-based ``__hash__`` (and ``__eq__``).
71+
# The dataclass has mutable ``list`` fields, so it cannot be value-hashed, yet
72+
# ``Application`` puts ``SwitchInfo`` objects into sets when validating
73+
# ``requires``/``excludes``. Identity semantics are correct here: each switch is
74+
# a unique descriptor. (A value-based ``__eq__`` sets ``__hash__ = None``, which
75+
# is what broke ``requires=``/``excludes=`` when this became a dataclass.)
76+
@dataclasses.dataclass(eq=False)
7177
class SwitchInfo:
7278
# Python 3.10+ can use slots=True
7379
__slots__ = (

tests/test_cli.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
from plumbum import cli, local
4+
from plumbum.cli.switches import SwitchInfo
45
from plumbum.cli.terminal import get_terminal_size
56

67

@@ -491,3 +492,65 @@ def main(self):
491492
)
492493
# No traceback should be in stderr
493494
assert "Traceback" not in result.stderr, f"Traceback in stderr: {result.stderr}"
495+
496+
497+
class ExcludesApp(cli.Application):
498+
alpha = cli.Flag("--alpha")
499+
beta = cli.Flag("--beta", excludes=["--alpha"])
500+
501+
def main(self):
502+
pass
503+
504+
505+
class RequiresApp(cli.Application):
506+
alpha = cli.Flag("--alpha")
507+
beta = cli.Flag("--beta", requires=["--alpha"])
508+
509+
def main(self):
510+
pass
511+
512+
513+
class TestSwitchCombinations:
514+
"""``requires=``/``excludes=`` switch combinations.
515+
516+
Regression test for #816: validating these put ``SwitchInfo`` objects into a
517+
set, which crashed with ``TypeError: unhashable type: 'SwitchInfo'`` after
518+
``SwitchInfo`` became a (value-comparing, therefore unhashable) dataclass.
519+
"""
520+
521+
def test_excludes_runs_when_not_conflicting(self):
522+
_, rc = ExcludesApp.run(["app", "--beta"], exit=False)
523+
assert rc == 0
524+
525+
def test_excludes_rejects_conflicting_switches(self, capsys):
526+
_, rc = ExcludesApp.run(["app", "--alpha", "--beta"], exit=False)
527+
assert rc == 2
528+
assert "invalid" in capsys.readouterr()[0]
529+
530+
def test_requires_runs_when_satisfied(self):
531+
_, rc = RequiresApp.run(["app", "--alpha", "--beta"], exit=False)
532+
assert rc == 0
533+
534+
def test_requires_rejects_when_dependency_missing(self, capsys):
535+
_, rc = RequiresApp.run(["app", "--beta"], exit=False)
536+
assert rc == 2
537+
assert "missing" in capsys.readouterr()[0]
538+
539+
def test_switchinfo_is_hashable(self):
540+
# The set membership above only works if SwitchInfo is hashable; the
541+
# list fields mean it can only be hashed by identity, not by value.
542+
info = SwitchInfo(
543+
names=["--x"],
544+
envname=None,
545+
argtype=None,
546+
list=False,
547+
func=lambda: None,
548+
mandatory=False,
549+
overridable=False,
550+
group="Switches",
551+
requires=[],
552+
excludes=[],
553+
argname="VALUE",
554+
help=None,
555+
)
556+
assert isinstance(hash(info), int)

0 commit comments

Comments
 (0)