plugins.rules: fix conflicting attributes in base classes

[Exirel]

Description

One PR, two issues:

• LGTM issues that are better fixed than ignored
• Type hint warning for documentation
• Update documentation to replace the mixin by the abstract named rule class
• Fix a deprecated usage of an abstract method decorator in Rule

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

[dgw]

Below is more of an idea. The "request changes" review status is about what I said in the line notes.

Surely there's a way to deal with this that doesn't require removing the mixin pattern? Off the top of my head, adding an AnonymousRuleMixin that provides get_rule_label() for Rule, leaving NamedRuleMixin to provide it for the command-type rules.

[dgw]

Was troubleshooting for some other doc-related thing I'm trying to fix, and was checking to see if this patch affected that issue at all. It doesn't, but test-building the docs on this branch showed me that it breaks some of docs/source/plugin/internals.rst due to the reference left behind to a NamedRuleMixin class that no longer exists after this change.

[Exirel]

I stole the type-hint fix from #2223 and also the classmethod/abstractclassmethod warning since I'm touching the same files/area.

[dgw]

I stole the type-hint fix from #2223 and also the classmethod/abstractclassmethod warning since I'm touching the same files/area.

I wouldn't do that just because you're touching the same stuff. Let this PR be about fixing the code, and #2223 can be about making the docs better.

[Exirel]

My main issue is that I need to check if the doc properly build, which it doesn't, and specifically on the classes I touch/modify. So maybe I just need #2223 to be merged before I can move forward.

As for the stolen part, don't worry: I did forgot the co-author tag, and that's my fault, I was a bit caught up into too many PRs at once, and I should have been more careful.

[dgw]

Yeah, I'm just dropping this line note for posterity, not because I want something to change as a result. Let's say this is ready.

[dgw]

My main issue is that I need to check if the doc properly build, which it doesn't, and specifically on the classes I touch/modify. So maybe I just need #2223 to be merged before I can move forward.

I spent a bit of time on that today and will shortly post a follow-up there with some things I noticed. But the bottom line for this patch is going to be that I no longer think it's a good idea to mess with Sphinx in this PR. Let's get the code changes from this in, only, and then I'll resume work on #2223 to check for any other warnings/issues in the docs.

Part of this patch no longer seems wise; see comments.

[Exirel]

I no longer think it's a good idea to mess with Sphinx in this PR

I kept the strict minimum that makes sense in this PR regarding the documentation: I replaced the autodoc for the mixin by the autodoc for the abstract class. I feel like it's a fair change of the documentation while not messing with Sphinx, i.e. not trying to fix too much.

[dgw]

Ack, thanks Exirel. I'll approve and merge once squashed.

[Exirel]

And done.

[dgw]

Remaining changes look sane, plus I know we have oodles of tests for this part of the code. 😏