[ty] disallow type variables within PEP-695 type variable bounds/constraints#22982
Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 82.56% to 82.61%. The percentage of expected errors that received a diagnostic increased from 73.94% to 74.21%. Summary
True positives addedDetails
|
|
76b2b5a to
8ba8c77
Compare
| any_over_type(db, self, &|ty| matches!(ty, Type::TypeVar(_)), false) | ||
| } | ||
|
|
||
| fn has_typevar_or_typevar_instance(self, db: &'db dyn Db) -> bool { |
There was a problem hiding this comment.
I think you could also use this new helper here:
ruff/crates/ty_python_semantic/src/types/infer/builder.rs
Lines 6523 to 6539 in 1d9c84d
carljm
left a comment
There was a problem hiding this comment.
With Alex's comment applied, this looks good to me!
There was a problem hiding this comment.
As well as #22982 (comment), I think you can do this check with much less code by integrating it into infer_typevar_deferred() rather than adding the new check_pep695_typevars() method. All your added tests pass with this diff applied:
diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs
index c4da5b72cb..ec0534d7e6 100644
--- a/crates/ty_python_semantic/src/types/infer/builder.rs
+++ b/crates/ty_python_semantic/src/types/infer/builder.rs
@@ -657,7 +657,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
self.check_static_class_definitions();
self.check_overloaded_functions(node);
self.check_type_guard_definitions();
- self.check_pep695_typevars();
self.check_legacy_positional_only_convention();
self.check_final_without_value();
}
@@ -2173,60 +2172,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
- fn check_pep695_typevars(&mut self) {
- let typevars = self.declarations.iter().filter_map(|(definition, ty)| {
- if let DefinitionKind::TypeVar(typevar) = definition.kind(self.db()) {
- Some((typevar.node(self.module()), ty.inner_type()))
- } else {
- None
- }
- });
-
- // The current Python type specification is that the bounds and constraints of type variables must be concrete types,
- // and an error must occur if type variables are included.
- for (node, ty) in typevars {
- let Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) = ty else {
- continue;
- };
- let Some(bound) = node.bound.as_deref() else {
- continue;
- };
- match typevar.bound_or_constraints_unchecked(self.db()) {
- Some(TypeVarBoundOrConstraints::UpperBound(bound_ty)) => {
- if bound_ty.has_typevar_or_typevar_instance(self.db())
- && let Some(builder) = self
- .context
- .report_lint(&INVALID_TYPE_VARIABLE_BOUND, bound)
- {
- builder.into_diagnostic("TypeVar upper bound cannot be generic");
- }
- }
- Some(TypeVarBoundOrConstraints::Constraints(constraints)) => {
- let ast::Expr::Tuple(tuple) = bound else {
- continue;
- };
- for constraint in constraints
- .elements(self.db())
- .iter()
- .enumerate()
- .filter_map(|(i, ty)| {
- ty.has_typevar_or_typevar_instance(self.db())
- .then_some(tuple.elts.get(i)?)
- })
- {
- if let Some(builder) = self
- .context
- .report_lint(&INVALID_TYPE_VARIABLE_CONSTRAINTS, constraint)
- {
- builder.into_diagnostic("TypeVar constraint cannot be generic");
- }
- }
- }
- None => {}
- }
- }
- }
-
fn infer_region_definition(&mut self, definition: Definition<'db>) {
match definition.kind(self.db()) {
DefinitionKind::Function(function) => {
@@ -4314,14 +4259,28 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
// Here, we interpret `bound` as a heterogeneous tuple and convert it to `TypeVarConstraints` in `TypeVarInstance::lazy_constraints`.
let tuple_ty = Type::heterogeneous_tuple(
self.db(),
- elts.iter()
- .map(|expr| self.infer_type_expression(expr))
- .collect::<Box<[_]>>(),
+ elts.iter().map(|expr| {
+ let constraint = self.infer_type_expression(expr);
+ if constraint.has_typevar_or_typevar_instance(self.db())
+ && let Some(builder) = self
+ .context
+ .report_lint(&INVALID_TYPE_VARIABLE_CONSTRAINTS, expr)
+ {
+ builder.into_diagnostic("TypeVar upper bound cannot be generic");
+ }
+ constraint
+ }),
);
self.store_expression_type(expr, tuple_ty);
}
Some(expr) => {
- self.infer_type_expression(expr);
+ let bound_ty = self.infer_type_expression(expr);
+ if bound_ty.has_typevar_or_typevar_instance(self.db())
+ && let Some(builder) =
+ self.context.report_lint(&INVALID_TYPE_VARIABLE_BOUND, expr)
+ {
+ builder.into_diagnostic("TypeVar upper bound cannot be generic");
+ }
}
None => {}
}ef7a6af to
8706ead
Compare
Memory usage reportMemory usage unchanged ✅ |
|
Thanks! |
8706ead to
fd2ff51
Compare
Summary
This is a revival of #21984.
Legacy typevar checks were already supported in #22949.
This PR addresses PEP-695 typevars.
Test Plan
mdtest updated