feat(client)!: Make kwargs for 'public api' Client classes explicit, disallow superfluous kwargs

[Sharp-Eyes]

Summary

This PR aims to improve two things. Firstly, passing 'incorrect'/unused kwargs to Client or any of its subclasses will now error. For example,

bot = commands.Bot("!", sync_command_debug=True)

would run fine in the current version of disnake but do nothing, as the actual kwarg is sync_commands_debug. With the changes in this PR, this would raise an error instead.

Furthermore, disnake.Client, disnake.AutoShardedClient, disnake.Bot, dinake.AutoShardedBot, disnake.InteractionBot and disnake.AutoShardedInteractionBot now all have their __init__s set in such a way that all possible kwargs display (unless I missed any, though I don't believe I did).

Checklist

• [x] If code changes were made, then they have been tested
  • [ ] I have updated the documentation to reflect the changes
  • [x] I have formatted the code properly by running task lint or pre-commit run --all-files
• [x] This PR fixes an issue
• [ ] This PR adds something new (e.g. new method or parameters)
• [x] 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]

A possible future improvement could also be removing **kwargs from Client._get_state, but that's somewhat out of scope for this PR as it wouldn't affect the public interface at all

[Sharp-Eyes]

That should be all (for now), fingers crossed ^^

[onerandomusername]

@Chromosomologist could you please update this pull with the newest options?

[Sharp-Eyes]

Alright, merging and updating with new params should be done, but I haven't had time to test yet.

[onerandomusername]

hi @Chromosomologist, would you please update this pull?

[Sharp-Eyes]

I do like the changes this pr makes, but I am curious about keeping **kwargs and an argument, allow_extraneous_arguments which if False, (the default) raises TypeError about unexpected arguments. However, if this True, a warning is emitted that an extra argument was provided and nothing else occurs

Sounds like a decent idea, yeah. I'll try to get this added, too.

[Sharp-Eyes]

Explicit kwargs for CommonBotBase and the requested allow_extraneous_arguments arg are still to-do; that should be all.

[shiftinv]

I do like the changes this pr makes, but I am curious about keeping **kwargs and an argument, allow_extraneous_arguments which if False, (the default) raises TypeError about unexpected arguments. However, if this True, a warning is emitted that an extra argument was provided and nothing else occurs

This is not required but would be nice for people updating IMO.

Not sure if I understand the purpose of this 🤔 If you were to update, you'd need to modify your code regardless, either fixing the extra args, or adding that allow_extraneous_arguments argument. Is there any practical reason to allow passing extra args?

[Sharp-Eyes]

Not sure if I understand the purpose of this 🤔 If you were to update, you'd need to modify your code regardless, either fixing the extra args, or adding that allow_extraneous_arguments argument. Is there any practical reason to allow passing extra args?

True, since it'd require a code change either way it's somewhat nonsensical.

Either way, the PR should be done now, changes-wise (assuming we agree on not adding the allow_extraneous_arguments argument). It does still need some amount of testing, though I may not have enough time to properly do so before I leave.

[onerandomusername]

(assuming we agree on not adding the allow_extraneous_arguments argument)

Yeah, it makes sense to not add this, as this would lead to bugs and doesn't make much sense in end-user code for purposes that aren't what I'm doing.

[onerandomusername]

@Chromosomologist would you please address the reviews on this pr?

[shiftinv]

Now that extra parameters result in errors, the warning about the sync_permissions parameter can be removed, which then also correctly results in an error if provided: https://github.com/DisnakeDev/disnake/blob/4c17fdc45a88dffd215db6b824c8f3e88dfe81b3/disnake/ext/commands/interaction_bot_base.py#L157-L158

I'm pretty sure this is the final change I'd propose in this PR, it's been going on for long enough at this point :p The only remaining discussion is about MISSING and None, haven't really kept up with it so perhaps you already came to a conclusion there.

[Sharp-Eyes]

There already is some prior ordering into different categories (see this discussion); whether that's better or worse than alphabetizing them, I am not really sure.

[onerandomusername]

I think we should have parameters ordered alphabetically but other than that, should be good to go

@shiftinv is this worth putting in a second pr instead of here, as I think it might be.

[shiftinv]

is this worth putting in a second pr instead of here

Would be better in a second PR, but I wouldn't mind if it was added in this one :shrug: