Thoughts on improving B902?

[snarkmaster]

I think B902 is great for detecting errors like this:

@classmethod
def foo(self):
    ...

However, I also find myself # noqa:ing it a bit too frequently. Here are some instances where I think it could be safely relaxed for everyone:

• Allow _cls instead of cls (ditto for metacls). Rationale: this can be required when a method needs to take, or transparently pass through, a keyword argument with one of these names.
• The first argument to __new__ of classes that subclass type should probably be called metacls, not cls. And similarly — regular instance methods on metaclasses should take cls, and any classmethods should take metacls.
• Consider allowing subcls / submetacls (depending on inheritance) for __init_subclass__, since that is actually significantly clearer.

What do you think?

[ambv]

In general I am in favor of making this less noisy. If we can detect that the user does something reasonable, we shouldn't warn. So I'm open to pull requests to that effect.

As for "subclassing type", I am already making a check like you're describing. There are passing tests to that effect. Maybe what you're seeing is this: Flake8 plugins are rather dumb, they don't see the full MRO of a class. So my fix only helps in the case where "type" is explicitly listed as a base on the class which we're analyzing now.

[snarkmaster]

don't see the full MRO

That makes sense. Integrating with mypy for type analysis sounds like a horrible pain (at least in its present incarnation).

less noisy

How do you feel about false negatives? I.e. if we have __new__ and the first arg is metacls, can we just assume good intent, even if type is not one of the explicit bases?

I think that unconditionally accepting the underscore-prefix version of the canonical variable sounds safe. Do you?

As far as subcls / submetacls — I would just add those to the list of canonical variables for __init_subclass__, rather than (opinionatedly) requiring them. Agreed?

[ambv]

All of your proposed changes sound good to me.

[mar10]

Here is another case where B902 is flagged, where it should not (IMO):
@abstractclassmethod should allow/expect the 'cls' argument name in the same way as @classmethod does.

from abc import ABC, abstractclassmethod

class Foo:
    @abstractclassmethod
    def bar(cls, baz):  # noqa: B902
        """..."""

[DylanYoung]

See https://github.com/PyCQA/pep8-naming#options for an alternate implementation that can leave a lot of these decisions up to the user.