Return Promise for lazy functions.

[PIG208]

Django uses a proxy method for lazy translation functions that actually returns instances of Promise instead of str. To be accurate, we should change the signatures for these functions.

Related issues

• Closes gettext_lazy should return a Promise object #688
• Refs gettext_lazy should return a Promise object #688

[sobolevn]

I think that Promise is a new thing, isn't it? Looks like that latest 3.2.x does not have this primitive: https://github.com/django/django/tree/3.2.6/django/utils

[sobolevn]

Moreover, even in latest master it is just a plain class with no methods or anything: https://github.com/django/django/blob/main/django/utils/functional.py#L68

[PIG208]

Hmm.. Promise has been there for quite a while: django/django@e4e28d9. But you are right that Promise is just a plain class. Since the proxy method knows about the resultclasses, I guess creating generics like Promise[str] would be better?

[sobolevn]

Maybe we can add these methods to Promise as well? https://github.com/django/django/blob/3.2.6/django/utils/functional.py#L84-L191

[PIG208]

Hi, I have updated the PR. I think it is probably not necessary to make Promise generic at this point, having these methods should be sufficient.

[PIG208]

There is still a problem with this. Promise proxied other methods of resultclasses, so we probably want those be available on the Promise object as well. Otherwise, these methods won't be usable. I will try to update the plugin for this.

[PIG208]

I believe that this implementation should be good enough for another review. @sobolevn could you take a look at this?

I ditched the generic approach and reimplement this with a simpler plugin handling the use case for lazystr (and *_lazy as well) suggested by @andersk . This is tested with some additional test cases and the entire codebase of Zulip (edit, there still seems to be some issues to be resolved. Will update soon ). The earlier commits are left unchanged.

Looks like the CI is still failing for another unrelated issue. Will look into this in a separate PR probably. Otherwise, I think we are good to go.

[PIG208]

@sobolevn Hi! The PR has been rebased and the CI is passing. I think it is ready for review.

[sobolevn]

I think that you should use analyze_member_access here from mypy/checkmember.py

[PIG208]

Updated.

[sobolevn]

Let's try it out! Thank you!

[hannseman]

This PR causes errors when passing gettext_lazy strings to parameters expecting strings like verbose_name and help_text on model fields:

error: Argument "verbose_name" to "PositiveBigIntegerField" has incompatible type
"_StrPromise"; expected "Optional[str]"  [arg-type]
            null=True, blank=True, verbose_name=_("number")

(where _ is defined as from django.utils.translation import gettext_lazy as _)

[sobolevn]

Please, open a new issue. I am not releasing new version yet, we have time to fix it.

[terencehonles]

@PIG208 was there a reason _StrPromise

django-stubs/django-stubs/utils/functional.pyi

Lines 31 to 46 in fa972f1

	class _StrPromise(Promise, Sequence[str]):
	def __add__(self, __s: str) -> str: ...
	# Incompatible with Sequence.__contains__
	def __contains__(self, __o: str) -> bool: ... # type: ignore[override]
	def __ge__(self, __x: str) -> bool: ...
	def __getitem__(self, __i: SupportsIndex | slice) -> str: ...
	def __gt__(self, __x: str) -> bool: ...
	def __le__(self, __x: str) -> bool: ...
	# __len__ needed here because it defined abstract in Sequence[str]
	def __len__(self) -> int: ...
	def __lt__(self, __x: str) -> bool: ...
	def __mod__(self, __x: Any) -> str: ...
	def __mul__(self, __n: SupportsIndex) -> str: ...
	def __rmul__(self, __n: SupportsIndex) -> str: ...
	# Mypy requires this for the attribute hook to take effect
	def __getattribute__(self, __name: str) -> Any: ...

Is a subclass of Sequence[str] and not str itself? A string is slice-able so I understand the thought process at a high level, but lazystr expects/returns a string not a sequence of strings and I'm not sure why this was made this generic / explicit. This has caused a lot of downstream code (even in this codebase) to need to declare everything as StrOrPromise. While this doesn't seem like that big of a lift, I'm not sure it should be needed at all.

I'm raising this question here instead of creating a separate issue since I figured I'd ask about the intent before completely suggesting this should change (maybe there was a reason I am not aware of).

It seems like Promise should have been made generic and returns and possibly subclasses the type it is so that callers can treat a promise and the object itself interchangeably.

@sobolevn not sure if you have any thoughts or would prefer this to be a separate issue.

[PIG208]

@terencehonles Thanks for bringing this up!

_StrPromise is a subclass of Sequence[str] because this is how builtins.str is typed in typeshed. _StrPromise is defined only because operators don't work well with mypy plugins, while we want methods on a regular str to be all available. These are just the implementation details.

The point of having _StrPromise is to avoid using them with regular str interchangeably. This can cause real bugs when dealing with third-party libraries, because Promise is not str.

We did not make Promise generic because that would require more work to implement a full-blown mypy plugin to support arbitrary type arguments, while the most prevalent use case of Promise is with lazily evaluated strings.

[andersk]

See also #1139 (comment) for concrete examples of what would go wrong if we incorrectly told mypy that _StrPromise is a subclass of str.

[terencehonles]

ok, thanks for the explanation.

The point of having _StrPromise is to avoid using them with regular str interchangeably. This can cause real bugs when dealing with third-party libraries, because Promise is not str.

A Promise[str] seems like it should be mostly considered a string and I'm not sure I agree or have run into an issue that this comment refers to, but I understand it would fail an isinstance(...) check and for mypy and potential future compilation would be concerned that would be an issue.

I had believed that Django's Promise implementation should be a more or less drop in replacement for the object it's promising since I thought it's blockingly coerced into the object when it's used, but it sounds like there's been some discussion of this and I will check out the comment @andersk pointed to 👍