`FURB180` should consider allowlisting some classes

[robsdedude]

Please consider

import abc
from typing_extensions import Protocol


class Foo(Protocol, metclass=abc.ABCMeta):
    ...

Now this rule will re-write the class to class Foo(Protocol, abc.ABC): .... Well, that won't work at runtime :) Protocol checks that it only inherits from other Protocols.

Not sure what the fix is here.

[AlexWaygood]

Hmm, what's the purpose of using metaclass=abc.ABCMeta here? Just inheriting from Protocol sets the metaclass as _ProtocolMeta, which is a subclass of ABCMeta

[robsdedude]

That's a fair point. But I'm a fan of being explicit here.. Imagine someone not being super familiar with Python (or just the typing module). They might easily not know that this is the case. Further, you have to dig into CPython's code to be sure about this. The docs are somewhat vague. They mention "abstract base class" (but not abc.ABC) when following Protocol's inheritance of Generic. Lastly, being explicit makes sure that the assumption is true and remains true in the future (or at least explodes with a metaclass conflict if Python decides to change this).

Regardless whether you agree with my approach or don't think it's worth supporting, the auto-fix does break the code and the documentation is incorrect.

Inheriting from the ABC wrapper class is semantically identical to setting metaclass=abc.ABCMeta, but more succinct.

It's not exactly semantically identical. Only in most cases.

[robsdedude]

I am considering crafting a PR for this. Here are my suggestions in descending order or preference:

1. Introduce a configuration option to allow metclass=abc.ABCMeta if all base classes are in the configure list. By default, the list would be typing.Protocol, typing_extensions.Protocol.
2. Hard code Protocol as an exception to the rule
3. Simply mark the fix as unsafe (if the class has any super-class(es)).

Maybe 3 should happen anyway as this fix cannot be blindly applied without breaking any code.

Before I invest time I'd like to know which option(s) you prefer.

PS: if the label "question" means "user question", I don't think the issue is labeled quite right. It's not really that I don't know how to use the tool, but more that I am of the opinion that this rule needs some small fixing.

[dylwil3]

Could you say a bit more @robsdedude about how this configuration might be used other than the Protocol example?

I feel like the original issue raised along with Alex's comment has been addressed now that the fix is considered unsafe. I don't necessarily find this justification in response to Alex's point super convincing:

Imagine someone not being super familiar with Python (or just the typing module). They might easily not know that this is the case.

If someone is not super familiar with Python, would they be writing code like the original example in the first place?

Anyway, I'm open to the idea of the configuration option, I just don't have a clear picture of the use-cases.

[robsdedude]

@dylwil3 there's also a discussion in parallel on the linked PR, in case you've not seen that. To not spread the high-level discussion about whether the option is even desired, I'll address parts of that MichaReiser's comment here as well.

---

[I] don't necessarily find this justification in response to Alex's point super convincing

Imagine someone not being super familiar with Python (or just the typing module). They might easily not know that this is the case.

Yeah, they would 😇 that "they" was me. I mean, I wouldn't necessarily say I'm not familiar with Python, but before looking it up, I wouldn't have know whether Protocol inherits from ABC/has ABCMeta (or a subclass) as its metaclass or not.

But even after finding out, I still want to keep that ABCMeta there for the aforementioned reason

being explicit makes sure that the assumption is true and remains true in the future (or at least explodes with a metaclass conflict if Python decides to change this).

---

I just don't have a clear picture of the use-cases.
Besides Protocol, I figured there could be any user-defined class or class defined in a 3rd party package that alters runtime behavior depending on subclasses' bases (validates bases, changes behavior depending on present bases, ...), and if that class requires you to use ABCMeta instead of ABC, this option would help.

Imagine a class that like Protocol validates that no other bases are present, but unlike Protocol does not inherit from ABC. This option would allow users to allow-list such classes instead of heaving to slap a # noqa at every sub-class definition. So in this case there isn't even a work-around of the sorts "just omit ABC all together", just noqa.

---

From @MichaReiser's comment on the PR

More specifically, options have a non-zero cost because users need to understand them.

That's a good point I've not considered.

Less than before but still I'd still like to see the option become a thing because I like the core of FURB180, but I disagree with having to work around it here in order to be explicit with my class being an abstract base class without having to use noqa comments.