refactor: make ApplicationContext.user non-optional

[Dorukyum]

Summary

At least one of the member and user fields in the interaction payload will be present unless it's a PING interaction. This change prevents type issues from occuring almost everywhere Interaction.user or ApplicationContext.user is used without a type check.

This will be a breaking change in case this attribute is accessed in a PING handler (such a case isn't in the lib) even though it would always be None.

Information

• 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, typehinting, examples, ...).

Checklist

• I have searched the open pull requests for duplicates.
• If code changes were made then they have been tested.
  • I have updated the documentation to reflect the changes.
• If type: ignore comments were used, a comment is also left explaining why.

[Lulalaby]

lgtm

[BobDotCom]

Alternatively I feel it would be better to keep Interaction.user optional (since it is), and instead make ApplicationContext.user non-optional with a type: ignore.

[Dorukyum]

Alternatively I feel it would be better to keep Interaction.user optional (since it is), and instead make ApplicationContext.user non-optional with a type: ignore.

Why keep Interaction.user the same? Not to remove the tons of type errors that occur in components or on_interaction? If we're changing one attribute, I think we should change the other too here.

[BobDotCom]

Alternatively I feel it would be better to keep Interaction.user optional (since it is), and instead make ApplicationContext.user non-optional with a type: ignore.

Why keep Interaction.user the same? Not to remove the tons of type errors that occur in components or on_interaction? If we're changing one attribute, I think we should change the other too here.

Because we should not make invalid type hints, which is what this would currently do. We can set it as optional and type: ignore when we know it will be present. Otherwise it defeats the purpose of having type hints, if they're wrong anyways.

[Dorukyum]

Alternatively I feel it would be better to keep Interaction.user optional (since it is), and instead make ApplicationContext.user non-optional with a type: ignore.

Why keep Interaction.user the same? Not to remove the tons of type errors that occur in components or on_interaction? If we're changing one attribute, I think we should change the other too here.

Because we should not make invalid type hints, which is what this would currently do. We can set it as optional and type: ignore when we know it will be present. Otherwise it defeats the purpose of having type hints, if they're wrong anyways.

It'll only be wrong internally when PONG-ing interactions, if done

[Lulalaby]

ping pong

[EmreTech]

Alternatively I feel it would be better to keep Interaction.user optional (since it is), and instead make ApplicationContext.user non-optional with a type: ignore.

I do not agree with this approach. It's a way better idea to just change the type hints. It'll be better in the long run.
It would also be a fantastic idea to split interactions into different classes like Disnake did, but that's for another PR.

[BobDotCom]

I do not agree with this approach. It's a way better idea to just change the type hints. It'll be better in the long run.

Why? In the solution I proposed, Interaction.user is type-hinted to Optional[User] which granted in most cases is a bit loose as it's typically not None. However, we cannot say that it is never None, because sometimes it is. That being said, we can say that it is never None for app command interactions, hence why ApplicationContext would not have an optional typehint.

It would also be a fantastic idea to split interactions into different classes like Disnake did, but that's for another PR.

That sounds like a potentially better approach. If we go this route, the best course of action may be closing this PR in favor of a new PR for the split-interaction approach.

[Dorukyum]

Splitting interaction classes is a major change and will most likely be breaking. It might just as well create more type issues like the ones this pr aims to solve.

[Dorukyum]

I've made changes after we decided to only make ApplicationContext.user non-optional and keep Interaction.user as-is.