Skip to content

Add PEP 675 LiteralString overloads to str class #7725

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

Merged
merged 8 commits into from
Jun 1, 2022

Conversation

gbleaney
Copy link
Contributor

This PR implements overloads on the str to support LiteralString, as suggested in PEP 675 (note that small changes have been made to fix some typos in the PEP as well as remove the __rmul__ overload).

The changes were tested by running pyre on the following sample file:

# pyre-strict
from typing_extensions import LiteralString

literal_string: LiteralString = ""

def check_literal(s: LiteralString) -> None:
    ...

check_literal(literal_string.capitalize())
check_literal(literal_string.casefold())
check_literal(literal_string.center(0, ""))
check_literal(literal_string.expandtabs(0))
check_literal(literal_string.format(""))
check_literal(literal_string.join([""]))
check_literal(literal_string.ljust(0, ""))
check_literal(literal_string.lower())
check_literal(literal_string.lstrip(""))
check_literal(literal_string.partition("")[0])
check_literal(literal_string.replace("", ""))
check_literal(literal_string.removeprefix(""))
check_literal(literal_string.removesuffix(""))
check_literal(literal_string.rjust(0, ""))
check_literal(literal_string.rpartition("")[0])
check_literal(literal_string.rsplit("")[0])
check_literal(literal_string.rstrip(""))
check_literal(literal_string.split("")[0])
check_literal(literal_string.splitlines()[0])
check_literal(literal_string.strip(""))
check_literal(literal_string.swapcase())
check_literal(literal_string.title())
check_literal(literal_string.upper())
check_literal(literal_string.zfill(0))
# __add__
check_literal(literal_string + literal_string)
# __iter__
for s in literal_string:
    check_literal(s)
# __mod__
check_literal(literal_string % "")
# __mul__
check_literal(literal_string * 0)

Prior to the changes, type errors were found (as expected):

gbleaney@gbleaney-mbp scratch % pyre check
ƛ Found 28 type errors!
test.py:9:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:10:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:11:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:12:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:13:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:14:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:15:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:16:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:17:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:18:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:19:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:20:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:21:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:22:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:23:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:24:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:25:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:26:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:27:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:28:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:29:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:30:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:31:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:32:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:34:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:37:18 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:39:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.
test.py:41:14 Incompatible parameter type [6]: In call `check_literal`, for 1st positional only parameter expected `typing_extensions.LiteralString` but got `str`.

After the changes, no errors were found (as expected):

gbleaney@gbleaney-mbp scratch % pyre check
ƛ No type errors found

@JelleZijlstra
Copy link
Member

Thanks! I think we'll have to upgrade our CI to mypy 0.950 first (which was released minutes ago) to make mypy "support" LiteralString (by treating it as an alias for str).

Also, looks like a few Union and Tuple usages need to be modernized.

@github-actions

This comment has been minimized.

@gbleaney gbleaney force-pushed the literalstring_str_overloads branch from 7c6a5c7 to 56aab63 Compare April 27, 2022 16:44
@github-actions

This comment has been minimized.

@srittau srittau closed this Apr 27, 2022
@srittau srittau reopened this Apr 27, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JelleZijlstra
Copy link
Member

Unfortunately the basic LiteralString support we added in mypy apparently doesn't work. We'll have to figure out what's wrong (python/mypy#12259).

@hauntsaninja
Copy link
Collaborator

@JelleZijlstra I don't think the LiteralString aliasing made it into 0.950. Also I'm not sure I see the connection to the issue you link to, did you mean to link python/mypy#12554?

@JelleZijlstra
Copy link
Member

Oh sorry, thought it made it into 0.950! Then we'll definitely have to wait a bit longer.

@hauntsaninja hauntsaninja added the status: deferred Issue or PR deferred until some precondition is fixed label Apr 27, 2022
@gbleaney
Copy link
Contributor Author

Looks like a new version of mypy was released. Any chance we could bump this for review @JelleZijlstra?

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

Mypy 0.960 recognises typing_extensions.LiteralString as a valid type annotation. However, it doesn't actually support LiteralString yet -- it currently believes LiteralString is a simple alias for str. As a result, as you can see by the CI failures, you'll have to add a lot of type: ignore[misc] comments to the str class if you want to avoid mypy complaints about overlapping overloads.

@AlexWaygood AlexWaygood removed the status: deferred Issue or PR deferred until some precondition is fixed label May 25, 2022
@AlexWaygood
Copy link
Member

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/multipart.py:95: error: Argument 1 to "map" has incompatible type "Callable[[AnyStr], AnyStr]"; expected "Callable[[str], AnyStr]"  [arg-type]

core (https://github.com/home-assistant/core)
+ homeassistant/components/sonos/media.py:180: error: List item 0 has incompatible type "Optional[str]"; expected "None"  [list-item]

For both of these hits, mypy's logic for overloads and mypy's logic for higher-order functions is interacting badly.

@JelleZijlstra
Copy link
Member

@gbleaney could you take a look at the CI failures and add # type: ignore where appropriate to silence mypy.

Ideally we'd also fix the mypy-primer hits, though we can ignore them if they're due to mypy bugs we can't work around. Sometimes using overloads vs. constrained typevars makes a difference in that sort of thing.

@gbleaney
Copy link
Contributor Author

@JelleZijlstra and @AlexWaygood thanks for the suggestions! Might be a few days until I have the time to sit down at a computer, but I'm on it :)

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/multipart.py:95: error: Argument 1 to "map" has incompatible type "Callable[[AnyStr], AnyStr]"; expected "Callable[[str], AnyStr]"  [arg-type]

core (https://github.com/home-assistant/core)
+ homeassistant/components/sonos/media.py:180: error: List item 0 has incompatible type "Optional[str]"; expected "None"  [list-item]

@AlexWaygood AlexWaygood requested a review from JelleZijlstra May 31, 2022 23:26
@JelleZijlstra
Copy link
Member

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/multipart.py:95: error: Argument 1 to "map" has incompatible type "Callable[[AnyStr], AnyStr]"; expected "Callable[[str], AnyStr]"  [arg-type]

https://github.com/aio-libs/aiohttp/blob/15f7398642c131a1c52989829351cba9f1969c32/aiohttp/multipart.py#L95

Reproducer:

from typing import AnyStr
def escape(pattern: AnyStr) -> AnyStr: ...
CHAR: set[str] = {"x"}
"".join(map(escape, CHAR))

core (https://github.com/home-assistant/core)

  • homeassistant/components/sonos/media.py:180: error: List item 0 has incompatible type "Optional[str]"; expected "None" [list-item]

https://github.com/home-assistant/core/blob/0df9cc907ad936df700ae61f6a4dd47514772203/homeassistant/components/sonos/media.py#L180

Repro:

from typing import Any
def sonos(channel: str | None, radio_show: Any):
    " • ".join(filter(None, [channel, radio_show]))

These both seem like mypy getting the type context subtly wrong. I'm OK with merging, but it's likely that more people are going to run into this mypy bug.

@gbleaney what do you think of dropping the LiteralString overload for str.join? Is it common to join an iterable of LiteralStrings?

@gbleaney
Copy link
Contributor Author

gbleaney commented Jun 1, 2022

@JelleZijlstra join was actually one of the first use cases we ran into internally at Meta where we needed an overload to support LiteralString in typeshed. I'm definitely willing to try workarounds for the bug if you have any ideas, but I'd really like to keep support for join on LiteralStrings if possible

@JelleZijlstra
Copy link
Member

OK, then let's just merge this, as I can't think of a way to express the stubs that would avoid those mypy bugs. If the disruption for mypy users is bad enough, we may have to think of an alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants