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

[UnknownPlatypus]

I have made things!

This is cast to a lazy Pattern in the __init__ so I think it makes sens to narrow here.

This fixes an issue I had at work when trying to access the regex pattern

validate_not_gmail = RegexValidator(
    regex=r".+@(?!gmail).+\..+",
    message="The email address cannot contain gmail",
)
...
class MyForm(forms.Form):
    email = EmailField(
        label="Mail",
        required=True,
        widget=Input(
            input_type="email",
            attrs={
                "pattern": validate_not_gmail.regex.pattern, # error: Item "str" of "str | Pattern[str]" has no attribute "pattern"
                "title": validate_not_gmail.message,
            },
        ),
        validators=[validate_email_required, validate_not_gmail],
    )

This makes the type of regex on the class slightly off but I think it's better than having it slightly off for instances which people are usually working with.

[sobolevn]

Do we need an overload for class / instance?

[UnknownPlatypus]

From what I've read in this thread https://discuss.python.org/t/support-different-type-for-class-variable-and-instance-variable/54198, descriptor only have a class level getter, setter and deleter only make sense at the instance level so this is probably the best we can get that satisfies mypy and pyright for the most likely usages.

I don't expect many user to try setting regex at the class level, looks like a convoluted way of defining a new class.

[sobolevn]

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'

[UnknownPlatypus]

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 ?

[sobolevn]

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

[sobolevn]

But in order to have more precise type at the instance level, we restrict the types

Sorry, we can't do that either. We can allow something that does not work to happen (false negative), but this will block valid useges (false positive). That's something we try to avoid, unless we are 100% sure that it is worth it.

[UnknownPlatypus]

Then I guess we cannot do more currently. At least I did not find a way. I tried another approach using __new__ but could not get it to work properly for both case either.

I'll close the PR, we might be able to revisit once the type system allows to represent this.

Thanks for the review nonetheless!

[UnknownPlatypus]

Maybe #2615 will find a way