🐛 fix: stop members with equal values collapsing into aliases

README.md

@@ -57,6 +57,26 @@ then
 then [`packaging.version.parse`][Version-link]. If the package cannot be found
 then it is considered `InstalledState.NOT_INSTALLED`
 
+Every member stays its own member, whatever its value. This matters because
+those values collide readily: any two dependencies that are both missing share
+the `NOT_INSTALLED` sentinel, and distributions released together share a
+version number. A plain [`enum.Enum`][Enum-link] folds equal-valued members into
+aliases of whichever was declared first, which would make
+`OptDeps.SECOND.installed` report the _first_ package's state.
+
+```python
+class OptDeps(OptionalDependencyEnum):
+    NOT_A_PACKAGE = auto()
+    ALSO_NOT_A_PACKAGE = auto()
+
+
+OptDeps.ALSO_NOT_A_PACKAGE.name
+# 'ALSO_NOT_A_PACKAGE'
+
+OptDeps.NOT_A_PACKAGE is OptDeps.ALSO_NOT_A_PACKAGE
+# False
+```
+
 `InstalledState.NOT_INSTALLED` is an [`enum.Enum`][Enum-link] member that has a
 truthy value of `False`. This can be useful for boolean checks, as
 [`packaging.Version`][Version-link] always has a truthy value of `True`.

src/optional_dependencies/_core.py

@@ -6,14 +6,18 @@
 from collections.abc import Callable
 from dataclasses import dataclass
 from enum import Enum
-from types import MethodType
-from typing import Literal, cast
+from types import DynamicClassAttribute, MethodType
+from typing import Literal, TypeVar, cast, final
 
 from packaging.utils import canonicalize_name
 from packaging.version import Version
 
 from .utils import InstalledState, get_version
 
+#: Stands in for `typing.Self`, which is 3.11+; this package supports 3.10, and
+#: `typing_extensions` is not a dependency.
+_EnumT = TypeVar("_EnumT", bound="OptionalDependencyEnum")
+
 
 @dataclass(frozen=True)
 class Comparator:
@@ -73,9 +77,98 @@ def __call__(self, enum: "OptionalDependencyEnum", other: object) -> bool:
 # ===================================================================
 
 
+@final
+# `eq=False` is load-bearing, not tidiness: the generated `__eq__` compares by
+# field, so two keys wrapping equal versions would compare equal and `enum`
+# would alias their members straight back together. `repr=False` leaves the
+# delegating `__repr__` below in place.
+@dataclass(frozen=True, slots=True, eq=False, repr=False)
+class _MemberKey:
+    """The internal ``_value_`` of an `OptionalDependencyEnum` member.
+
+    `enum.Enum` folds any member whose ``_value_`` compares equal to an earlier
+    one into an alias of that member -- silently, keeping the first name. A
+    version cannot serve as that key: every uninstalled dependency carries the
+    same `InstalledState.NOT_INSTALLED` sentinel, and distributions released
+    together share a version number, so a second uninstalled member, or a second
+    member of a co-released family, used to vanish into the first and report the
+    wrong package's state.
+
+    Wrapping the resolved version in one of these gives every member a key that
+    is distinct by identity, so no two of them can ever compare equal. The
+    wrapper is internal: `OptionalDependencyEnum.value` unwraps it, and `__repr__`
+    delegates so members still show as ``<OptDeps.PACKAGING: <Version('...')>>``.
+    """
+
+    resolved: Version | Literal[InstalledState.NOT_INSTALLED]
+
+    def __repr__(self) -> str:
+        return repr(self.resolved)
+
+
 class OptionalDependencyEnum(Enum):
     """An enumeration of optional dependencies."""
 
