disnake icon indicating copy to clipboard operation
disnake copied to clipboard

Enable reportInconsistentOverload

Open CoderJoshDK opened this issue 2 months ago • 8 comments

Summary

The pyright linting of reportInconsistentOverload should be enabled

What is the feature request for?

The core library

The Problem

The static check is currently disabled. To enable it, invalid overloading would need to be removed. I am unsure what it was thought to be doing? Overloading the self shouldn't have any effect? Maybe there is some case I am missing?

So far, all offenders of this rule that I have found, are on overloads that only change the type of self.

The Ideal Solution

diff --git a/disnake/ui/item.py b/disnake/ui/item.py
index f1db88d7..468a6cba 100644
--- a/disnake/ui/item.py
+++ b/disnake/ui/item.py
@@ -162,12 +162,6 @@ class Item(WrappedComponent, Generic[V_co]):
 
     __repr_attributes__: ClassVar[Tuple[str, ...]] = ("row",)
 
-    @overload
-    def __init__(self: Item[None]) -> None: ...
-
-    @overload
-    def __init__(self: Item[V_co]) -> None: ...
-
     def __init__(self) -> None:
         self._view: V_co = None  # type: ignore
         self._row: Optional[int] = None

The Current Solution

No response

Additional Context

It might also require going through each instance of these classes and adding in the specified generic type:

@@ -170,7 +170,7 @@ class View:
             await asyncio.sleep(self.__timeout_expiry - now)
 
     def to_components(self) -> List[ActionRowPayload]:
-        def key(item: Item) -> int:
+        def key(item: Item[View]) -> int:
             return item._rendered_row or 0

This should be done anyway, as to avoid unknown scope. But I don't think this step is needed for the removal. That can be its own thing.

So my question is A) what was the overloads trying to do? And B) am I good to remove them, to enable this static check?

CoderJoshDK avatar Oct 17 '25 12:10 CoderJoshDK

I am unsure what it was thought to be doing? Overloading the self shouldn't have any effect? Maybe there is some case I am missing?

This was originally done to set default typevar values before this was an official language feature. Not providing anything would match the first overload, i.e. Item[None] and thus set the value to None by default; this isn't relevant for Items themselves, but for subclasses like disnake.ui.Button this allows typehinting and instantiating buttons without having to explicitly set the typevar.

Nowadays, as mentioned, this is an actual language feature through Foo = TypeVar("Foo", default=...) (at least via typing_extensions, I'm not sure in what version support for this was added). If it's supported by the builtin typing module at disnake's minimum version, we can just use that; or otherwise through typing_extensions but I'm not exactly sure what our "rules" on the use of that package were. I believe making sure it's type-checking only is fine?

Sharp-Eyes avatar Oct 17 '25 13:10 Sharp-Eyes

It is a 3.13 addition. Unfortunately

CoderJoshDK avatar Oct 17 '25 13:10 CoderJoshDK

I might be missing something, but:

Not providing anything would match the first overload, i.e. Item[None] and thus set the value to None by default;

Doesn't do what you say? When I check the type, everything still shows up as Unknown (off of master). What static checkers support this behavior?

CoderJoshDK avatar Oct 17 '25 13:10 CoderJoshDK

Works for me on the latest stable version of pylance: Image

Sharp-Eyes avatar Oct 17 '25 18:10 Sharp-Eyes

I believe we started using typing_extensions at top level; we probably should update the signatures that use overloads to default to None for a generic to use the typing_extensions.TypeVar + default instead

Enegg avatar Oct 17 '25 19:10 Enegg

Sounds good to me, then ^^

Sharp-Eyes avatar Oct 17 '25 19:10 Sharp-Eyes

The current behavior does default the TypeVar with an initialized class. But it does not provide that info for the type:

Image

I was only checking it with type usage.

CoderJoshDK avatar Oct 17 '25 19:10 CoderJoshDK

Aha my bad then, it's been a while since I hacked that together, haha

Sharp-Eyes avatar Oct 17 '25 19:10 Sharp-Eyes