Cleanup

Author: sharkdp

crates/ty_python_semantic/resources/mdtest/public_types.md

@@ -189,9 +189,7 @@ if flag():
 
 ## Mixed declarations and bindings
 
-Since we currently treat all public uses not just as definitely-bound but also as
-definitely-declared, we do consider the the `A: str` declaration to be definite, and do not consider
-the `A = None` binding in the type of `A`:
+When a declaration only appears in one branch, we also consider types of bindings:
 
 ```py
 def flag() -> bool:
@@ -208,8 +206,8 @@ def _():
     reveal_type(A)  # revealed: str | None
 ```
 
-This pattern appears frequently with conditional imports. Here, the import is treated as both a
-declaration and a binding, and therefore shadows the `None` binding.
+This pattern appears frequently with conditional imports. The `import` statement is both a
+declaration and a binding, but we still add `None` to the public type union:
 
 ```py
 try:
@@ -418,5 +416,7 @@ reveal_type(f)  # revealed: (Overload[(x: int) -> int, (x: str) -> str]) | (Over
 
 def _():
     # TODO: ideally, this should be the same union type as above.
-    reveal_type(f)  # revealed: Overload[(x: int) -> int, (x: bytes) -> bytes]
+    reveal_type(
+        f
+    )  # revealed: (def f(x: int) -> int) | (Overload[(x: int) -> int, (x: str) -> str]) | (Overload[(x: int) -> int, (x: bytes) -> bytes])
 ```

crates/ty_python_semantic/src/place.rs

@@ -840,7 +840,6 @@ fn place_from_bindings_impl<'db>(
         }) if binding.is_undefined_or(is_non_exported) => Some(*reachability_constraint),
         _ => None,
     };
-    let mut all_bindings_definitely_reachable = true;
     let mut deleted_reachability = Truthiness::AlwaysFalse;
 
     // Evaluate this lazily because we don't always need it (for example, if there are no visible
@@ -933,8 +932,6 @@ fn place_from_bindings_impl<'db>(
             }
 
             let binding_ty = binding_type(db, binding);
-            all_bindings_definitely_reachable =
-                all_bindings_definitely_reachable && static_reachability.is_always_true();
             Some(narrowing_constraint.narrow(db, binding_ty, binding.place(db)))
         },
     );
@@ -947,13 +944,7 @@ fn place_from_bindings_impl<'db>(
         };
 
         let boundness = match boundness_analysis {
-            BoundnessAnalysis::AlwaysBound => {
-                if all_bindings_definitely_reachable {
-                    Boundness::Bound
-                } else {
-                    Boundness::PossiblyUnbound
-                }
-            }
+            BoundnessAnalysis::AlwaysBound => Boundness::Bound,
             BoundnessAnalysis::BasedOnUnboundVisibility => match unbound_visibility() {
                 Some(Truthiness::AlwaysTrue) => {
                     unreachable!(
@@ -1135,6 +1126,10 @@ fn place_from_declarations_impl<'db>(
                 if all_declarations_definitely_reachable {
                     Boundness::Bound
                 } else {
+                    // For declarations, it is important to consider the possibility that they might only
+                    // be bound in one control flow path, while the other path contains a binding. In order
+                    // to even consider the bindings as well in `place_by_id`, we return `PossiblyUnbound`
+                    // here.
                     Boundness::PossiblyUnbound
                 }
             }