Skip to content
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

Wrong LiteralString type set for the join(list[Uknown]) result #9603

Closed
piotr-piatkowski opened this issue Dec 19, 2024 · 2 comments
Closed
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@piotr-piatkowski
Copy link

Describe the bug
The result of join is improperly tagged as LiteralString even if the list contains (non-literal) str

Code or Screenshots

from typing import reveal_type, LiteralString

l1 = [input("A: ")]
v1 = ''.join(l1)
reveal_type(l1)
reveal_type(v1)

l2 = []
l2.append(input("A: "))
v2 = ''.join(l2)
reveal_type(l2)
reveal_type(v2)

l3 = ["A"]
v3 = ''.join(l3)
reveal_type(l3)
reveal_type(v3)

pyright output:

$ pyright test.py
/workspace/test.py
  /workspace/test.py:7:13 - information: Type of "l1" is "list[str]"
  /workspace/test.py:8:13 - information: Type of "v1" is "str"
  /workspace/test.py:13:13 - information: Type of "l2" is "list[Unknown]"
  /workspace/test.py:14:13 - information: Type of "v2" is "LiteralString"
  /workspace/test.py:18:13 - information: Type of "l3" is "list[str]"
  /workspace/test.py:19:13 - information: Type of "v3" is "str"
0 errors, 0 warnings, 6 informations 

First is correct, third is wrong but acceptable (should be LiteralString), but the second is the problem. For the list[Unknown] it should return str as we have no idea if the list contains any non-literal strings (and it definitely does in this example).

VS Code extension or command-line

I'm using Pylance, but I checked also on the pyright, version 1.1.391

@piotr-piatkowski piotr-piatkowski added the bug Something isn't working label Dec 19, 2024
@erictraut
Copy link
Collaborator

Pyright is working as intended here, so I don't consider this a bug.

The typeshed stubs declare join as follows:

class str(Sequence[str]):
    ...
    @overload
    def join(self: LiteralString, iterable: Iterable[LiteralString], /) -> LiteralString: ...
    @overload
    def join(self, iterable: Iterable[str], /) -> str: ...  # type: ignore[misc]

In your second example, the type of self is Literal[''], and l2 is list[Unknown]. This matches both overloads. In this case, pyright looks at the return results to determine if the types overlap. If so, it returns the narrower of the two types. This results in a return type of LiteralString in this case.

The Python typing spec doesn't currently provide guidance to type checkers about how to evaluate calls to overloaded functions. I recently wrote and submitted a proposed addition to the typing spec. I also recently gave a talk about this proposal in a typing meetup. If you'd like to view the presentation, refer to this thread.

Once we get agreement on this proposal and it is officially accepted into the typing spec, I will update pyright to conform to the spec. Mypy and other type checkers will likewise need to be changed. I don't plan to make any changes to pyright's overload behaviors prior to this because it will result in unnecessary churn for pyright users if the proposal is changed before it is ratified.

If the current draft of the proposal is ratified, your second example will evaluate to a return type of Unknown rather than LiteralString.

I'm going to close this issue for now because I don't plan to make any changes unless/until the spec is clarified.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2024
@erictraut erictraut added the as designed Not a bug, working as intended label Dec 19, 2024
@piotr-piatkowski
Copy link
Author

@erictraut: thanks for the detailed explanation.

I see your point, but the problem is that in case of LiteralString vs. str taking "narrower" is contradicting the goal of the LiteralString which I believe was introduced for avoiding things like sql-injection. So it reminds me "tainted" variables from the perl - if you know that concept.

And in general that makes problems - my actual case was also with @overload, like this:

@overload
def foo(with_status: Literal[False] = ...) -> str:
    ...

@overload
def foo(with_status: bool = ...) -> tuple[str,int]:
    ...

def foo(with_status = False):
    status = int(0)
    output = ''.join([])  # Here we get LiteralString
    if with_status:
        return output, status
    return output

And then I've got:

  /workspace/test.py:6:5 - error: Overload 1 for "foo" overlaps overload 2 and returns an incompatible type (reportOverlappingOverload)
  /workspace/test.py:13:5 - error: Overloaded implementation is not consistent with signature of overload 1
    Function return type "str" is incompatible with type "tuple[LiteralString, int] | LiteralString"
      Type "str" is not assignable to type "tuple[LiteralString, int] | LiteralString"
        "str" is not assignable to "tuple[LiteralString, int]"
        "str" is not assignable to "LiteralString" (reportInconsistentOverload)
  /workspace/test.py:13:5 - error: Overloaded implementation is not consistent with signature of overload 2
    Function return type "tuple[str, int]" is incompatible with type "tuple[LiteralString, int] | LiteralString"
      Type "tuple[str, int]" is not assignable to type "tuple[LiteralString, int] | LiteralString"
        "tuple[str, int]" is not assignable to "tuple[LiteralString, int]"
          Tuple entry 1 is incorrect type
            "str" is not assignable to "LiteralString"
        "tuple[str, int]" is not assignable to "LiteralString" (reportInconsistentOverload)

Which is confusing, at least for me - as, yes, str is not assignable to LiteralString but in this case shouldn't this be checked the other way round? And regardless of type for the join result, the same problem occurs when I use simple return "ABC"

I could fix it by adding explicit return type to the actual foo definition, but I think this is only workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants