[refurb] add list of base class exceptions for FURB180

[robsdedude]

Summary

Add a config option tool.ruff.lint.refurb.allow-abc-meta-bases (["typing.Protocol", "typing_extensions.Protocol"] by default) that allows to specify a list such that if class has at least one base-class and all base-classes are in this list it is exempt from FURB180. This is useful for classes that validate their base-classes at runtime such as the two that are the default value.

Test Plan

Besides the adjusted test ruff_linter::rules::refurb::tests::rules, I created a minimal Python project to test the configuration option.

References

Fixes #13307 (together with #18149)

[MichaReiser]

@ntBre could you take a look at this PR when you're back?

[ntBre]

Thanks, this all makes sense to me! I just had a couple of minor suggestions.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[ntBre]

Thanks, this is looking good. Just a couple more suggestions, and one question about naming

[ntBre]

Awesome, thanks again! This is good to go from my perspective, modulo any further bike-shedding on the name.

[robsdedude]

@ntBre Just want to make sure this is still on your radar 👀

[ntBre]

Oh yes, thank you! Let me actually cc someone who might have an opinion on the name (@MichaReiser any thoughts on the names and maybe the two separate options?)

I believe the current state is that we added two new config options:

• lint.refurb.allowed_abc_meta_bases
• lint.refurb.extend_allowed_abc_meta_bases

[MichaReiser]

Do we need to gate this behind preview, given that we now emit fewer lints (which can lead to unused noqa comments) or do we consider this a bug fix?

From our versioning policy

A new configuration option is added in a backwards compatible way (no formatting changes or new lint errors)

So I think we need to gate this behind preview

[MichaReiser]

Oh yes, thank you! Let me actually cc someone who might have an opinion on the name (@MichaReiser any thoughts on the names and maybe the two separate options?)

I'm not entirely sure I understnad what you want feedback on. I see that this PR introduces two new options. Are you asking for my opinion on the option names? Is there mroe feedback that you're looking for?

Looking at the issue #13307 (comment) it's unclear if we want these options. More specifically, options have a non-zero cost because users need to understand them. This PR also changes the default for everyone using the rule, which may not what they want.

[robsdedude]

This PR also changes the default for everyone using the rule, which may not what they want.

That's a fair point. I figured if users are in the situation where this becomes relevant, they have code along the lines of

class Foo(Protocol, metaclass=ABCMeta): ...

I imagine if a user wants this to be linted, the lint shouldn't be "use ABC as a base instead of ABCMeta", but rather "don't bother specifying ABC/ABCMeta if the base(s) already includes ABC in the class hierarchy".