DisnakeDev / disnake

An API wrapper for Discord written in Python.
https://docs.disnake.dev
MIT License
721 stars 137 forks source link

A defect in InteractionBotBase typings causes valid code to not type check properly. #1045

Closed elenakrittik closed 1 year ago

elenakrittik commented 1 year ago

Summary

A defect in InteractionBotBase typings causes valid application commands checks-related methods usage to make pyright sad.

Reproduction Steps

I'm using pdm here but this bug is not it's issue; choose whatever PM you prefer.

Reproduction steps:

pdm venv create # (optional) Create virtual environment
pdm init # Create pyproject.toml
pdm add disnake # Add disnake to dependencies
pdm add pyright --dev # Install pyright wrapper
# Edit `main.py` file as shown in "Minimal reproduction code" section
pdm run pyright main.py # Then run pyright on it
main.py
  main.py:10:5 - error: Argument of type "(inter: ApplicationCommandInteraction) -> Coroutine[Any, Any, bool]" cannot be assigned to parameter "func" of type "Check" in function "add_app_command_check"
    Type "(inter: ApplicationCommandInteraction) -> Coroutine[Any, Any, bool]" cannot be assigned to type "Check"
      Type "(inter: ApplicationCommandInteraction) -> Coroutine[Any, Any, bool]" cannot be assigned to type "(Cog, Context[Any]) -> MaybeCoro[bool]"
        Function accepts too many positional parameters; expected 1 but received 2
          Parameter 1: type "Cog" cannot be assigned to type "ApplicationCommandInteraction"
            "Cog" is incompatible with "ApplicationCommandInteraction"
      Type "(inter: ApplicationCommandInteraction) -> Coroutine[Any, Any, bool]" cannot be assigned to type "(Context[Any]) -> MaybeCoro[bool]"
        Parameter 1: type "Context[Any]" cannot be assigned to type "ApplicationCommandInteraction"
          "Context[Any]" is incompatible with "ApplicationCommandInteraction" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations

Minimal Reproducible Code

import disnake
from disnake.ext import commands

bot = commands.InteractionBot(intents=disnake.Intents.default())

async def my_check(inter: disnake.ApplicationCommandInteraction) -> bool:
    return 2 * 2 == 4

bot.add_app_command_check(
    my_check,               # <--- here
    slash_commands=True,
    user_commands=True,
    message_commands=True
)

Expected Results

I expected code to type check properly as the usage is perfectly correct.

Actual Results

Due to wrong typing inside disnake itself code cannot be type checked properly, though at runtime it works flawlessly.

Intents

Intents.default() but that doesn't matter in this case.

System Information

- Python v3.11.3-final
- disnake v2.8.1-final
    - disnake importlib.metadata: v2.8.1
- aiohttp v3.8.4
- system info: Linux 6.3.4-arch1-1 #1 SMP PREEMPT_DYNAMIC Wed, 24 May 2023 17:44:00 +0000 x86_64

Checklist

Additional Context

I am aware of #614, but there no any mentions about possible downsides of broken typings. The phrasing there is also a bit confusing: "are not entirely correct" does not quite represent application commands methods using types intended for prefix ones (Check, specifically). Decorator versions work fine because of # type: ignores placed there (as mentioned accordingly in #614), despite for another reason, and are currently the only type-"safe" workaround (a partial workaround, as you still would not be able to remove checks as there isn't a decorator for that).

elenakrittik commented 1 year ago

Adding the Callable[["CommandInteraction"], MaybeCoro[bool]] variant to the Check type solves the issue, though i'm not sure whether that's a correct solution.

shiftinv commented 1 year ago

Thanks for the bug report, appreciate all the details c: "are not entirely correct" in #614 refers to things like the @commands.check decorator, which can be used for both prefix commands and application commands, while only type-checking properly for the former. That being said, this is definitely still an issue.

Enegg commented 1 year ago

InteractionBotBase is indeed a defect 🙃

EQUENOS commented 1 year ago

InteractionBotBase is indeed a defect 🙃

It abstracts the entire app command framework into a mixin that can be used together with Client and/or BotBase. Although I do agree that it turned out clumsy.

Enegg commented 1 year ago

It could be a composite object. Instead of a bunch of subclasses for sharding and commands, a single Bot class could hold a (Sharded)Client, and a container for command managers.

elenakrittik commented 1 year ago

It could be a composite object. Instead of a bunch of subclasses for sharding and commands, a single Bot class could hold a (Sharded)Client, and a container for command managers.

This sounds like a separate issue about refactoring how all these Clients, Bots and *Bases are structured.

Enegg commented 1 year ago

Of course, it's not related to this issue.

I'll create an issue for it once I have a half decent idea for it.

EQUENOS commented 1 year ago

Of course, it's not related to this issue.

I'll create an issue for it once I have a half decent idea for it.

Such refactor won't happen before disnake 3.0, so there's no need for a gh issue. Instead we can include it in a list on the /projects tab