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

CheckRegistry().register() should allow *tags when called as function #2232

Closed
jwhitlock opened this issue Jun 24, 2024 · 0 comments · Fixed by #2233
Closed

CheckRegistry().register() should allow *tags when called as function #2232

jwhitlock opened this issue Jun 24, 2024 · 0 comments · Fixed by #2233
Labels
bug Something isn't working

Comments

@jwhitlock
Copy link
Contributor

Bug report

The type hints for CheckRegistry.register do not allow passing tags when calling as a function:

class CheckRegistry:
registered_checks: set[_ProcessedCheckCallable]
deployment_checks: set[_ProcessedCheckCallable]
def __init__(self) -> None: ...
@overload
def register(self, check: _C, /) -> _ProcessedCheckCallable[_C]: ...
@overload
def register(self, *tags: str, **kwargs: Any) -> Callable[[_C], _ProcessedCheckCallable[_C]]: ...

What's wrong

According to the System check framework docs, these (annotated) variants should be equivalent:

from typing import Any, Sequence, Optional, Union, List
from django.apps.config import AppConfig
from django.core.checks import register, Tags, messages


@register(Tags.security)
def my_check_decorated(
    app_configs: Union[Sequence[AppConfig], None], **kwargs: Any
) -> List[messages.Warning]:
    return []


def my_check(
    app_configs: Union[Sequence[AppConfig], None], **kwargs: Any
) -> List[messages.Warning]:
    return []


register(my_check, Tags.security)

However, the second version does not pass mypy:

example.py:20: error: No overload variant matches argument types "Callable[[Sequence[AppConfig] | None, KwArg(Any)], list[Warning]]", "str"  [call-overload]
example.py:20: note: Possible overload variants:
example.py:20: note:     def [_C <: _CheckCallable] register(self, _C, /) -> _ProcessedCheckCallable[_C]
example.py:20: note:     def register(self, *tags: str, **kwargs: Any) -> Callable[[_C], _ProcessedCheckCallable[_C]]

How is that should be

The second version should be accepted. This way, register() can be called in AppConfig.ready(), such as in https://github.com/mozilla/django-csp/blob/main/csp/apps.py.

System information

  • OS:
  • python version: 3.12.3
  • django version: 5.0.6
  • mypy version: 1.8.0 (compiled: yes)
  • django-stubs version: 5.0.2
  • django-stubs-ext version: 5.0.2
@jwhitlock jwhitlock added the bug Something isn't working label Jun 24, 2024
jwhitlock added a commit to jwhitlock/django-stubs that referenced this issue Jun 24, 2024
Allow setting tags and "deploy" flag when using register as a function.
Previously, only the decorator overload allowed these.
jwhitlock added a commit to jwhitlock/django-stubs that referenced this issue Jun 25, 2024
Allow setting tags and "deploy" flag when using register as a function.
Previously, only the decorator overload allowed these.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

1 participant