Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No errors for keyword arguments in class statement #6403

Closed
JelleZijlstra opened this issue Nov 9, 2023 · 7 comments
Closed

No errors for keyword arguments in class statement #6403

JelleZijlstra opened this issue Nov 9, 2023 · 7 comments
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request

Comments

@JelleZijlstra
Copy link
Contributor

Example:

from typing import TypedDict

class TD(TypedDict, a=3):
    a: int

https://pyright-play.net/?code=GYJw9gtgBALgngBwJYDsDmUkQWEMoAqiApgCYAiSAxjAFC1UA2AhgM6uHkAURCZlNADRRmAXgDMASgBctKPJHTMKGEA

This fails at runtime, and type checkers should reject it too. (cf. python/mypy#16438)

@JelleZijlstra JelleZijlstra added the bug Something isn't working label Nov 9, 2023
@erictraut
Copy link
Collaborator

This is currently by design. Pyright doesn't ever report extra keyword arguments passed in a class declaration. So this issue isn't specific to TypedDict.

I'm going to change this from a bug to an enhancement request and consider it in that light. I think it's a reasonable feature to add, assuming it doesn't produce too many false positives in typical code bases.

@erictraut erictraut added enhancement request New feature or request and removed bug Something isn't working labels Nov 9, 2023
@erictraut erictraut changed the title No errors for keyword arguments in TypedDict class statement No errors for keyword arguments in class statement Nov 9, 2023
@JelleZijlstra
Copy link
Contributor Author

Thanks, I didn't know that. For normal classes, keyword arguments may be valid, depending on the metaclass and __init_subclass__ in base classes. It would be good if pyright could check keyword arguments for correctness, but it's not simple. For TypedDict classes, however, any keyword arguments are always invalid.

@erictraut
Copy link
Collaborator

Currently, pyright does check __init_subclass__ if it's defined somewhere in the class hierarchy other than the object class. The object.__init_subclass__ method is ignored. I don't recall why I did it this way. It may have been because of TypedDict, which is a special form and doesn't define an __init_subclass__ or a metaclass.

@erictraut
Copy link
Collaborator

The reason pyright is not flagging the error for a TypedDict is because of a faulty type definition in typing.pyi and typing_extensions.pyi. I suspect the same is true for mypy. Both of these stubs define _TypedDict with a metaclass ABCMeta (which is a lie — the actual runtime metaclass is _TypedDictMeta). ABCMeta's __new__ method accepts any keyword arguments, which is why you're not seeing an error in your code sample above.

I propose that we fix this by modifying the definition of _TypedDict as follows.

@type_check_only
class _TypedDictMeta(type):
    def __new__(
        __mcls: type[typing_extensions.Self],
        __name: str,
        __bases: tuple[type, ...],
        __namespace: dict[str, Any],
        *,
        total: bool = False,
    ) -> typing_extensions.Self: ...

@type_check_only
class _TypedDict(Mapping[str, object], metaclass=_TypedDictMeta):
    ...

This not only addresses the bug and more closely reflects the actual runtime class hierarchy, but it also has the benefit of providing better in-editor support for the total parameter.

Thoughts?

@JelleZijlstra
Copy link
Contributor Author

Good catch, that sounds like a good idea!

Type checkers may still need a special case to prohibit a custom metaclass= on TypedDict, but your suggestion should automatically prohibit other keyword arguments.

erictraut added a commit that referenced this issue Nov 9, 2023
…class` statement when the class has no custom metaclass or `__init_subclass__` in its hierarchy. In this case, the `object.__init_subclass__` method applies, and it accepts no additional keyword arguments. Also improved the error reporting for `__init_subclass__` in general. This addresses #6403.
erictraut added a commit that referenced this issue Nov 9, 2023
…class` statement when the class has no custom metaclass or `__init_subclass__` in its hierarchy. In this case, the `object.__init_subclass__` method applies, and it accepts no additional keyword arguments. Also improved the error reporting for `__init_subclass__` in general. This addresses #6403. (#6405)
@erictraut
Copy link
Collaborator

I've improved pyright's handling of __init_subclass__ to handle the case where there is no custom metaclass with a __new__ method and no __init_subclass__ that overrides object.__init_subclass__. I've also improved the error messages for __init_subclass__ in general. This set of changes still doesn't result in an error for the code in the original sample above, but that will be fixed once the _TypedDict definition is updated in typeshed.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Nov 9, 2023
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.336, which I just published. It will also be included in a future release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants