language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-28 02:39:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Fix false-positive diagnostics on super() calls (#20814)

Browse source
This commit is contained in:
Alex Waygood 2025-10-13 11:57:46 +01:00 • committed by GitHub
parent 565dbf3c9d
commit d83d7a0dcd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 975 additions and 123 deletions
Download patch file Download diff file Expand all files Collapse all files

384
crates/ty_python_semantic/src/types.rs
View file

@ -1020,6 +1020,13 @@ impl<'db> Type<'db> {
.expect("Expected a Type::Dynamic variant")
}
pub(crate) const fn into_protocol_instance(self) -> Option<ProtocolInstanceType<'db>> {
match self {
Type::ProtocolInstance(instance) => Some(instance),
_ => None,
}
}
#[track_caller]
pub(crate) fn expect_class_literal(self) -> ClassLiteral<'db> {
self.into_class_literal()
@ -11248,21 +11255,62 @@ impl<'db> EnumLiteralType<'db> {
}
}
/// Enumeration of ways in which a `super()` call can cause us to emit a diagnostic.
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum BoundSuperError<'db> {
InvalidPivotClassType {
/// The second argument to `super()` (which may have been implicitly provided by
/// the Python interpreter) has an abstract or structural type.
/// It's impossible to determine whether a `Callable` type or a synthesized protocol
/// type is an instance or subclass of the pivot class, so these are rejected.
AbstractOwnerType {
owner_type: Type<'db>,
pivot_class: Type<'db>,
/// If `owner_type` is a type variable, this contains the type variable instance
typevar_context: Option<TypeVarInstance<'db>>,
},
/// The first argument to `super()` (which may have been implicitly provided by
/// the Python interpreter) is not a valid class type.
InvalidPivotClassType { pivot_class: Type<'db> },
/// The second argument to `super()` was not a subclass or instance of the first argument.
/// (Note that both arguments may have been implicitly provided by the Python interpreter.)
FailingConditionCheck {
pivot_class: Type<'db>,
owner: Type<'db>,
/// If `owner_type` is a type variable, this contains the type variable instance
typevar_context: Option<TypeVarInstance<'db>>,
},
/// It was a single-argument `super()` call, but we were unable to determine
/// the implicit arguments provided by the Python interpreter.
UnavailableImplicitArguments,
}
impl BoundSuperError<'_> {
pub(super) fn report_diagnostic(&self, context: &InferContext, node: AnyNodeRef) {
impl<'db> BoundSuperError<'db> {
pub(super) fn report_diagnostic(&self, context: &'db InferContext<'db, '_>, node: AnyNodeRef) {
match self {
BoundSuperError::AbstractOwnerType {
owner_type,
pivot_class,
typevar_context,
} => {
if let Some(builder) = context.report_lint(&INVALID_SUPER_ARGUMENT, node) {
if let Some(typevar_context) = typevar_context {
let mut diagnostic = builder.into_diagnostic(format_args!(
"`{owner}` is a type variable with an abstract/structural type as \
its bounds or constraints, in `super({pivot_class}, {owner})` call",
pivot_class = pivot_class.display(context.db()),
owner = owner_type.display(context.db()),
));
Self::describe_typevar(context.db(), &mut diagnostic, *typevar_context);
} else {
builder.into_diagnostic(format_args!(
"`{owner}` is an abstract/structural type in \
`super({pivot_class}, {owner})` call",
pivot_class = pivot_class.display(context.db()),
owner = owner_type.display(context.db()),
));
}
}
}
BoundSuperError::InvalidPivotClassType { pivot_class } => {
if let Some(builder) = context.report_lint(&INVALID_SUPER_ARGUMENT, node) {
builder.into_diagnostic(format_args!(
@ -11271,14 +11319,28 @@ impl BoundSuperError<'_> {
));
}
}
BoundSuperError::FailingConditionCheck { pivot_class, owner } => {
BoundSuperError::FailingConditionCheck {
pivot_class,
owner,
typevar_context,
} => {
if let Some(builder) = context.report_lint(&INVALID_SUPER_ARGUMENT, node) {
builder.into_diagnostic(format_args!(
let mut diagnostic = builder.into_diagnostic(format_args!(
"`{owner}` is not an instance or subclass of \
`{pivot_class}` in `super({pivot_class}, {owner})` call",
`{pivot_class}` in `super({pivot_class}, {owner})` call",
pivot_class = pivot_class.display(context.db()),
owner = owner.display(context.db()),
));
if let Some(typevar_context) = typevar_context {
let bound_or_constraints_union =
Self::describe_typevar(context.db(), &mut diagnostic, *typevar_context);
diagnostic.info(format_args!(
"`{bounds_or_constraints}` is not an instance or subclass of `{pivot_class}`",
bounds_or_constraints =
bound_or_constraints_union.display(context.db()),
pivot_class = pivot_class.display(context.db()),
));
}
}
}
BoundSuperError::UnavailableImplicitArguments => {
@ -11292,6 +11354,44 @@ impl BoundSuperError<'_> {
}
}
}
/// Add an `info`-level diagnostic describing the bounds or constraints,
/// and return the type variable's upper bound or the union of its constraints.
fn describe_typevar(
db: &'db dyn Db,
diagnostic: &mut Diagnostic,
type_var: TypeVarInstance<'db>,
) -> Type<'db> {
match type_var.bound_or_constraints(db) {
Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
diagnostic.info(format_args!(
"Type variable `{}` has upper bound `{}`",
type_var.name(db),
bound.display(db)
));
bound
}
Some(TypeVarBoundOrConstraints::Constraints(constraints)) => {
diagnostic.info(format_args!(
"Type variable `{}` has constraints `{}`",
type_var.name(db),
constraints
.elements(db)
.iter()
.map(|c| c.display(db))
.join(", ")
));
Type::Union(constraints)
}
None => {
diagnostic.info(format_args!(
"Type variable `{}` has `object` as its implicit upper bound",
type_var.name(db)
));
Type::object()
}
}
}
}
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, get_size2::GetSize)]
@ -11326,14 +11426,6 @@ impl<'db> SuperOwnerKind<'db> {
}
}
fn into_type(self) -> Type<'db> {
match self {
SuperOwnerKind::Dynamic(dynamic) => Type::Dynamic(dynamic),
SuperOwnerKind::Class(class) => class.into(),
SuperOwnerKind::Instance(instance) => instance.into(),
}
}
fn into_class(self, db: &'db dyn Db) -> Option<ClassType<'db>> {
match self {
SuperOwnerKind::Dynamic(_) => None,
@ -11341,35 +11433,6 @@ impl<'db> SuperOwnerKind<'db> {
SuperOwnerKind::Instance(instance) => Some(instance.class(db)),
}
}
fn try_from_type(db: &'db dyn Db, ty: Type<'db>) -> Option<Self> {
match ty {
Type::Dynamic(dynamic) => Some(SuperOwnerKind::Dynamic(dynamic)),
Type::ClassLiteral(class_literal) => Some(SuperOwnerKind::Class(
class_literal.apply_optional_specialization(db, None),
)),
Type::NominalInstance(instance) => Some(SuperOwnerKind::Instance(instance)),
Type::BooleanLiteral(_) => {
SuperOwnerKind::try_from_type(db, KnownClass::Bool.to_instance(db))
}
Type::IntLiteral(_) => {
SuperOwnerKind::try_from_type(db, KnownClass::Int.to_instance(db))
}
Type::StringLiteral(_) => {
SuperOwnerKind::try_from_type(db, KnownClass::Str.to_instance(db))
}
Type::LiteralString => {
SuperOwnerKind::try_from_type(db, KnownClass::Str.to_instance(db))
}
Type::BytesLiteral(_) => {
SuperOwnerKind::try_from_type(db, KnownClass::Bytes.to_instance(db))
}
Type::SpecialForm(special_form) => {
SuperOwnerKind::try_from_type(db, special_form.instance_fallback(db))
}
_ => None,
}
}
}
impl<'db> From<SuperOwnerKind<'db>> for Type<'db> {
@ -11397,8 +11460,8 @@ fn walk_bound_super_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
bound_super: BoundSuperType<'db>,
visitor: &V,
) {
visitor.visit_type(db, bound_super.pivot_class(db).into());
visitor.visit_type(db, bound_super.owner(db).into_type());
visitor.visit_type(db, Type::from(bound_super.pivot_class(db)));
visitor.visit_type(db, Type::from(bound_super.owner(db)));
}
impl<'db> BoundSuperType<'db> {
@ -11414,16 +11477,171 @@ impl<'db> BoundSuperType<'db> {
pivot_class_type: Type<'db>,
owner_type: Type<'db>,
) -> Result<Type<'db>, BoundSuperError<'db>> {
if let Type::Union(union) = owner_type {
return Ok(UnionType::from_elements(
db,
union
let delegate_to =
|type_to_delegate_to| BoundSuperType::build(db, pivot_class_type, type_to_delegate_to);
let delegate_with_error_mapped =
|type_to_delegate_to, error_context: Option<TypeVarInstance<'db>>| {
delegate_to(type_to_delegate_to).map_err(|err| match err {
BoundSuperError::AbstractOwnerType {
owner_type: _,
pivot_class: _,
typevar_context: _,
} => BoundSuperError::AbstractOwnerType {
owner_type,
pivot_class: pivot_class_type,
typevar_context: error_context,
},
BoundSuperError::FailingConditionCheck {
pivot_class,
owner: _,
typevar_context: _,
} => BoundSuperError::FailingConditionCheck {
pivot_class,
owner: owner_type,
typevar_context: error_context,
},
BoundSuperError::InvalidPivotClassType { pivot_class } => {
BoundSuperError::InvalidPivotClassType { pivot_class }
}
BoundSuperError::UnavailableImplicitArguments => {
BoundSuperError::UnavailableImplicitArguments
}
})
};
let owner = match owner_type {
Type::Never => SuperOwnerKind::Dynamic(DynamicType::Unknown),
Type::Dynamic(dynamic) => SuperOwnerKind::Dynamic(dynamic),
Type::ClassLiteral(class) => SuperOwnerKind::Class(ClassType::NonGeneric(class)),
Type::SubclassOf(subclass_of_type) => match subclass_of_type.subclass_of() {
SubclassOfInner::Class(class) => SuperOwnerKind::Class(class),
SubclassOfInner::Dynamic(dynamic) => SuperOwnerKind::Dynamic(dynamic),
},
Type::NominalInstance(instance) => SuperOwnerKind::Instance(instance),
Type::ProtocolInstance(protocol) => {
if let Some(nominal_instance) = protocol.as_nominal_type() {
SuperOwnerKind::Instance(nominal_instance)
} else {
return Err(BoundSuperError::AbstractOwnerType {
owner_type,
pivot_class: pivot_class_type,
typevar_context: None,
});
}
}
Type::Union(union) => {
return Ok(union
.elements(db)
.iter()
.map(|ty| BoundSuperType::build(db, pivot_class_type, *ty))
.collect::<Result<Vec<_>, _>>()?,
));
}
.try_fold(UnionBuilder::new(db), |builder, element| {
delegate_to(*element).map(|ty| builder.add(ty))
})?
.build());
}
Type::Intersection(intersection) => {
let mut builder = IntersectionBuilder::new(db);
let mut one_good_element_found = false;
for positive in intersection.positive(db) {
if let Ok(good_element) = delegate_to(*positive) {
one_good_element_found = true;
builder = builder.add_positive(good_element);
}
}
if !one_good_element_found {
return Err(BoundSuperError::AbstractOwnerType {
owner_type,
pivot_class: pivot_class_type,
typevar_context: None,
});
}
for negative in intersection.negative(db) {
if let Ok(good_element) = delegate_to(*negative) {
builder = builder.add_negative(good_element);
}
}
return Ok(builder.build());
}
Type::TypeAlias(alias) => {
return delegate_with_error_mapped(alias.value_type(db), None);
}
Type::TypeVar(type_var) | Type::NonInferableTypeVar(type_var) => {
let type_var = type_var.typevar(db);
return match type_var.bound_or_constraints(db) {
Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
delegate_with_error_mapped(bound, Some(type_var))
}
Some(TypeVarBoundOrConstraints::Constraints(constraints)) => {
delegate_with_error_mapped(Type::Union(constraints), Some(type_var))
}
None => delegate_with_error_mapped(Type::object(), Some(type_var)),
};
}
Type::BooleanLiteral(_) | Type::TypeIs(_) => {
return delegate_to(KnownClass::Bool.to_instance(db));
}
Type::IntLiteral(_) => return delegate_to(KnownClass::Int.to_instance(db)),
Type::StringLiteral(_) | Type::LiteralString => {
return delegate_to(KnownClass::Str.to_instance(db));
}
Type::BytesLiteral(_) => {
return delegate_to(KnownClass::Bytes.to_instance(db));
}
Type::SpecialForm(special_form) => {
return delegate_to(special_form.instance_fallback(db));
}
Type::KnownInstance(instance) => {
return delegate_to(instance.instance_fallback(db));
}
Type::FunctionLiteral(_) | Type::DataclassDecorator(_) => {
return delegate_to(KnownClass::FunctionType.to_instance(db));
}
Type::WrapperDescriptor(_) => {
return delegate_to(KnownClass::WrapperDescriptorType.to_instance(db));
}
Type::KnownBoundMethod(method) => {
return delegate_to(method.class().to_instance(db));
}
Type::BoundMethod(_) => return delegate_to(KnownClass::MethodType.to_instance(db)),
Type::ModuleLiteral(_) => {
return delegate_to(KnownClass::ModuleType.to_instance(db));
}
Type::GenericAlias(_) => return delegate_to(KnownClass::GenericAlias.to_instance(db)),
Type::PropertyInstance(_) => return delegate_to(KnownClass::Property.to_instance(db)),
Type::EnumLiteral(enum_literal_type) => {
return delegate_to(enum_literal_type.enum_class_instance(db));
}
Type::BoundSuper(_) => return delegate_to(KnownClass::Super.to_instance(db)),
Type::TypedDict(td) => {
// In general it isn't sound to upcast a `TypedDict` to a `dict`,
// but here it seems like it's probably sound?
let mut key_builder = UnionBuilder::new(db);
let mut value_builder = UnionBuilder::new(db);
for (name, field) in td.items(db) {
key_builder = key_builder.add(Type::string_literal(db, &name));
value_builder = value_builder.add(field.declared_ty);
}
return delegate_to(
KnownClass::Dict
.to_specialized_instance(db, [key_builder.build(), value_builder.build()]),
);
}
Type::Callable(callable) if callable.is_function_like(db) => {
return delegate_to(KnownClass::FunctionType.to_instance(db));
}
Type::AlwaysFalsy
| Type::AlwaysTruthy
| Type::Callable(_)
| Type::DataclassTransformer(_) => {
return Err(BoundSuperError::AbstractOwnerType {
owner_type,
pivot_class: pivot_class_type,
typevar_context: None,
});
}
};
// We don't use `Classbase::try_from_type` here because:
// - There are objects that may validly be present in a class's bases list
@ -11452,24 +11670,22 @@ impl<'db> BoundSuperType<'db> {
}
};
let owner = SuperOwnerKind::try_from_type(db, owner_type)
.and_then(|owner| {
let Some(pivot_class) = pivot_class.into_class() else {
return Some(owner);
};
let Some(owner_class) = owner.into_class(db) else {
return Some(owner);
};
if owner_class.is_subclass_of(db, pivot_class) {
Some(owner)
} else {
None
}
})
.ok_or(BoundSuperError::FailingConditionCheck {
pivot_class: pivot_class_type,
owner: owner_type,
})?;
if let Some(pivot_class) = pivot_class.into_class()
&& let Some(owner_class) = owner.into_class(db)
{
let pivot_class = pivot_class.class_literal(db).0;
if !owner_class.iter_mro(db).any(|superclass| match superclass {
ClassBase::Dynamic(_) => true,
ClassBase::Generic | ClassBase::Protocol | ClassBase::TypedDict => false,
ClassBase::Class(superclass) => superclass.class_literal(db).0 == pivot_class,
}) {
return Err(BoundSuperError::FailingConditionCheck {
pivot_class: pivot_class_type,
owner: owner_type,
typevar_context: None,
});
}
}
Ok(Type::BoundSuper(BoundSuperType::new(
db,
@ -11525,19 +11741,22 @@ impl<'db> BoundSuperType<'db> {
db,
attribute,
Type::none(db),
owner.into_type(),
Type::from(owner),
)
.0,
),
SuperOwnerKind::Instance(_) => Some(
Type::try_call_dunder_get_on_attribute(
db,
attribute,
owner.into_type(),
owner.into_type().to_meta_type(db),
SuperOwnerKind::Instance(_) => {
let owner = Type::from(owner);
Some(
Type::try_call_dunder_get_on_attribute(
db,
attribute,
owner,
owner.to_meta_type(db),
)
.0,
)
.0,
),
}
}
}
@ -11551,9 +11770,8 @@ impl<'db> BoundSuperType<'db> {
) -> PlaceAndQualifiers<'db> {
let owner = self.owner(db);
let class = match owner {
SuperOwnerKind::Dynamic(_) => {
return owner
.into_type()
SuperOwnerKind::Dynamic(dynamic) => {
return Type::Dynamic(dynamic)
.find_name_in_mro_with_policy(db, name, policy)
.expect("Calling `find_name_in_mro` on dynamic type should return `Some`");
}

4
crates/ty_python_semantic/src/types/display.rs
View file

@ -571,9 +571,7 @@ impl Display for DisplayRepresentation<'_> {
"<super: {pivot}, {owner}>",
pivot = Type::from(bound_super.pivot_class(self.db))
.display_with(self.db, self.settings.singleline()),
owner = bound_super
.owner(self.db)
.into_type()
owner = Type::from(bound_super.owner(self.db))
.display_with(self.db, self.settings.singleline())
)
}

58
crates/ty_python_semantic/src/types/instance.rs
View file

@ -156,31 +156,27 @@ impl<'db> Type<'db> {
// This matches the behaviour of other type checkers, and is required for us to
// recognise `str` as a subtype of `Container[str]`.
structurally_satisfied.or(db, || {
if let Protocol::FromClass(class) = protocol.inner {
// if `self` and `other` are *both* protocols, we also need to treat `self` as if it
// were a nominal type, or we won't consider a protocol `P` that explicitly inherits
// from a protocol `Q` to be a subtype of `Q` to be a subtype of `Q` if it overrides
// `Q`'s members in a Liskov-incompatible way.
let type_to_test = if let Type::ProtocolInstance(ProtocolInstanceType {
inner: Protocol::FromClass(class),
..
}) = self
{
Type::non_tuple_instance(db, class)
} else {
self
};
let Some(nominal_instance) = protocol.as_nominal_type() else {
return ConstraintSet::from(false);
};
type_to_test.has_relation_to_impl(
db,
Type::non_tuple_instance(db, class),
relation,
relation_visitor,
disjointness_visitor,
)
} else {
ConstraintSet::from(false)
}
// if `self` and `other` are *both* protocols, we also need to treat `self` as if it
// were a nominal type, or we won't consider a protocol `P` that explicitly inherits
// from a protocol `Q` to be a subtype of `Q` to be a subtype of `Q` if it overrides
// `Q`'s members in a Liskov-incompatible way.
let type_to_test = self
.into_protocol_instance()
.and_then(ProtocolInstanceType::as_nominal_type)
.map(Type::NominalInstance)
.unwrap_or(self);
type_to_test.has_relation_to_impl(
db,
Type::NominalInstance(nominal_instance),
relation,
relation_visitor,
disjointness_visitor,
)
})
}
}
@ -605,6 +601,20 @@ impl<'db> ProtocolInstanceType<'db> {
}
}
/// If this is a class-based protocol, convert the protocol-instance into a nominal instance.
///
/// If this is a synthesized protocol that does not correspond to a class definition
/// in source code, return `None`. These are "pure" abstract types, that cannot be
/// treated in a nominal way.
pub(super) fn as_nominal_type(self) -> Option<NominalInstanceType<'db>> {
match self.inner {
Protocol::FromClass(class) => {
Some(NominalInstanceType(NominalInstanceInner::NonTuple(class)))
}
Protocol::Synthesized(_) => None,
}
}
/// Return the meta-type of this protocol-instance type.
pub(super) fn to_meta_type(self, db: &'db dyn Db) -> Type<'db> {
match self.inner {