Skip to content

[red-knot] Narrowing on in tuple[...] and in str #17059

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 12 commits into from
Mar 31, 2025

Conversation

MatthewMckee4
Copy link
Contributor

Summary

Part of #13694

Seems there a bit more to cover regarding in and other types, but i can cover them in different PRs

Test Plan

Add in.md file in narrowing conditionals folder

Copy link
Contributor

github-actions bot commented Mar 29, 2025

mypy_primer results

No ecosystem changes detected ✅

@MatthewMckee4 MatthewMckee4 marked this pull request as draft March 30, 2025 00:13
@MatthewMckee4 MatthewMckee4 changed the title [red-knot] Narrowing on in tuple[...] [red-knot] Narrowing on in tuple[...] and in str Mar 30, 2025
@MatthewMckee4
Copy link
Contributor Author

MatthewMckee4 commented Mar 30, 2025

Okay, i realise this is not right

from typing import Literal

def _(x: Literal["a", "b", "c", "d"]):
    if x in "abc":
        reveal_type(x)  # revealed: Literal["a", "b", "c"]
    else:
        reveal_type(x)  # revealed: Literal["d"]

The true type is Literal["a", "b", "c", "ab", "ac", "bc", "abc"]. So i should just infer as str right?

Edit: In fact im wrong in this instance, but if the argument x was str then i think this is the case?

@MatthewMckee4 MatthewMckee4 marked this pull request as ready for review March 30, 2025 00:50
@MatthewMckee4
Copy link
Contributor Author

MatthewMckee4 commented Mar 30, 2025

Im a bit unsure about if we should support other types in str. For example we have these right now:

def _(x: Literal["a"]) -> None:
    if x in "abc":
        reveal_type(x) # revealed: Literal["a"]
def _(x: Literal["a", "b"]) -> None:
    if x in "abc":
        reveal_type(x) # revealed: Literal["a", "b"]
def _(x: str) -> None:
    if x in "abc":
        reveal_type(x) # revealed: str

but we also have:

def _(x: object) -> None:
    if x in "abc":
        reveal_type(x) # revealed: Literal["a", "b", "c"]

*we also get [unsupported-operator] error here
but i feel like this should be str.

My thinking for all StringLiteral and Union containing only StringLiteral, we can narrow the type, but otherwise we can only infer as str? How does this sounds.

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Mar 30, 2025
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't reviewed the code yet, just the tests, but there are some things we'll have to be a bit more conservative about here.

@MatthewMckee4
Copy link
Contributor Author

Okay thank you, I'll look over this again

@AlexWaygood AlexWaygood removed their request for review March 31, 2025 12:18
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior here looks good! I think we can simplify the implementation and make it a bit more general.

@MatthewMckee4
Copy link
Contributor Author

Thank you

@carljm carljm enabled auto-merge (squash) March 31, 2025 23:34
@carljm carljm merged commit 3ad123b into astral-sh:main Mar 31, 2025
22 checks passed
dcreager added a commit that referenced this pull request Apr 1, 2025
* main:
  [red-knot] Add property tests for callable types (#17006)
  [red-knot] Disjointness for callable types (#17094)
  [red-knot] Flatten `Type::Callable` into four `Type` variants (#17126)
  mdtest.py: do a full mdtest run immediately when the script is executed (#17128)
  [red-knot] Fix callable subtyping for standard parameters (#17125)
  [red-knot] Fix more `redundant-cast` false positives (#17119)
  Sync vendored typeshed stubs (#17106)
  [red-knot] support Any as a class in typeshed (#17107)
  Visit `Identifier` node as part of the `SourceOrderVisitor` (#17110)
  [red-knot] Don't infer Todo for quite so many tuple type expressions (#17116)
  CI: Run pre-commit on depot machine (#17120)
  Error instead of `panic!` when running Ruff from a deleted directory (#16903) (#17054)
  Control flow graph: setup (#17064)
  [red-knot] Playground improvements (#17109)
  [red-knot] IDE crate (#17045)
  Update dependency vite to v6.2.4 (#17104)
  [red-knot] Add redundant-cast error (#17100)
  [red-knot] Narrowing on `in tuple[...]` and `in str` (#17059)
@MatthewMckee4 MatthewMckee4 deleted the narrowing-in-tuples branch April 1, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants