Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correctly handle unions in verify_type #584

Merged
merged 8 commits into from
Jul 2, 2022

Conversation

shiftinv
Copy link
Member

Summary

Fixes #577.

An annotation like Union[User, Member] passes the issubclass_(self.type, disnake.Member) check since issubclass_ explicitly handles unions; the following isinstance(argument, disnake.Member) check did not accept User objects, even though they should've been permitted based on the annotation type.
This PR improves the validation logic to correctly handle cases like this as well.

Regression from #283 - third test case of test_verify_type checks for this particular one.

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running task lint
    • I have type-checked the code by running task pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@shiftinv shiftinv added bug Something isn't working p: medium Medium priority s: needs review Issue/PR is awaiting reviews backport needed: 2.5 labels Jun 26, 2022
@shiftinv shiftinv added this to the disnake v2.6 milestone Jun 26, 2022
disnake/ext/commands/params.py Outdated Show resolved Hide resolved
Copy link
Member

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still has a minor bug with users and members:
image

    @parent.sub_command("user")
    async def user(self, itr: disnake.CommandInteraction, user: disnake.User):
        await itr.send(type(user))

@shiftinv
Copy link
Member Author

shiftinv commented Jul 2, 2022

This still has a minor bug with users and members

it's not a bug, it's a feature
But yeah, that seems to be working as intended, even if Member doesn't inherit from User it ends up having the same attributes+methods due to @flatten_user

@onerandomusername
Copy link
Member

Sounds good, then.

@onerandomusername onerandomusername enabled auto-merge (squash) July 2, 2022 15:04
@onerandomusername onerandomusername merged commit 58583a0 into master Jul 2, 2022
@onerandomusername onerandomusername deleted the fix/member-option-verify branch July 2, 2022 15:07
onerandomusername added a commit to shiftinv/disnake that referenced this pull request Jul 17, 2022
@shiftinv shiftinv removed the s: needs review Issue/PR is awaiting reviews label Jul 27, 2022
@onerandomusername onerandomusername added s: in progress Issue/PR is being worked on s: waiting for api/docs Issue/PR is waiting for API support/documentation and removed s: in progress Issue/PR is being worked on s: waiting for api/docs Issue/PR is waiting for API support/documentation labels Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p: medium Medium priority
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Traceback when "Union[Member, User]" is used as type hint in slash command parameters
2 participants