Repository navigation
Definition of str.split, str.rsplit and str.splitlines results in false positive errors #10887
Description
I recently fixed a bug in pyright that caused a false negative when attempting to assign a list[LiteralString] to a list[str]. As we know, the type parameter for list is invariant, so these types are not consistent.
from typing import LiteralString
v1: list[LiteralString] = ["a", "b"]
v2: list[str] = v1 # Should be a type violationWith this bug fix now in place, the definitions for the methods str.split, str.rsplit and str.splitlines are problematic. These three methods are defined using overloaded signatures, and the first overload returns list[LiteralString]. This means the following code now results in a type violation.
def process(x: list[str]) -> str:
return " ".join(x)
process("1,2,3".split(",")) # Type violationThis is going to generate a lot of new type errors in existing code bases.
I initially attempted to work around this by adding a heuristic in pyright's overload matching logic so it deprioritized the first overload in this case, but this heuristic broke other code and really isn't sound. I've therefore decided to remove the heuristic.
I don't have any good ideas about how to fix this, but I wanted to open the issue to start the discussion to see if anyone had a suggestion. If we're going to fix it, I think the fix needs to be in the typeshed stubs. I don't see a way to fix this in a type checker — at least while remaining true to the type system.
One possible solution is to omit the LiteralString overloads for these three methods. This would eliminate the type violation error in the common case, but it would introduce new errors in cases where callers expect to see a return result of list[LiteralString].
Note that this doesn't currently affect mypy users because mypy has not implemented support for LiteralString.
Activity
Daverball commentedon Oct 14, 2023
I've run into similar issues with overloads before, where sometimes you are forced to have a fairly narrow first overload, since it overlaps with a more general overload, but then you run into exactly this type of issue where type inference sometimes gives you too narrow of a type in cases where it doesn't know which overload to pick, so it defaults to the first one.
It would probably make sense to decouple overload order from what's considered the nominal fallback overload if we don't know which overload matches. Something like an optional
defaultparameter on theoverloaddecorator, or a new decoratordefault_overload. Something like PEP696 but for overloads.It doesn't come up very often, but when it does, it is always frustrating, because there's no satisfying way to solve it, you always end up having to get rid of the more narrow overload and have to deal with false negatives instead.
AlexWaygood commentedon Oct 15, 2023
Broader questions about how
@overloadworks, and possible ways of improving@overload, are somewhat off-topic for the typeshed issue tracker @Daverball :) https://discuss.python.org/c/typing/32 would be a better place to discuss that kind of thing.For this specific question, I'd like the opinions of @gbleaney (author of #7725, and co-author of PEP-675) and @pradeep90 (the other co-author of PEP-675).
I'll note that the PEP specifically lists these methods in Appendix C as methods that should be typed with
LiteralString, so we'll be technically contravening the specification if we stop usingLiteralStringfor these methods. (I don't think it's that big a deal, though.)srittau commentedon Oct 16, 2023
I don't have much to contribute, but note that the underlying problem is users using
list[...]as an argument annotation. This is understandable as it's the most obvious and ergonomic approach (andfoo: list[str]arguably looks "cleaner" thanfoo: Iterable[str]), but maybe we could find a way to make it easier for users to use the "correct" annotation. For example allowingfoo: iter[str](although I'm aware thatiter()returns an iterator) orfoo: seq[str], without needing an import.AlexWaygood commentedon Oct 16, 2023
For now, I suppose I lean towards this solution. Anything else seems too breaking, in my opinion :/
srittau commentedon Oct 17, 2023
I would like to see the impact this would have, compared to the impact of not changing it. There is a reason, the
LiteralStringoverload was introduced, so removing it would most likely negatively affect users as well.gbleaney commentedon Oct 19, 2023
I'm inclined to agree with @srittau and say that the underlying problem is the typing on
processin @erictraut's example, but I understand that that's not much consolation for anyone who's code is full ofLiteralStringtype errors all of a sudden, when they don't even useLiteralStringexplicitly anywhere.LiteralStringis likely always going to be a niche feature, and it's existence shouldn't be causing pain for those who don't use it (maybe this is my C++ "don't pay for what you don't use" mindset sneaking in).I have a few concerns with the proposal of omitting the
LiteralStringoverrides forstr.split,str.rsplitandstr.splitlines:[c for c in "string"]), which could create the same problem outlined in this issue. If we do this, I'd like for us to have a clear line for when we remove these overrides vs ask people to change their typing, rather than doing it piecemeal and inconsistently.My personal preference would be for the overrides to stay, and for fixing/suppressing errors like what @erictraut outlined to be a part of the normal upgrade path. Internally at Meta, when a Pyre upgrade leads to new type errors the we can't automate fixing, we use a Pyre flag to automatically introduce error suppression at the sites of the newly surfaced errors. That lets us keep the type check passing and prevent new issues from being introduced, while not forcing us to block upgrades on massive refactors.
hmc-cs-mdrissi commentedon Oct 20, 2023
My view is usage of LiteralString is closer to a strict typing feature vs one codebase new to typing should have to handle. Pyright, mypy, and I'm assuming other type checkers have flags/strict mode to control which rules are enabled. I think LiteralString ideally should have a flag where enableLiteralString (enabled in --strict like modes) would treat it as separate type and use rules documented in PEP, while enableLiteralString: False (default) would instead treat it as an alias to str.
This would not be first typing feature that has different behavior depending on flag.
overridePEP explicitly specifies that type checkers should offer strict enforcement as,Strict enforcement should be opt-in.erictraut commentedon Oct 20, 2023
That's different. The
overridedecorator doesn't affect type consistency rules. I don't think type consistency rules should change based on the strictness mode of a type checker.codestyle: Use cast() to set type after splitlines()
codestyle: Use cast() to set type after splitlines()
codestyle: Use cast() to set type after splitlines()
codestyle: Use cast() to set type after splitlines()
mikeshardmind commentedon Jan 19, 2024
I commented on this briefly where it came up for someone in the python discord's typing channel. I believe that this is neither the typeshed being wrong nor pyright being wrong. That isn't to say this can't be improved for users though.
It essentially comes from that Literals (and LiteralStr) are refinement types (academic theory, not an established term within python's type system) without any rules other than subtyping to go off of. I think this is worth examining in more detail as part of the spec. It should be okay to promote a refinement type entirely implicitly to a less specific type in some cases, even those involving invariant typevars, and remain type safe, but the rules around this would need review and specification. This would also only ever be safe going from the literal to the type the literal belongs to.
I can help with exploring the ramifications of changing this after other things on my todo list, it may or may not be the best option for python specifically.