plugin: new require_bot_privilege decorator

[Exirel]

Description

This wasn't requested, and probably not needed that much. However, when reading the code for #1981 I realized that we repeat a lot of code, so I figured out I could add these:

• bot.has_channel_privilege(channel, level) will tell you if the bot itself has the right privilege level,
• plugin.require_bot_privilege is a new decorator that uses this new method,

which is probably useful only for channel administration. Probably.

Anyway, it has tests, and it gave me some ideas for the future (in particular, how we could manage more complex privilege check). So it's good!

Second round description

OK, after second thought, I realized I could improve the sopel.tools.target.Channel class, by adding privilege related methods, and use one of these methods into the bot.

However, I found out that there is no test for Channel (or User), and that's not good, so I kind of hijacked this PR to add these tests, while trying to stay on topic.

Checklist

• I have read CONTRIBUTING.md
• I can and do license this contribution under the EFLv2
• No issues are reported by make qa (runs make quality and make test)
• I have tested the functionality of the things this change touches

[half-duplex]

Seems useful. Lgtm besides nitpicks.

[Exirel]

Re-requesting reviews because I added a new commit with some tests on sopel.tools.target.Channel with new methods.

[dgw]

I did the editing thing again, with a bit of bonus commenting on actual code.

[half-duplex]

is_oper is misleading - several levels deeper, what it's actually checking for is +y/+Y in the channel, which when present (and not remapped) does indicate oper, but when absent or unsupported may make is_oper() returning False confusing. is_owner as a Channel method is also more easily confused with Trigger.owner/plugin.require_owner, though that's not a hill I'll die on.

More generally, I'm not sold on the need for is_*() helpers. I can't think of any mainstream case that would necessitate ease of checking for exactly a certain privilege level. While it is documented, I can definitely see someone thinking e.g. is_voice() would tell them if a user could speak when they really want has_privilege(..., plugin.VOICE).
My inclination would be to keep the API smaller and less confusing, and let the few people who need to check for a specific presence and absence check Channel.privileges themselves.

is_*() also further enshrines those priv modes in code when it's theoretically possible to have more/different ones. If we're going to drop any pretense of supporting others, that's fine, but we should acknowledge what we're doing.

This started as a tangential thought, but... Ideally we would track +y/+Y separately, so a bot could avoid harassing an oper joining on official business... Maybe we should track the actual user priv modes, and add only a has_mode("mal", "+h") type thing to allow for generic specific mode querying? Might require WHOX or something to work properly though.

[dgw]

As far as is_oper() goes, I had the same thought but wound up distracted by docstring cleanup. As usual.

Ignoring the specific cases that could be confusing, though: I see @half-duplex's point about enshrining the privilege modes Sopel "supports". My question is then, how could we rework Sopel's channel privilege tracking and requirement enforcement to be more dynamic? If Sopel assigns bitwise values to modes from the server's PREFIX parameter dynamically, it would necessarily compromise plugins' ability to require certain privileges.

Example: On Network A with only RFC-defined modes, +v and +o, the maximum (bitwise) user privilege mask would be 11; but Network B with +v, +h, +o, and +a would have up to 1111. More importantly, if Sopel assigns bit positions to modes in the order given in PREFIX, +o from network A (10) wouldn't be equivalent to the +o from Network B (0100). How could plugins reasonably require "mode +o or higher" then?

I'm not saying that it's impossible, but it would require a thoughtful rewrite of all decorators etc. dealing with privileges. That's obviously out of scope for this PR, though it should inform our decision about whether to introduce the is_*() family of privilege-checking functions at all.

And to stop ignoring the potentially confusing cases: I say we explicitly should not have is_oper() as implemented. Out of all the de facto non-RFC modes that Sopel supports, those are among the weirdest and least widely supported. See data below—which also calls into question whether reworking Sopel's privilege tracking to dynamically read the available user privilege modes from the server's PREFIX is even worthwhile in this context.

IRC networks' advertised PREFIX modes

I (well, my Python console) downloaded all 477 network pages currently linked from netsplit.de's network list and analyzed the PREFIX tokens, where available.

Of the 317 network pages that matched my PREFIX regex, here are how many advertised each mode letter:

v: 317
o: 317
h: 272
a: 206
q: 205
Y: 47
V: 10
u: 2
Z: 2
y: 2
d: 1
f: 1

Outside of the six buckets Sopel hard-codes (v, h, o, a, q, y/Y), the most common is advertised on only 10 networks out of over 300 surveyed. Maybe we should just keep doing what we're doing.

[Exirel]

And to stop ignoring the potentially confusing cases: I say we explicitly should not have is_oper() as implemented.

I honestly didn't know about OPER privilege level: it was here, in the code, as a constant, so I used it, but that's pretty much the full extent of my knowledge about it. It sound sane to remove is_oper in any case.

Now, Sopel has all these constants for years now, and they assume a system where each level is a different power of 2. It's already the assumption made by the code, both when registering user's privileges in channel and when checking for a user's privilege (with decorators such as require_privilege).

I've made, in this PR, 2 decisions:

• decorators use the lax has_privilege, which requires a minimum level, so there is no behavior change in any case
• is_* methods (apart from OPER) are shortcut for something that developers are already doing anyway, so any change that affect this assumption will break things, and I intend these methods to be a way to 1. add deprecation warning if needed and 2. change the underlying way to register privileges without changing the interface

To support these decisions, I've documented how to protect a callable for the bot's privilege level, but there is no specific documentation about how to deal with the Channel object itself. The autodoc part is usually overlooked and more for advanced users, and as seen on IRC, it's usually the doc people don't read anyway.

Now I've a question: do we really care about exotic mode/privilege levels? I mean, IRC is quite old, the only 2 privileges that really matters are VOICE and OP, everything else is mostly luck based, and it's not like Sopel offer some kind of enterprise level support. I think this topic suffer a bit from overthinking the problems.

[Exirel]

I modified the RuntimeError to ValueError, and rebased to be more up to date, given there wasn't more comment about the code itself and more a conversation around the privileges.

Is there a final statement about that? Should I just remove the methods around non-OP/VOICED privilege levels? Should I keep them?

[dgw]

By way of answering @Exirel's question about whether to keep the OPER-related stuff around, I've gone through most of this again and annotated a few typos I missed, plus put forward an idea for warnings in the documentation so plugin developers unfamiliar with how fragmented IRC mode support can be are aware.

[Exirel]

Thanks for the review, I added the important to the other methods and constants.

[dgw]

Ready for history cleanup at your leisure 🚀