+    _value_: _MemberKey
+
+    # PYI019 wants `typing.Self` here; see `_EnumT` for why it cannot be used.
+    def __new__(  # noqa: PYI019
+        cls: type[_EnumT], value: Version | Literal[InstalledState.NOT_INSTALLED]
+    ) -> _EnumT:
+        """Give the member an alias-proof ``_value_``.
+
+        See `_MemberKey` for why the resolved version cannot be that key.
+
+        This has to happen in ``__new__`` rather than ``__init__``: from Python
+        3.11 on, `enum` snapshots ``_value_`` for the duplicate scan *before*
+        calling ``__init__``, so a reassignment there comes too late. Every
+        supported version (3.10+) honours a ``_value_`` set in ``__new__``.
+        """
+        obj = object.__new__(cls)
+        obj._value_ = _MemberKey(value)
+        return obj
+
+    @classmethod
+    def _missing_(cls, value: object) -> "OptionalDependencyEnum | None":
+        """Look a member up by its version, as ``Enum(value)`` used to.
+
+        Members are keyed internally on `_MemberKey`, so the by-value map no
+        longer holds bare versions. This restores the lookup, with the ambiguity
+        it always had: where several members share a version, the first one
+        declared wins.
+        """
+        return next((m for m in cls if m.value == value), None)
+
+    def __reduce_ex__(self, proto: object) -> tuple[object, ...]:
+        """Pickle by name.
+
+        `enum.Enum` pickles by ``_value_``, which is now an identity-keyed
+        wrapper that would not survive the round trip. The name is the exact
+        key besides: versions do not distinguish members, which is the whole
+        problem being fixed here.
+        """
+        return getattr, (self.__class__, self.name)
+
+    @DynamicClassAttribute
+    def value(self) -> Version | Literal[InstalledState.NOT_INSTALLED]:
+        """The version of the optional dependency, or `NOT_INSTALLED`.
+
+        Examples
+        --------
+        >>> from enum import auto
+        >>> class OptDeps(OptionalDependencyEnum):
+        ...     PACKAGING = auto()
+        ...     NOTINSTALLED = auto()
+
+        >>> OptDeps.PACKAGING.value
+        <Version('...')>
+
+        >>> OptDeps.NOTINSTALLED.value
+        <InstalledState.NOT_INSTALLED: False>
+
+        """
+        return self._value_.resolved
+
     @staticmethod
     def _generate_next_value_(
         name: str,

tests/test_no_aliasing.py

@@ -0,0 +1,129 @@
+"""Every member stays its own member, whatever its value.
+
+`enum.Enum` folds members whose values compare equal into a single member,
+keeping the first name and aliasing the rest. The values here are versions, so
+collisions are the norm rather than the exception: every uninstalled dependency
+carries the same `NOT_INSTALLED` sentinel, and co-released distributions share a
+version number.
+"""
+
+import pickle
+
+import pytest
+from packaging.version import Version
+
+from optional_dependencies import OptionalDependencyEnum, auto
+from optional_dependencies.utils import NOT_INSTALLED, get_version, is_installed
+
+
+class OptDeps(OptionalDependencyEnum):
+    PACKAGING = auto()  # runtime dependency
+    PYTEST = auto()  # test dependency
+    NOTINSTALLED = auto()  # not installed
+    ALSONOTINSTALLED = auto()  # not installed either
+    # Low-level API: an explicitly assigned value, colliding with PACKAGING.
+    PACKAGING_AGAIN = get_version("packaging")
+
+
+def test_all_members_are_distinct() -> None:
+    """Iteration yields every member, none folded into an alias."""
+    assert [m.name for m in OptDeps] == list(OptDeps.__members__)
+    assert len(set(OptDeps)) == 5
+
+
+@pytest.mark.parametrize("name", list(OptDeps.__members__))
+def test_member_keeps_its_own_name(name: str) -> None:
+    """Attribute lookup returns the member it was asked for."""
+    assert OptDeps[name].name == name
+    assert getattr(OptDeps, name).name == name
+
+
+def test_two_uninstalled_members_do_not_collapse() -> None:
+    """Both share the `NOT_INSTALLED` sentinel; neither becomes the other."""
+    assert OptDeps.NOTINSTALLED is not OptDeps.ALSONOTINSTALLED
+    assert not OptDeps.NOTINSTALLED.installed
+    assert not OptDeps.ALSONOTINSTALLED.installed
+
+
+def test_two_members_sharing_a_version_do_not_collapse() -> None:
+    """An explicit value equal to another member's is still its own member."""
+    assert OptDeps.PACKAGING is not OptDeps.PACKAGING_AGAIN
+    assert OptDeps.PACKAGING.version == OptDeps.PACKAGING_AGAIN.version
+
+
+def test_value_is_still_the_version() -> None:
+    """`value` keeps its documented meaning: a `Version` or `NOT_INSTALLED`."""
+    assert isinstance(OptDeps.PACKAGING.value, Version)
+    assert OptDeps.PACKAGING.value == OptDeps.PACKAGING.version
+    assert OptDeps.NOTINSTALLED.value is NOT_INSTALLED
+
+
+@pytest.mark.parametrize("name", ["PACKAGING", "NOTINSTALLED"])
+def test_repr_delegates_to_the_version(name: str) -> None:
+    """The key wrapper is invisible: a member still renders as its version."""
+    member = OptDeps[name]
+    assert repr(member.value) in repr(member)
+    assert "_MemberKey" not in repr(member)
+
+
+def test_members_are_hashable_and_usable_as_keys() -> None:
+    """Distinct members occupy distinct slots in a dict/set."""
+    mapping = {m: m.name for m in OptDeps}
+    assert len(mapping) == 5
+    assert mapping[OptDeps.ALSONOTINSTALLED] == "ALSONOTINSTALLED"
+
+
+@pytest.mark.parametrize("name", list(OptDeps.__members__))
+def test_roundtrips_through_pickle(name: str) -> None:
+    """Pickling resolves back to the same member, not to an alias."""
+    member = OptDeps[name]
+    assert pickle.loads(pickle.dumps(member)) is member  # noqa: S301
+
+
+def test_lookup_by_value_still_works() -> None:
+    """`Enum(value)` keeps resolving through the version, as before.
+
+    It cannot distinguish members that share one -- that ambiguity is inherent
+    to looking a member up by a non-unique key -- but it must still find *a*
+    member rather than raise.
+    """
+    assert OptDeps(get_version("packaging")) in {
+        OptDeps.PACKAGING,
+        OptDeps.PACKAGING_AGAIN,
+    }
+    assert OptDeps(NOT_INSTALLED) in {OptDeps.NOTINSTALLED, OptDeps.ALSONOTINSTALLED}
+
+
+def test_lookup_by_unknown_value_still_raises() -> None:
+    """A version no member carries is still a `ValueError`."""
+    with pytest.raises(ValueError, match="is not a valid OptDeps"):
+        OptDeps(Version("0.0.0.dev0"))
+
+
+def test_lookup_by_name_still_works() -> None:
+    """`Enum[name]` is exact, and now the only exact lookup."""
+    assert OptDeps["PACKAGING_AGAIN"] is OptDeps.PACKAGING_AGAIN
+    assert OptDeps["ALSONOTINSTALLED"] is OptDeps.ALSONOTINSTALLED
+
+
+def test_class_level_value_access_is_rejected() -> None:
+    """`value` stays a per-member attribute, as on a plain `Enum`."""
+    with pytest.raises(AttributeError):
+        _ = OptDeps.value  # type: ignore[attr-defined]
+
+
+def test_low_level_api_members_are_distinct() -> None:
+    """`chain_checks`-style values do not collapse either."""
+
+    class Chained(OptionalDependencyEnum):
+        A = get_version("packaging")
+        B = get_version("packaging")
+        C = get_version("this-is-not-a-package")
+        D = get_version("nor-is-this")
+
+    assert len({Chained.A, Chained.B, Chained.C, Chained.D}) == 4
+    assert Chained.A.installed
+    assert Chained.B.installed
+    assert not Chained.C.installed
+    assert not Chained.D.installed
+    assert is_installed("packaging")