Initial attempt

Author: sharkdp

crates/ty_python_semantic/resources/mdtest/public_types.md

@@ -18,8 +18,7 @@ def outer() -> None:
     x = A()
 
     def inner() -> None:
-        # TODO: should be `Unknown | A | B`
-        reveal_type(x)  # revealed: Unknown | B
+        reveal_type(x)  # revealed: Unknown | A | B
     inner()
 
     x = B()
@@ -34,8 +33,7 @@ def outer(flag: bool) -> None:
     x = A()
 
     def inner() -> None:
-        # TODO: should be `Unknown | A | B | C`
-        reveal_type(x)  # revealed: Unknown | B | C
+        reveal_type(x)  # revealed: Unknown | A | B | C
     inner()
 
     if flag:
@@ -53,18 +51,34 @@ def outer(flag: bool) -> None:
 If a binding is not reachable, it is not considered in the public type:
 
 ```py
-def outer(flag: bool) -> None:
+def outer() -> None:
     x = A()
 
     def inner() -> None:
-        # TODO: should be `Unknown | A | C`
-        reveal_type(x)  # revealed: Unknown | C
+        reveal_type(x)  # revealed: Unknown | A | C
+    inner()
+
     if False:
         x = B()
         inner()
 
     x = C()
     inner()
+
+def outer(flag: bool) -> None:
+    x = A()
+
+    def inner() -> None:
+        reveal_type(x)  # revealed: Unknown | A | C
+    inner()
+
+    if flag:
+        return
+
+        x = B()
+
+    x = C()
+    inner()
 ```
 
 If a symbol is only conditionally bound, we do not raise any errors:
@@ -113,8 +127,7 @@ def outer(flag: bool) -> None:
     x = A()
 
     def inner() -> None:
-        # TODO: should be `Unknown | A | B`
-        reveal_type(x)  # revealed: Unknown | A
+        reveal_type(x)  # revealed: Unknown | A | B
     if flag:
         x = B()
         inner()
@@ -124,6 +137,24 @@ def outer(flag: bool) -> None:
     inner()
 ```
 
+Interplay with type narrowing:
+
+```py
+def outer(x: A | None):
+    def inner() -> None:
+        reveal_type(x)  # revealed: A | None
+    inner()
+    if x is None:
+        inner()
+
+def outer(x: A | None):
+    if x is not None:
+        def inner() -> None:
+            # TODO: should ideally be `A`
+            reveal_type(x)  # revealed: A | None
+        inner()
+```
+
 The same set of problems can appear at module scope:
 
 ```py
@@ -134,9 +165,8 @@ if flag():
     x = 1
 
     def f() -> None:
-        # TODO: no error, type should be `Unknown | Literal[1, 2]`
         # error: [possibly-unresolved-reference]
-        reveal_type(x)  # revealed: Unknown | Literal[2]
+        reveal_type(x)  # revealed: Unknown | Literal[1, 2]
     # Function only used inside this branch
     f()
 

crates/ty_python_semantic/resources/mdtest/scopes/eager.md

@@ -12,7 +12,7 @@ Function definitions are evaluated lazily.
 x = 1
 
 def f():
-    reveal_type(x)  # revealed: Unknown | Literal[2]
+    reveal_type(x)  # revealed: Unknown | Literal[1, 2]
 
 x = 2
 ```
@@ -299,7 +299,7 @@ def _():
     x = 1
 
     def f():
-        # revealed: Unknown | Literal[2]
+        # revealed: Unknown | Literal[1, 2]
         [reveal_type(x) for a in range(1)]
     x = 2
 ```
@@ -316,7 +316,7 @@ def _():
 
     class A:
         def f():
-            # revealed: Unknown | Literal[2]
+            # revealed: Unknown | Literal[1, 2]
             reveal_type(x)
 
     x = 2
@@ -333,7 +333,7 @@ def _():
 
     def f():
         def g():
-            # revealed: Unknown | Literal[2]
+            # revealed: Unknown | Literal[1, 2]
             reveal_type(x)
     x = 2
 ```
@@ -351,7 +351,7 @@ def _():
 
     class A:
         def f():
-            # revealed: Unknown | Literal[2]
+            # revealed: Unknown | Literal[1, 2]
             [reveal_type(x) for a in range(1)]
 
     x = 2

crates/ty_python_semantic/resources/mdtest/terminal_statements.md

@@ -580,15 +580,13 @@ resolve free references _at the end of the containing scope_. That means that in
 all of the `x` bindings should be visible to the `reveal_type`, regardless of where we place the
 `return` statements.
 
-TODO: These currently produce the wrong results, but not because of our terminal statement support.
-See [ruff#15777](https://github.com/astral-sh/ruff/issues/15777) for more details.
-
 ```py
 def top_level_return(cond1: bool, cond2: bool):
     x = 1
 
     def g():
-        # TODO eliminate Unknown
+        # TODO We could potentially eliminate Unknown here, if we make sure
+        # that `g` is only called from within `top_level_return`.
         reveal_type(x)  # revealed: Unknown | Literal[1, 2, 3]
     if cond1:
         if cond2:
@@ -601,8 +599,7 @@ def return_from_if(cond1: bool, cond2: bool):
     x = 1
 
     def g():
-        # TODO: Literal[1, 2, 3]
-        reveal_type(x)  # revealed: Unknown | Literal[1]
+        reveal_type(x)  # revealed: Unknown | Literal[1, 2, 3]
     if cond1:
         if cond2:
             x = 2
@@ -614,8 +611,7 @@ def return_from_nested_if(cond1: bool, cond2: bool):
     x = 1
 
     def g():
-        # TODO: Literal[1, 2, 3]
-        reveal_type(x)  # revealed: Unknown | Literal[1, 3]
+        reveal_type(x)  # revealed: Unknown | Literal[1, 2, 3]
     if cond1:
         if cond2:
             x = 2

crates/ty_python_semantic/resources/mdtest/unreachable.md

@@ -241,16 +241,16 @@ def f():
 
 ### Use of variable in nested function
 
-In the example below, since we use `x` in the `inner` function, we use the "public" type of `x`,
-which currently refers to the end-of-scope type of `x`. Since the end of the `outer` scope is
-unreachable, we need to make sure that we do not emit an `unresolved-reference` diagnostic:
+This is a regression test for a behavior that previously caused problems when the public type still
+referred to the end-of-scope, which would result in an unresolved-reference error here since the end
+of the scope is unreachable.
 
 ```py
 def outer():
     x = 1
 
     def inner():
-        reveal_type(x)  # revealed: Unknown
+        reveal_type(x)  # revealed: Unknown | Literal[1]
     while True:
         pass
 ```

crates/ty_python_semantic/src/place.rs

