Skip to content

Commit 311730a

Browse files
committed
Analyze boundness closer
1 parent bde535f commit 311730a

File tree

2 files changed

+44
-13
lines changed

2 files changed

+44
-13
lines changed

crates/ty_python_semantic/resources/mdtest/public_types.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,7 @@ else:
205205
reveal_type(A) # revealed: Literal[""] | None
206206

207207
def _():
208-
# TODO: this should be `str | None`
209-
reveal_type(A) # revealed: str
208+
reveal_type(A) # revealed: str | None
210209
```
211210

212211
This pattern appears frequently with conditional imports. Here, the import is treated as both a
@@ -221,8 +220,7 @@ except ImportError:
221220
reveal_type(some_library) # revealed: Unknown | None
222221

223222
def _():
224-
# TODO: this should be `Unknown | None`
225-
reveal_type(some_library) # revealed: Unknown
223+
reveal_type(some_library) # revealed: Unknown | None
226224
```
227225

228226
## Limitations

crates/ty_python_semantic/src/place.rs

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ fn place_by_id<'db>(
661661
qualifiers,
662662
}) => {
663663
let bindings = all_considered_bindings();
664+
let boundness_analysis = bindings.boundness_analysis;
664665
let inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
665666

666667
let place = match inferred {
@@ -674,7 +675,11 @@ fn place_by_id<'db>(
674675
// Place is possibly undeclared and (possibly) bound
675676
Place::Type(inferred_ty, boundness) => Place::Type(
676677
UnionType::from_elements(db, [inferred_ty, declared_ty]),
677-
boundness,
678+
if boundness_analysis == BoundnessAnalysis::AlwaysBound {
679+
Boundness::Bound
680+
} else {
681+
boundness
682+
},
678683
),
679684
};
680685

@@ -686,7 +691,14 @@ fn place_by_id<'db>(
686691
qualifiers: _,
687692
}) => {
688693
let bindings = all_considered_bindings();
689-
let inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
694+
let boundness_analysis = bindings.boundness_analysis;
695+
let mut inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport);
696+
697+
if boundness_analysis == BoundnessAnalysis::AlwaysBound {
698+
if let Place::Type(ty, Boundness::PossiblyUnbound) = inferred {
699+
inferred = Place::Type(ty, Boundness::Bound);
700+
}
701+
}
690702

691703
// `__slots__` is a symbol with special behavior in Python's runtime. It can be
692704
// modified externally, but those changes do not take effect. We therefore issue
@@ -828,6 +840,7 @@ fn place_from_bindings_impl<'db>(
828840
}) if binding.is_undefined_or(is_non_exported) => Some(*reachability_constraint),
829841
_ => None,
830842
};
843+
let mut all_bindings_definitely_reachable = true;
831844
let mut deleted_reachability = Truthiness::AlwaysFalse;
832845

833846
// Evaluate this lazily because we don't always need it (for example, if there are no visible
@@ -920,13 +933,27 @@ fn place_from_bindings_impl<'db>(
920933
}
921934

922935
let binding_ty = binding_type(db, binding);
936+
all_bindings_definitely_reachable =
937+
all_bindings_definitely_reachable && static_reachability.is_always_true();
923938
Some(narrowing_constraint.narrow(db, binding_ty, binding.place(db)))
924939
},
925940
);
926941

927942
if let Some(first) = types.next() {
943+
let ty = if let Some(second) = types.next() {
944+
UnionType::from_elements(db, [first, second].into_iter().chain(types))
945+
} else {
946+
first
947+
};
948+
928949
let boundness = match boundness_analysis {
929-
BoundnessAnalysis::AlwaysBound => Boundness::Bound,
950+
BoundnessAnalysis::AlwaysBound => {
951+
if all_bindings_definitely_reachable {
952+
Boundness::Bound
953+
} else {
954+
Boundness::PossiblyUnbound
955+
}
956+
}
930957
BoundnessAnalysis::BasedOnUnboundVisibility => match unbound_visibility() {
931958
Some(Truthiness::AlwaysTrue) => {
932959
unreachable!(
@@ -938,11 +965,6 @@ fn place_from_bindings_impl<'db>(
938965
},
939966
};
940967

941-
let ty = if let Some(second) = types.next() {
942-
UnionType::from_elements(db, [first, second].into_iter().chain(types))
943-
} else {
944-
first
945-
};
946968
match deleted_reachability {
947969
Truthiness::AlwaysFalse => Place::Type(ty, boundness),
948970
Truthiness::AlwaysTrue => Place::Unbound,
@@ -1066,6 +1088,8 @@ fn place_from_declarations_impl<'db>(
10661088
_ => Truthiness::AlwaysFalse,
10671089
};
10681090

1091+
let mut all_declarations_definitely_reachable = true;
1092+
10691093
let types = declarations.filter_map(
10701094
|DeclarationWithConstraint {
10711095
declaration,
@@ -1085,6 +1109,9 @@ fn place_from_declarations_impl<'db>(
10851109
if static_reachability.is_always_false() {
10861110
None
10871111
} else {
1112+
all_declarations_definitely_reachable =
1113+
all_declarations_definitely_reachable && static_reachability.is_always_true();
1114+
10881115
Some(declaration_type(db, declaration))
10891116
}
10901117
},
@@ -1104,7 +1131,13 @@ fn place_from_declarations_impl<'db>(
11041131
}
11051132

11061133
let boundness = match boundness_analysis {
1107-
BoundnessAnalysis::AlwaysBound => Boundness::Bound,
1134+
BoundnessAnalysis::AlwaysBound => {
1135+
if all_declarations_definitely_reachable {
1136+
Boundness::Bound
1137+
} else {
1138+
Boundness::PossiblyUnbound
1139+
}
1140+
}
11081141
BoundnessAnalysis::BasedOnUnboundVisibility => match undeclared_reachability {
11091142
Truthiness::AlwaysTrue => {
11101143
unreachable!(

0 commit comments

Comments
 (0)