Skip to content

Narrow RegexValidator.regex instance level type to Pattern[str] #2650

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.

Sign up for GitHub

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

Closed
wants to merge 9 commits into from
Closed

Narrow RegexValidator.regex instance level type to Pattern[str] #2650

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
03f7ea8
Narrow `REgexValidator.regex` type because it cast to a pattern in init
UnknownPlatypus May 2, 2025
d7f9a54
Use a descriptor to describe the type
UnknownPlatypus May 2, 2025
e86f563
Fix after CR
UnknownPlatypus May 2, 2025
4647a3e
Amend the allowlist
UnknownPlatypus May 2, 2025
af7948e
Add pyright ignores too + use `typing_extensions`
UnknownPlatypus May 2, 2025
ec1a145
Add missing pyright ignores
UnknownPlatypus May 2, 2025
0ca8419
Add test cases
UnknownPlatypus May 3, 2025
31b6939
Remove descriptor approach, It doesn't handle subclassing well
UnknownPlatypus May 3, 2025
1d4eff0
Correct pyright ignore
UnknownPlatypus May 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions django-stubs/core/validators.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from collections.abc import Callable, Collection, Sequence, Sized
from decimal import Decimal
from re import Pattern, RegexFlag
from typing import Any, TypeAlias
from typing import Any, Generic, TypeAlias, TypeVar, overload, type_check_only

from django.core.files.base import File
from django.utils.deconstruct import _Deconstructible
Expand All @@ -13,8 +13,19 @@ _Regex: TypeAlias = str | Pattern[str]

_ValidatorCallable: TypeAlias = Callable[[Any], None] # noqa: PYI047

_ClassT = TypeVar("_ClassT")
_InstanceT = TypeVar("_InstanceT")

@type_check_only
class _ClassOrInstanceAttribute(Generic[_ClassT, _InstanceT]):
@overload
def __get__(self, instance: None, owner: type[object]) -> _ClassT: ...
@overload
def __get__(self, instance: object, owner: type[object]) -> _InstanceT: ...
def __set__(self, instance: object, value: _InstanceT) -> None: ...

class RegexValidator(_Deconstructible):
regex: _Regex # Pattern[str] on instance, but may be str on class definition
regex: _ClassOrInstanceAttribute[_Regex, Pattern[str]]
message: _StrOrPromise
code: str
inverse_match: bool
Expand Down
6 changes: 6 additions & 0 deletions scripts/stubtest/allowlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,9 @@ django.contrib.gis.forms.BaseModelFormSet.save_m2m

# Dynamically generated in https://github.com/django/django/blob/0ee06c04e0256094270db3ffe8b5dafa6a8457a3/django/core/mail/backends/locmem.py#L24
django.core.mail.outbox

# We use a trick using a descriptor class to represent an attribute with different type at the class and instance level.
# Here to narrow `str | Pattern[str]` at the class level to `Pattern[str]` at the instance level.
django.core.validators.RegexValidator.regex
django.contrib.auth.validators.ASCIIUsernameValidator.regex
django.contrib.auth.validators.UnicodeUsernameValidator.regex
22 changes: 22 additions & 0 deletions tests/assert_type/core/test_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import re
from re import Pattern

from django.contrib.auth.validators import UnicodeUsernameValidator
from django.core.validators import RegexValidator
from typing_extensions import assert_type

assert_type(RegexValidator.regex, str | Pattern[str])
assert_type(RegexValidator().regex, Pattern[str])
RegexValidator().regex = re.compile("")

assert_type(UnicodeUsernameValidator.regex, str | Pattern[str])
assert_type(UnicodeUsernameValidator().regex, Pattern[str])
UnicodeUsernameValidator().regex = re.compile("")

# expect "Pattern[str]"
RegexValidator().regex = "" # type: ignore[assignment] # pyright: ignore[reportAttributeAccessIssue]
UnicodeUsernameValidator().regex = "" # type: ignore[assignment] # pyright: ignore[reportAttributeAccessIssue]

# expect "_ClassOrInstanceAttribute[Union[str, Pattern[str]], Pattern[str]]"
RegexValidator.regex = "anything fails here" # type: ignore[assignment] # pyright: ignore[reportAttributeAccessIssue]
UnicodeUsernameValidator.regex = "anything fails here" # type: ignore[assignment] # pyright: ignore[reportAttributeAccessIssue]
Copy link
Member

@sobolevn sobolevn May 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, one more idea: will subtyping work correctly after this change?

We need to test two cases:

class RegexSubtype(RegexValidator):
     regex = re.compile('abc')

class StrSubtype(RegexValidator):
      regex = 'abc'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, It causes the same errors, I suppose it's a blocker.

tests/assert_type/core/test_validators.py:27: error: Incompatible types in assignment (expression has type "Pattern[str]", base class "RegexValidator" defined the type as "_ClassOrInstanceAttribute[Union[str, Pattern[str]], Pattern[str]]")  [assignment]
tests/assert_type/core/test_validators.py:31: error: Incompatible types in assignment (expression has type "str", base class "RegexValidator" defined the type as "_ClassOrInstanceAttribute[Union[str, Pattern[str]], Pattern[str]]")  [assignment]

This will probably be a blocker for #2615 too otherwise field subclass with custom widgets will raise errors (and this is quite a common pattern for custom fields).

I feel like maybe the initial version of this PR is the best we can get.
We would have accurate type on the instance and a slightly too specific type on the class causing an issue for this case:

class StrSubtype(RegexValidator):
      regex = 'abc' # error: Incompatible types in assignment (expression has type "str", base class "RegexValidator" defined the type as "Pattern[str]")  [assignment]

But it can be solved by using re.compile("abc"). I think it's mosty good because it's more explicit about the fact it's a regex.

The same applies for #2615 were we could do the same but it would require every field subclasses to pass instances and not classes (django already handle this correctly when instanciating)

class MyField(CharField):
-    widget = MyWidget
+    widget = MyWidget()

class MyEurosField(CharField):
    widget = MyWidget(unit="€") # Already good, it's an instance

This might cause a bit more churn but is not too bad to fix.

What do you think @sobolevn ? Maybe you have other ideas on how to make the descriptor approach work ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to revert any descriptor-based changes, looks like it is too complex :(

Loading