From 07ba2f4f22af6f439fc506355a0c2a09fa0266bf Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 19 Dec 2025 10:44:09 -0500 Subject: [PATCH] store unwidened --- crates/ty_python_semantic/src/place.rs | 69 +++++++++++++------ crates/ty_python_semantic/src/types.rs | 4 +- crates/ty_python_semantic/src/types/class.rs | 2 +- crates/ty_python_semantic/src/types/enums.rs | 28 -------- .../src/types/infer/builder.rs | 4 +- 5 files changed, 52 insertions(+), 55 deletions(-) diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index d6f79a1618..5749dc0e8e 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -76,6 +76,16 @@ pub(crate) enum Widening { WithUnknown, } +impl Widening { + /// Apply widening to the type if this is `WithUnknown`. + pub(crate) fn apply_if_needed<'db>(self, db: &'db dyn Db, ty: Type<'db>) -> Type<'db> { + match self { + Self::None => ty, + Self::WithUnknown => UnionType::from_elements(db, [Type::unknown(), ty]), + } + } +} + /// The result of a place lookup, which can either be a (possibly undefined) type /// or a completely undefined place. /// @@ -152,9 +162,6 @@ impl<'db> Place<'db> { /// /// If the place is *definitely* undefined, this function will return `None`. Otherwise, /// if there is at least one control-flow path where the place is defined, return the type. - /// - /// Note: This returns the raw type without applying widening. Use `widened_type()` if you - /// need the type with the Unknown union applied for undeclared public symbols. pub(crate) fn ignore_possibly_undefined(&self) -> Option> { match self { Place::Defined(ty, _, _, _) => Some(*ty), @@ -162,6 +169,17 @@ impl<'db> Place<'db> { } } + /// Returns the type of the place without widening applied. + /// + /// The stored type is always the unwidened type. Widening (union with `Unknown`) + /// is applied lazily when converting to `LookupResult`. + pub(crate) fn unwidened_type(&self) -> Option> { + match self { + Place::Defined(ty, _, _, _) => Some(*ty), + Place::Undefined => None, + } + } + #[cfg(test)] #[track_caller] pub(crate) fn expect_type(self) -> Type<'db> { @@ -266,7 +284,7 @@ impl<'db> LookupError<'db> { db: &'db dyn Db, fallback: PlaceAndQualifiers<'db>, ) -> LookupResult<'db> { - let fallback = fallback.into_lookup_result(); + let fallback = fallback.into_lookup_result(db); match (&self, &fallback) { (LookupError::Undefined(_), _) => fallback, (LookupError::PossiblyUndefined { .. }, Err(LookupError::Undefined(_))) => Err(self), @@ -687,18 +705,27 @@ impl<'db> PlaceAndQualifiers<'db> { /// Transform place and qualifiers into a [`LookupResult`], /// a [`Result`] type in which the `Ok` variant represents a definitely defined place /// and the `Err` variant represents a place that is either definitely or possibly undefined. - pub(crate) fn into_lookup_result(self) -> LookupResult<'db> { + /// + /// For places marked with `Widening::WithUnknown`, this applies the gradual typing guarantee + /// by creating a union with `Unknown`. + pub(crate) fn into_lookup_result(self, db: &'db dyn Db) -> LookupResult<'db> { match self { PlaceAndQualifiers { - place: Place::Defined(ty, origin, Definedness::AlwaysDefined, _), + place: Place::Defined(ty, origin, Definedness::AlwaysDefined, widening), qualifiers, - } => Ok(TypeAndQualifiers::new(ty, origin, qualifiers)), + } => { + let ty = widening.apply_if_needed(db, ty); + Ok(TypeAndQualifiers::new(ty, origin, qualifiers)) + } PlaceAndQualifiers { - place: Place::Defined(ty, origin, Definedness::PossiblyUndefined, _), + place: Place::Defined(ty, origin, Definedness::PossiblyUndefined, widening), qualifiers, - } => Err(LookupError::PossiblyUndefined(TypeAndQualifiers::new( - ty, origin, qualifiers, - ))), + } => { + let ty = widening.apply_if_needed(db, ty); + Err(LookupError::PossiblyUndefined(TypeAndQualifiers::new( + ty, origin, qualifiers, + ))) + } PlaceAndQualifiers { place: Place::Undefined, qualifiers, @@ -706,17 +733,18 @@ impl<'db> PlaceAndQualifiers<'db> { } } - /// Safely unwrap the place and the qualifiers into a [`TypeQualifiers`]. + /// Safely unwrap the place and the qualifiers into a [`TypeAndQualifiers`]. /// /// If the place is definitely unbound or possibly unbound, it will be transformed into a /// [`LookupError`] and `diagnostic_fn` will be applied to the error value before returning - /// the result of `diagnostic_fn` (which will be a [`TypeQualifiers`]). This allows the caller + /// the result of `diagnostic_fn` (which will be a [`TypeAndQualifiers`]). This allows the caller /// to ensure that a diagnostic is emitted if the place is possibly or definitely unbound. pub(crate) fn unwrap_with_diagnostic( self, + db: &'db dyn Db, diagnostic_fn: impl FnOnce(LookupError<'db>) -> TypeAndQualifiers<'db>, ) -> TypeAndQualifiers<'db> { - self.into_lookup_result().unwrap_or_else(diagnostic_fn) + self.into_lookup_result(db).unwrap_or_else(diagnostic_fn) } /// Fallback (partially or fully) to another place if `self` is partially or fully unbound. @@ -735,7 +763,7 @@ impl<'db> PlaceAndQualifiers<'db> { db: &'db dyn Db, fallback_fn: impl FnOnce() -> PlaceAndQualifiers<'db>, ) -> Self { - self.into_lookup_result() + self.into_lookup_result(db) .or_else(|lookup_error| lookup_error.or_fall_back_to(db, fallback_fn())) .into() } @@ -988,13 +1016,10 @@ pub(crate) fn place_by_id<'db>( { inferred.into() } else { - // Gradual typing guarantee: Union with `Unknown` for undeclared public symbols. - // This allows external code to assign any type to these symbols. - // We also mark the widening flag to identify this type as widened. - inferred - .map_type(|ty| UnionType::from_elements(db, [Type::unknown(), ty])) - .with_widening(Widening::WithUnknown) - .into() + // Gradual typing guarantee: Mark undeclared public symbols for widening. + // The actual union with `Unknown` is applied lazily when converting to + // LookupResult via `into_lookup_result`. + inferred.with_widening(Widening::WithUnknown).into() } } } diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 5ca2134da8..8f1507dc25 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -11780,7 +11780,7 @@ impl<'db> BoolError<'db> { ); if let Some((func_span, parameter_span)) = not_boolable_type .member(context.db(), "__bool__") - .into_lookup_result() + .into_lookup_result(context.db()) .ok() .and_then(|quals| quals.inner_type().parameter_span(context.db(), None)) { @@ -11808,7 +11808,7 @@ impl<'db> BoolError<'db> { ); if let Some((func_span, return_type_span)) = not_boolable_type .member(context.db(), "__bool__") - .into_lookup_result() + .into_lookup_result(context.db()) .ok() .and_then(|quals| quals.inner_type().function_spans(context.db())) .and_then(|spans| Some((spans.name, spans.return_type?))) diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index ff1ac73af9..3f32f9efce 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -2361,7 +2361,7 @@ impl<'db> ClassLiteral<'db> { // For enum classes, `nonmember(value)` creates a non-member attribute. // At runtime, the enum metaclass unwraps the value, so accessing the attribute // returns the inner value, not the `nonmember` wrapper. - if let Some(ty) = member.inner.place.ignore_possibly_undefined() { + if let Some(ty) = member.inner.place.unwidened_type() { if let Some(value_ty) = try_unwrap_nonmember_value(db, ty) { if is_enum_class_by_inheritance(db, self) { return Member::definitely_declared(value_ty); diff --git a/crates/ty_python_semantic/src/types/enums.rs b/crates/ty_python_semantic/src/types/enums.rs index 36e56e0c53..d6f5f23d28 100644 --- a/crates/ty_python_semantic/src/types/enums.rs +++ b/crates/ty_python_semantic/src/types/enums.rs @@ -319,34 +319,6 @@ pub(crate) fn try_unwrap_nonmember_value<'db>(db: &'db dyn Db, ty: Type<'db>) -> .unwrap_or(Type::unknown()), ) } - Type::Union(union) => { - // TODO: This is a hack. The proper fix is to avoid unioning Unknown from - // declarations into Place when we have concrete bindings. - // - // For now, we filter out Unknown and expect exactly one nonmember type - // to remain. If there are other non-Unknown types mixed in, we bail out. - let mut non_unknown = union.elements(db).iter().filter(|elem| !elem.is_unknown()); - - let first = non_unknown.next()?; - - // Ensure there's exactly one non-Unknown element. - if non_unknown.next().is_some() { - return None; - } - - if let Type::NominalInstance(instance) = first { - if instance.has_known_class(db, KnownClass::Nonmember) { - return Some( - first - .member(db, "value") - .place - .ignore_possibly_undefined() - .unwrap_or(Type::unknown()), - ); - } - } - None - } _ => None, } } diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index e7667e6c68..84502c669a 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -9029,7 +9029,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } let ty = - resolved_after_fallback.unwrap_with_diagnostic(|lookup_error| match lookup_error { + resolved_after_fallback.unwrap_with_diagnostic(db, |lookup_error| match lookup_error { LookupError::Undefined(qualifiers) => { self.report_unresolved_reference(name_node); TypeAndQualifiers::new(Type::unknown(), TypeOrigin::Inferred, qualifiers) @@ -9570,7 +9570,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { }); let attr_name = &attr.id; - let resolved_type = fallback_place.unwrap_with_diagnostic(|lookup_err| match lookup_err { + let resolved_type = fallback_place.unwrap_with_diagnostic(db, |lookup_err| match lookup_err { LookupError::Undefined(_) => { let fallback = || { TypeAndQualifiers::new(