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

Fix Literal strings containing pipe characters #17148

Merged
merged 11 commits into from
Apr 22, 2024

Conversation

JelleZijlstra
Copy link
Member

Fixes #16367

During semantic analysis, we try to parse all strings as types, including those inside Literal[]. Previously, we preserved the original string in the UnboundType.original_str_expr attribute, but if a type is parsed as a Union, we didn't have a place to put the value.

This PR instead always wraps string types in a RawExpressionType node, which now optionally includes a .node attribute containing the parsed type. This way, we don't need to worry about preserving the original string as a custom attribute on different kinds of types that can appear in this context.

The downside is that more code needs to be aware of RawExpressionType.

This comment has been minimized.

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@JelleZijlstra JelleZijlstra merged commit 1072c78 into python:master Apr 22, 2024
19 checks passed
@JelleZijlstra JelleZijlstra deleted the literalpipe branch April 22, 2024 02:31
andersk added a commit to andersk/mypy that referenced this pull request Jul 26, 2024
Commit 1072c78 (python#17148) converted all
quoted types into RawExpressionType, which raised an AssertionError
when accepting a TypeTriggersVisitor.

Fixes python#17587.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
JelleZijlstra pushed a commit that referenced this pull request Jul 26, 2024
)

Commit 1072c78 (#17148) converted all
quoted types into `RawExpressionType`, which raised an `AssertionError`
when `accept`ing a `TypeTriggersVisitor`.

- Fixes #17574.
- Fixes #17587.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
hauntsaninja pushed a commit that referenced this pull request Jul 27, 2024
)

Commit 1072c78 (#17148) converted all
quoted types into `RawExpressionType`, which raised an `AssertionError`
when `accept`ing a `TypeTriggersVisitor`.

- Fixes #17574.
- Fixes #17587.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@ilevkivskyi
Copy link
Member

I am going to revert this. This breaks a fundamental invariant that synthetic types do not leak from semantic analysis. It looks like the code that caused the problem in the first place was intended to support something like this:

x: Literal["A", "B"]
A = Literal[1, 2]
B = Literal[3, 4]
reveal_type(x)  # Literal[1, 2, 3, 4]

But I am not sure it ever worked (or at least not consistently). Instead I am going to change (either limit or completely prohibit) this behavior.

ilevkivskyi added a commit that referenced this pull request Aug 4, 2024
mr-c pushed a commit to mr-c/mypy that referenced this pull request Aug 14, 2024
md384 pushed a commit to md384/mypy that referenced this pull request Aug 14, 2024
hauntsaninja pushed a commit that referenced this pull request Aug 24, 2024
@JelleZijlstra JelleZijlstra restored the literalpipe branch September 10, 2024 23:37
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.

mypy interprets arguments which contains a pipe to Literal as type strings instead of literals
3 participants