Enable reportInconsistentOverload
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?
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?
It is a 3.13 addition. Unfortunately
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?
Works for me on the latest stable version of pylance:
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
Sounds good to me, then ^^
The current behavior does default the TypeVar with an initialized class. But it does not provide that info for the type:
I was only checking it with type usage.
Aha my bad then, it's been a while since I hacked that together, haha