microsoft / pyright

Static Type Checker for Python
Other
13.12k stars 1.4k forks source link

Relax reportSelfClsParameterName to allow mcs , mcls, and metacls when inheriting from type. #8717

Closed DylanLukes closed 1 month ago

DylanLukes commented 1 month ago

When implementing metaclasses, it is relatively commonplace in methods such as __new__ and @classmethod's such as __prepare__ to name the first positional argument mcs, mcls or metacls.

This has a few benefits. In particular, it allows for the name cls to be used for an actual class, such as in:

def __new__(metacls, cls_name: str, cls_bases: tuple[type, ...], cls_attrs: dict[str. Any], **kwargs):
    cls = super().__new__(metacls, cls_name, cls_bases, cls_attrs)
    # do things with cls
    return cls

This is a good bit clearer than naming the first parameter cls and requiring a different name to be used in the body. The current inspection, I think, forces either:

This seems like such a common naming pattern that it

Usage in the Wild

While not ubiquitous, these naming choices are certainly common in many large and old projects, and besides that the intent behind them are immediately clear (except perhaps mcs).

Related to a Problem?

Not really, just an inconvenience. Disabling this inspection project-wide is a sledgehammer for a nail. Ditto for an entire (meta)class. Disabling it per method or per line adds unnecessary noise, especially if there are @overloads in play.

Proposed Solution

Relax reportSelfClsParameterName when the containing class inherits from type, both for @classmethod's in general as well as for __new__ to allow for mcs, mcls and metacls.

erictraut commented 1 month ago

Thanks for the suggestion.

This is one of the oldest parts of the pyright code, and it contains a number of inconsistencies. For example, it allows all of the names you've listed above for __new__ methods within metaclasses, but it uses a different (more restricted) list for class methods. I've reworked the logic so it applies these checks consistently. The names in your list above work fine now.

This change will be included in the next release.

erictraut commented 1 month ago

This is addressed in pyright 1.1.376.