@@ -622,7 +622,7 @@ fn place_by_id<'db>(
             place: Place::Type(declared_ty, Boundness::PossiblyUnbound),
             qualifiers,
         }) => {
-            let bindings = use_def.public_bindings(place_id);
+            let bindings = use_def.all_bindings(place_id);
             let inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
 
             let place = match inferred {
@@ -647,7 +647,7 @@ fn place_by_id<'db>(
             place: Place::Unbound,
             qualifiers: _,
         }) => {
-            let bindings = use_def.public_bindings(place_id);
+            let bindings = use_def.all_bindings(place_id);
             let inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
 
             // `__slots__` is a symbol with special behavior in Python's runtime. It can be
@@ -870,9 +870,10 @@ fn place_from_bindings_impl<'db>(
     if let Some(first) = types.next() {
         let boundness = match unbound_reachability() {
             Some(Truthiness::AlwaysTrue) => {
-                unreachable!(
-                    "If we have at least one binding, the implicit `unbound` binding should not be definitely visible"
-                )
+                // unreachable!(
+                //     "If we have at least one binding, the implicit `unbound` binding should not be definitely visible"
+                // )
+                Boundness::Bound // TODO
             }
             Some(Truthiness::AlwaysFalse) | None => Boundness::Bound,
             Some(Truthiness::Ambiguous) => Boundness::PossiblyUnbound,

crates/ty_python_semantic/src/semantic_index/use_def.rs

@@ -298,6 +298,8 @@ pub(crate) struct UseDefMap<'db> {
     /// [`PlaceState`] visible at end of scope for each place.
     end_of_scope_places: IndexVec<ScopedPlaceId, PlaceState>,
 
+    potentially_reachable_bindings: IndexVec<ScopedPlaceId, Bindings>,
+
     /// Snapshot of bindings in this scope that can be used to resolve a reference in a nested
     /// eager scope.
     eager_snapshots: EagerSnapshots,
@@ -401,6 +403,13 @@ impl<'db> UseDefMap<'db> {
         self.bindings_iterator(self.end_of_scope_places[place].bindings())
     }
 
+    pub(crate) fn all_bindings(
+        &self,
+        place: ScopedPlaceId,
+    ) -> BindingWithConstraintsIterator<'_, 'db> {
+        self.bindings_iterator(&self.potentially_reachable_bindings[place])
+    }
+
     pub(crate) fn eager_snapshot(
         &self,
         eager_bindings: ScopedEagerSnapshotId,
@@ -648,7 +657,7 @@ pub(super) struct FlowSnapshot {
 
 #[derive(Debug)]
 pub(super) struct UseDefMapBuilder<'db> {
-    /// Append-only array of [`Definition`].
+    /// Append-only array of [`DefinitionState`].
     all_definitions: IndexVec<ScopedDefinitionId, DefinitionState<'db>>,
 
     /// Builder of predicates.
@@ -679,6 +688,9 @@ pub(super) struct UseDefMapBuilder<'db> {
     /// Currently live bindings and declarations for each place.
     place_states: IndexVec<ScopedPlaceId, PlaceState>,
 
+    /// All potentially reachable bindings, for each place.
+    potentially_reachable_bindings: IndexVec<ScopedPlaceId, Bindings>,
+
     /// Snapshots of place states in this scope that can be used to resolve a reference in a
     /// nested eager scope.
     eager_snapshots: EagerSnapshots,
@@ -700,6 +712,7 @@ impl<'db> UseDefMapBuilder<'db> {
             declarations_by_binding: FxHashMap::default(),
             bindings_by_declaration: FxHashMap::default(),
             place_states: IndexVec::new(),
+            potentially_reachable_bindings: IndexVec::new(),
             eager_snapshots: EagerSnapshots::default(),
             is_class_scope,
         }
@@ -720,6 +733,10 @@ impl<'db> UseDefMapBuilder<'db> {
             .place_states
             .push(PlaceState::undefined(self.reachability));
         debug_assert_eq!(place, new_place);
+        let new_place = self
+            .potentially_reachable_bindings
+            .push(Bindings::unbound(self.reachability));
+        debug_assert_eq!(place, new_place);
     }
 
     pub(super) fn record_binding(
@@ -738,6 +755,14 @@ impl<'db> UseDefMapBuilder<'db> {
             self.is_class_scope,
             is_place_name,
         );
+
+        self.potentially_reachable_bindings[place].record_binding(
+            def_id,
+            self.reachability,
+            self.is_class_scope,
+            is_place_name,
+            false,
+        );
     }
 
     pub(super) fn add_predicate(&mut self, predicate: Predicate<'db>) -> ScopedPredicateId {
@@ -1014,6 +1039,7 @@ impl<'db> UseDefMapBuilder<'db> {
             bindings_by_use: self.bindings_by_use,
             node_reachability: self.node_reachability,
             end_of_scope_places: self.place_states,
+            potentially_reachable_bindings: self.potentially_reachable_bindings,
             declarations_by_binding: self.declarations_by_binding,
             bindings_by_declaration: self.bindings_by_declaration,
             eager_snapshots: self.eager_snapshots,

crates/ty_python_semantic/src/semantic_index/use_def/place_state.rs

@@ -205,7 +205,7 @@ pub(super) struct LiveBinding {
 pub(super) type LiveBindingsIterator<'a> = std::slice::Iter<'a, LiveBinding>;
 
 impl Bindings {
-    fn unbound(reachability_constraint: ScopedReachabilityConstraintId) -> Self {
+    pub(super) fn unbound(reachability_constraint: ScopedReachabilityConstraintId) -> Self {
         let initial_binding = LiveBinding {
             binding: ScopedDefinitionId::UNBOUND,
             narrowing_constraint: ScopedNarrowingConstraint::empty(),
@@ -224,6 +224,7 @@ impl Bindings {
         reachability_constraint: ScopedReachabilityConstraintId,
         is_class_scope: bool,
         is_place_name: bool,
+        clear: bool,
     ) {
         // If we are in a class scope, and the unbound name binding was previously visible, but we will
         // now replace it, record the narrowing constraints on it:
@@ -232,7 +233,9 @@ impl Bindings {
         }
         // The new binding replaces all previous live bindings in this path, and has no
         // constraints.
-        self.live_bindings.clear();
+        if clear {
+            self.live_bindings.clear();
+        }
         self.live_bindings.push(LiveBinding {
             binding,
             narrowing_constraint: ScopedNarrowingConstraint::empty(),
@@ -349,6 +352,7 @@ impl PlaceState {
             reachability_constraint,
             is_class_scope,
             is_place_name,
+            true,
         );
     }