refactor(ext.commands): completely rewrite Range and String, require type argument (#991)

Co-authored-by: arl <me@arielle.codes>

Author: shiftinv

changelog/991.bugfix.rst

@@ -0,0 +1 @@
+|commands| Fix type-checker support for :class:`~disnake.ext.commands.Range` and :class:`~disnake.ext.commands.String` by requiring type argument (i.e. ``Range[int, 1, 5]`` instead of ``Range[1, 5]``).

changelog/991.deprecate.0.rst

@@ -0,0 +1 @@
+|commands| :class:`~disnake.ext.commands.Range` and :class:`~disnake.ext.commands.String` now require a type argument (i.e. ``Range[int, 1, 5]`` instead of ``Range[1, 5]``, similarly with ``String[str, 2, 4]``). The old form is deprecated.

changelog/991.deprecate.1.rst

@@ -0,0 +1 @@
+|commands| The mypy plugin is now a no-op. It was previously used for supporting ``Range[]`` and ``String[]`` annotations.

disnake/ext/commands/params.py

@@ -10,6 +10,8 @@
 import itertools
 import math
 import sys
+from abc import ABC, abstractmethod
+from dataclasses import dataclass
 from enum import Enum, EnumMeta
 from typing import (
     TYPE_CHECKING,
@@ -22,6 +24,7 @@
     Generic,
     List,
     Literal,
+    NoReturn,
     Optional,
     Sequence,
     Tuple,
@@ -31,7 +34,6 @@
     get_args,
     get_origin,
     get_type_hints,
-    overload,
 )
 
 import disnake
@@ -279,135 +281,163 @@ def decorator(func: CallableT) -> CallableT:
         return decorator
 
 
-class RangeMeta(type):
-    """Custom Generic implementation for Range"""
+@dataclass(frozen=True)
+class _BaseRange(ABC):
+    """Internal base type for supporting ``Range[...]`` and ``String[...]``."""
 
-    @overload
-    def __getitem__(
-        self, args: Tuple[Union[int, EllipsisType], Union[int, EllipsisType]]
-    ) -> Type[int]:
-        ...
+    _allowed_types: ClassVar[Tuple[Type[Any], ...]]
 
-    @overload
-    def __getitem__(
-        self, args: Tuple[Union[float, EllipsisType], Union[float, EllipsisType]]
-    ) -> Type[float]:
-        ...
+    underlying_type: Type[Any]
+    min_value: Optional[Union[int, float]]
+    max_value: Optional[Union[int, float]]
 
-    def __getitem__(self, args: Tuple[Any, ...]) -> Any:
-        a, b = [None if isinstance(x, type(Ellipsis)) else x for x in args]
-        return Range.create(min_value=a, max_value=b)
+    def __class_getitem__(cls, params: Tuple[Any, ...]) -> Self:
+        # deconstruct type arguments
+        if not isinstance(params, tuple):
+            params = (params,)
 
+        name = cls.__name__
 
-class Range(type, metaclass=RangeMeta):
-    """Type depicting a limited range of allowed values.
+        if len(params) == 2:
+            # backwards compatibility for `Range[1, 2]`
 
-    See :ref:`param_ranges` for more information.
+            # FIXME: the warning context is incorrect when used with stringified annotations,
+            # and points to the eval frame instead of user code
+            disnake.utils.warn_deprecated(
+                f"Using `{name}` without an explicit type argument is deprecated, "
+                "as this form does not work well with modern type-checkers. "
+                f"Use `{name}[<type>, <min>, <max>]` instead.",
+                stacklevel=2,
+            )
 
-    .. versionadded:: 2.4
+            # infer type from min/max values
+            params = (cls._infer_type(params),) + params
 
-    """
+        if len(params) != 3:
+            raise TypeError(
+                f"`{name}` expects 3 type arguments ({name}[<type>, <min>, <max>]), got {len(params)}"
+            )
 
-    min_value: Optional[float]
-    max_value: Optional[float]
+        underlying_type, min_value, max_value = params
 
-    @overload
-    @classmethod
-    def create(
-        cls,
-        min_value: Optional[int] = None,
-        max_value: Optional[int] = None,
-        *,
-        le: Optional[int] = None,
-        lt: Optional[int] = None,
-        ge: Optional[int] = None,
-        gt: Optional[int] = None,
-    ) -> Type[int]:
-        ...
-
-    @overload
-    @classmethod
-    def create(
-        cls,
-        min_value: Optional[float] = None,
-        max_value: Optional[float] = None,
-        *,
-        le: Optional[float] = None,
-        lt: Optional[float] = None,
-        ge: Optional[float] = None,
-        gt: Optional[float] = None,
-    ) -> Type[float]:
-        ...
+        # validate type (argument 1)
+        if not isinstance(underlying_type, type):
+            raise TypeError(f"First `{name}` argument must be a type, not `{underlying_type!r}`")
 
-    @classmethod
-    def create(
-        cls,
-        min_value: Optional[float] = None,
-        max_value: Optional[float] = None,
-        *,
-        le: Optional[float] = None,
-        lt: Optional[float] = None,
-        ge: Optional[float] = None,
-        gt: Optional[float] = None,
-    ) -> Any:
-        """Construct a new range with any possible constraints"""
-        self = cls(cls.__name__, (), {})
-        self.min_value = min_value if min_value is not None else _xt_to_xe(le, lt, -1)
-        self.max_value = max_value if max_value is not None else _xt_to_xe(ge, gt, 1)
-        return self
+        if not issubclass(underlying_type, cls._allowed_types):
+            allowed = "/".join(t.__name__ for t in cls._allowed_types)
+            raise TypeError(f"First `{name}` argument must be {allowed}, not `{underlying_type!r}`")
 
-    @property
-    def underlying_type(self) -> Union[Type[int], Type[float]]:
-        if isinstance(self.min_value, float) or isinstance(self.max_value, float):
-            return float
+        # validate min/max (arguments 2/3)
+        min_value = cls._coerce_bound(min_value, "min")
+        max_value = cls._coerce_bound(max_value, "max")
+
+        if min_value is None and max_value is None:
+            raise ValueError(f"`{name}` bounds cannot both be empty")
 
-        return int
+        # n.b. this allows bounds to be equal, which doesn't really serve a purpose with numbers,
+        # but is still accepted by the api
+        if min_value is not None and max_value is not None and min_value > max_value:
+            raise ValueError(
+                f"`{name}` minimum ({min_value}) must be less than or equal to maximum ({max_value})"
+            )
+
+        return cls(underlying_type=underlying_type, min_value=min_value, max_value=max_value)
+
+    @staticmethod
+    def _coerce_bound(value: Any, name: str) -> Optional[Union[int, float]]:
+        if value is None or isinstance(value, EllipsisType):
+            return None
+        elif isinstance(value, (int, float)):
+            if not math.isfinite(value):
+                raise ValueError(f"{name} value may not be NaN, inf, or -inf")
+            return value
+        else:
+            raise TypeError(f"{name} value must be int, float, None, or `...`, not `{type(value)}`")
 
     def __repr__(self) -> str:
         a = "..." if self.min_value is None else self.min_value
         b = "..." if self.max_value is None else self.max_value
-        return f"{type(self).__name__}[{a}, {b}]"
+        return f"{type(self).__name__}[{self.underlying_type.__name__}, {a}, {b}]"
 
+    @classmethod
+    @abstractmethod
+    def _infer_type(cls, params: Tuple[Any, ...]) -> Type[Any]:
+        raise NotImplementedError
 
-class StringMeta(type):
-    """Custom Generic implementation for String."""
+    # hack to get `typing._type_check` to pass, e.g. when using `Range` as a generic parameter
+    def __call__(self) -> NoReturn:
+        raise NotImplementedError
 
-    def __getitem__(
-        self, args: Tuple[Union[int, EllipsisType], Union[int, EllipsisType]]
-    ) -> Type[str]:
-        a, b = [None if isinstance(x, EllipsisType) else x for x in args]
-        return String.create(min_length=a, max_length=b)
+    # support new union syntax for `Range[int, 1, 2] | None`
+    if sys.version_info >= (3, 10):
 
+        def __or__(self, other):
+            return Union[self, other]  # type: ignore
 
-class String(type, metaclass=StringMeta):
-    """Type depicting a string option with limited length.
 
-    See :ref:`string_lengths` for more information.
+if TYPE_CHECKING:
+    # aliased import since mypy doesn't understand `Range = Annotated`
+    from typing_extensions import Annotated as Range, Annotated as String
+else:
 
-    .. versionadded:: 2.6
+    @dataclass(frozen=True, repr=False)
+    class Range(_BaseRange):
+        """Type representing a number with a limited range of allowed values.
 
-    """
+        See :ref:`param_ranges` for more information.
 
-    min_length: Optional[int]
-    max_length: Optional[int]
-    underlying_type: Final[Type[str]] = str
+        .. versionadded:: 2.4
 
-    @classmethod
-    def create(
-        cls,
-        min_length: Optional[int] = None,
-        max_length: Optional[int] = None,
-    ) -> Any:
-        """Construct a new String with constraints."""
-        self = cls(cls.__name__, (), {})
-        self.min_length = min_length
-        self.max_length = max_length
-        return self
+        .. versionchanged:: 2.9
+            Syntax changed from ``Range[5, 10]`` to ``Range[int, 5, 10]``;
+            the type (:class:`int` or :class:`float`) must now be specified explicitly.
+        """
 
-    def __repr__(self) -> str:
-        a = "..." if self.min_length is None else self.min_length
-        b = "..." if self.max_length is None else self.max_length
-        return f"{type(self).__name__}[{a}, {b}]"
+        _allowed_types = (int, float)
+
+        def __post_init__(self):
+            for value in (self.min_value, self.max_value):
+                if value is None:
+                    continue
+
+                if self.underlying_type is int and not isinstance(value, int):
+                    raise TypeError("Range[int, ...] bounds must be int, not float")
+
+        @classmethod
+        def _infer_type(cls, params: Tuple[Any, ...]) -> Type[Any]:
+            if any(isinstance(p, float) for p in params):
+                return float
+            return int
+
+    @dataclass(frozen=True, repr=False)
+    class String(_BaseRange):
+        """Type representing a string option with a limited length.
+
+        See :ref:`string_lengths` for more information.
+
+        .. versionadded:: 2.6
+
+        .. versionchanged:: 2.9
+            Syntax changed from ``String[5, 10]`` to ``String[str, 5, 10]``;
+            the type (:class:`str`) must now be specified explicitly.
+        """
+
+        _allowed_types = (str,)
+
+        def __post_init__(self):
+            for value in (self.min_value, self.max_value):
+                if value is None:
+                    continue
+
+                if not isinstance(value, int):
+                    raise TypeError("String bounds must be int, not float")
+                if value < 0:
+                    raise ValueError("String bounds may not be negative")
+
+        @classmethod
+        def _infer_type(cls, params: Tuple[Any, ...]) -> Type[Any]:
+            return str
 
 
 class LargeInt(int):
@@ -701,14 +731,14 @@ def parse_annotation(self, annotation: Any, converter_mode: bool = False) -> boo
         if annotation is inspect.Parameter.empty or annotation is Any:
             return False
 
-        # resolve type aliases
+        # resolve type aliases and special types
         if isinstance(annotation, Range):
             self.min_value = annotation.min_value
             self.max_value = annotation.max_value
             annotation = annotation.underlying_type
         if isinstance(annotation, String):
-            self.min_length = annotation.min_length
-            self.max_length = annotation.max_length
+            self.min_length = annotation.min_value
+            self.max_length = annotation.max_value
             annotation = annotation.underlying_type
         if issubclass_(annotation, LargeInt):
             self.large = True

disnake/ext/mypy_plugin/__init__.py

@@ -2,67 +2,12 @@
 
 import typing as t
 
-from mypy.plugin import AnalyzeTypeContext, Plugin
-from mypy.types import AnyType, EllipsisType, RawExpressionType, Type, TypeOfAny
+from mypy.plugin import Plugin
 
 
+# FIXME: properly deprecate this in the future
 class DisnakePlugin(Plugin):
-    def get_type_analyze_hook(
-        self, fullname: str
-    ) -> t.Optional[t.Callable[[AnalyzeTypeContext], Type]]:
-        if fullname == "disnake.ext.commands.params.Range":
-            return range_type_analyze_callback
-        if fullname == "disnake.ext.commands.params.String":
-            return string_type_analyze_callback
-        return None
-
-
-def range_type_analyze_callback(ctx: AnalyzeTypeContext) -> Type:
-    args = ctx.type.args
-
-    if len(args) != 2:
-        ctx.api.fail(f'"Range" expected 2 parameters, got {len(args)}', ctx.context)
-        return AnyType(TypeOfAny.from_error)
-
-    for arg in args:
-        if isinstance(arg, EllipsisType):
-            continue
-        if not isinstance(arg, RawExpressionType):
-            ctx.api.fail('invalid usage of "Range"', ctx.context)
-            return AnyType(TypeOfAny.from_error)
-
-        name = arg.simple_name()
-        # if one is a float, `Range.underlying_type` returns `float`
-        if name == "float":
-            return ctx.api.named_type("builtins.float", [])
-        # otherwise it should be an int; fail if it isn't
-        elif name != "int":
-            ctx.api.fail(f'"Range" parameters must be int or float, not {name}', ctx.context)
-            return AnyType(TypeOfAny.from_error)
-
-    return ctx.api.named_type("builtins.int", [])
-
-
-def string_type_analyze_callback(ctx: AnalyzeTypeContext) -> Type:
-    args = ctx.type.args
-
-    if len(args) != 2:
-        ctx.api.fail(f'"String" expected 2 parameters, got {len(args)}', ctx.context)
-        return AnyType(TypeOfAny.from_error)
-
-    for arg in args:
-        if isinstance(arg, EllipsisType):
-            continue
-        if not isinstance(arg, RawExpressionType):
-            ctx.api.fail('invalid usage of "String"', ctx.context)
-            return AnyType(TypeOfAny.from_error)
-
-        name = arg.simple_name()
-        if name != "int":
-            ctx.api.fail(f'"String" parameters must be int, not {name}', ctx.context)
-            return AnyType(TypeOfAny.from_error)
-
-    return ctx.api.named_type("builtins.str", [])
+    """Custom mypy plugin; no-op as of version 2.9."""
 
 
 def plugin(version: str) -> t.Type[Plugin]: