Skip to content

Commit

Permalink
Fix broken red-knot property tests (#16574)
Browse files Browse the repository at this point in the history
## Summary

Fixes #16566, fixes #16575

The semantics of `Type::class_member` changed in
#16416, but the property-test
infrastructure was not updated. That means that the property tests were
panicking on the second `expect_type` call here:


https://github.com/astral-sh/ruff/blob/036102186392f1263774bfd42d78acc65c0641b8/crates/red_knot_python_semantic/src/types/property_tests.rs#L151-L158

With the somewhat unhelpful message:

```
Expected a (possibly unbound) type, not an unbound symbol
```

Applying this patch, and then running `QUICKCHECK_TESTS=1000000 cargo
test --release -p red_knot_python_semantic -- --ignored
types::property_tests::stable::equivalent_to_is_reflexive` showed
clearly that it was no longer able to find _any_ methods on _any_
classes due to the change in semantics of `Type::class_member`:

```diff
--- a/crates/red_knot_python_semantic/src/types/property_tests.rs
+++ b/crates/red_knot_python_semantic/src/types/property_tests.rs
@@ -27,7 +27,7 @@
 use std::sync::{Arc, Mutex, MutexGuard, OnceLock};
 
 use crate::db::tests::{setup_db, TestDb};
-use crate::symbol::{builtins_symbol, known_module_symbol};
+use crate::symbol::{builtins_symbol, known_module_symbol, Symbol};
 use crate::types::{
     BoundMethodType, CallableType, IntersectionBuilder, KnownClass, KnownInstanceType,
     SubclassOfType, TupleType, Type, UnionType,
@@ -150,10 +150,11 @@ impl Ty {
             Ty::BuiltinsFunction(name) => builtins_symbol(db, name).symbol.expect_type(),
             Ty::BuiltinsBoundMethod { class, method } => {
                 let builtins_class = builtins_symbol(db, class).symbol.expect_type();
-                let function = builtins_class
-                    .class_member(db, method.into())
-                    .symbol
-                    .expect_type();
+                let Symbol::Type(function, ..) =
+                    builtins_class.class_member(db, method.into()).symbol
+                else {
+                    panic!("no method `{method}` on class `{class}`");
+                };
 
                 create_bound_method(db, function, builtins_class)
             }
```

This PR updates the property-test infrastructure to use `Type::member`
rather than `Type::class_member`.

## Test Plan

- Ran `QUICKCHECK_TESTS=1000000 cargo test --release -p
red_knot_python_semantic -- --ignored types::property_tests::stable`
successfully
- Checked that there were no remaining uses of `Type::class_member` in
`property_tests.rs`
  • Loading branch information
AlexWaygood authored Mar 9, 2025
1 parent 335b264 commit c970b79
Showing 1 changed file with 1 addition and 4 deletions.
5 changes: 1 addition & 4 deletions crates/red_knot_python_semantic/src/types/property_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,7 @@ impl Ty {
Ty::BuiltinsFunction(name) => builtins_symbol(db, name).symbol.expect_type(),
Ty::BuiltinsBoundMethod { class, method } => {
let builtins_class = builtins_symbol(db, class).symbol.expect_type();
let function = builtins_class
.class_member(db, method.into())
.symbol
.expect_type();
let function = builtins_class.member(db, method).symbol.expect_type();

create_bound_method(db, function, builtins_class)
}
Expand Down

0 comments on commit c970b79

Please sign in to comment.