mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-31 07:37:38 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

[ty] Use all reachable bindings for instance attributes and deferred lookups (#18955)

Browse source 
## Summary

Remove a hack in control flow modeling that was treating `return`
statements at the end of function bodies in a special way (basically
considering the state *just before* the `return` statement as the
end-of-scope state). This is not needed anymore now that #18750 has been
merged.

In order to make this work, we now use *all reachable bindings* for
purposes of finding implicit instance attribute assignments as well as
for deferred lookups of symbols. Both would otherwise be affected by
this change:
```py
def C:
    def f(self):
        self.x = 1  # a reachable binding that is not visible at the end of the scope
        return
```

```py
def f():
    class X: ...  # a reachable binding that is not visible at the end of the scope
    x: "X" = X()  # deferred use of `X`
    return
```

Implicit instance attributes also required another change. We previously
kept track of possibly-unbound instance attributes in some cases, but we
now give up on that completely and always consider *implicit* instance
attributes to be bound if we see a reachable binding in a reachable
method. The previous behavior was somewhat inconsistent anyway because
we also do not consider attributes possibly-unbound in other scenarios:
we do not (and can not) keep track of whether or not methods are called
that define these attributes.

closes https://github.com/astral-sh/ty/issues/711

## Ecosystem analysis

I think this looks very positive!

* We see an unsurprising drop in `possibly-unbound-attribute`
diagnostics (599), mostly for classes that define attributes in `try …
except` blocks, `for` loops, or `if … else: raise …` constructs. There
might obviously also be true positives that got removed, but the vast
majority should be false positives.
* There is also a drop in `possibly-unresolved-reference` /
`unresolved-reference` diagnostics (279+13) from the change to deferred
lookups.
* Some `invalid-type-form` false positives got resolved (13), because we
can now properly look up the names in the annotations.
* There are some new *true* positives in `attrs`, since we understand
the `Attribute` annotation that was previously inferred as `Unknown`
because of a re-assignment after the class definition.


## Test Plan

The existing attributes.md test suite has sufficient coverage here.
This commit is contained in:
David Peter 2025-07-01 14:38:36 +02:00 • committed by GitHub
parent ebf59e2bef
commit dac4e356eb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 30 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ty_python_semantic/src/place.rs
View file

@ -62,10 +62,6 @@ impl<'db> Place<'db> {
Place::Type(ty.into(), Boundness::Bound)
}
pub(crate) fn possibly_unbound(ty: impl Into<Type<'db>>) -> Self {
Place::Type(ty.into(), Boundness::PossiblyUnbound)
}
/// Constructor that creates a [`Place`] with a [`crate::types::TodoType`] type
/// and boundness [`Boundness::Bound`].
#[allow(unused_variables)] // Only unused in release builds

2
crates/ty_python_semantic/src/semantic_index.rs
View file

@ -118,7 +118,7 @@ pub(crate) fn attribute_assignments<'db, 's>(
let place = place_table.place_id_by_instance_attribute_name(name)?;
let use_def = &index.use_def_maps[function_scope_id];
Some((
use_def.inner.end_of_scope_bindings(place),
use_def.inner.all_reachable_bindings(place),
function_scope_id,
))
})

24
crates/ty_python_semantic/src/semantic_index/builder.rs
View file

@ -1122,29 +1122,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
&mut first_parameter_name,
);
// TODO: Fix how we determine the public types of symbols in a
// function-like scope: https://github.com/astral-sh/ruff/issues/15777
//
// In the meantime, visit the function body, but treat the last statement
// specially if it is a return. If it is, this would cause all definitions
// in the function to be marked as non-visible with our current treatment
// of terminal statements. Since we currently model the externally visible
// definitions in a function scope as the set of bindings that are visible
// at the end of the body, we then consider this function to have no
// externally visible definitions. To get around this, we take a flow
// snapshot just before processing the return statement, and use _that_ as
// the "end-of-body" state that we resolve external references against.
if let Some((last_stmt, first_stmts)) = body.split_last() {
builder.visit_body(first_stmts);
let pre_return_state = matches!(last_stmt, ast::Stmt::Return(_))
.then(|| builder.flow_snapshot());
builder.visit_stmt(last_stmt);
let reachability = builder.current_use_def_map().reachability;
if let Some(pre_return_state) = pre_return_state {
builder.flow_restore(pre_return_state);
builder.current_use_def_map_mut().reachability = reachability;
}
}
builder.visit_body(body);
builder.current_first_parameter_name = first_parameter_name;
builder.pop_scope()

38
crates/ty_python_semantic/src/types/class.rs
View file

@ -1740,7 +1740,7 @@ impl<'db> ClassLiteral<'db> {
// attribute might be externally modified.
let mut union_of_inferred_types = UnionBuilder::new(db).add(Type::unknown());
let mut is_attribute_bound = Truthiness::AlwaysFalse;
let mut is_attribute_bound = false;
let file = class_body_scope.file(db);
let module = parsed_module(db, file).load(db);
@ -1771,7 +1771,7 @@ impl<'db> ClassLiteral<'db> {
let method = index.expect_single_definition(method_def);
let method_place = class_table.place_id_by_name(&method_def.name).unwrap();
class_map
.end_of_scope_bindings(method_place)
.all_reachable_bindings(method_place)
.find_map(|bind| {
(bind.binding.is_defined_and(|def| def == method))
.then(|| class_map.is_binding_reachable(db, &bind))
@ -1806,13 +1806,8 @@ impl<'db> ClassLiteral<'db> {
.is_binding_reachable(db, &attribute_assignment)
.and(is_method_reachable)
{
Truthiness::AlwaysTrue => {
is_attribute_bound = Truthiness::AlwaysTrue;
}
Truthiness::Ambiguous => {
if is_attribute_bound.is_always_false() {
is_attribute_bound = Truthiness::Ambiguous;
}
Truthiness::AlwaysTrue | Truthiness::Ambiguous => {
is_attribute_bound = true;
}
Truthiness::AlwaysFalse => {
continue;
@ -1832,7 +1827,7 @@ impl<'db> ClassLiteral<'db> {
.and(is_method_reachable)
.is_always_true()
{
is_attribute_bound = Truthiness::AlwaysTrue;
is_attribute_bound = true;
}
match binding.kind(db) {
@ -1849,17 +1844,12 @@ impl<'db> ClassLiteral<'db> {
);
// TODO: check if there are conflicting declarations
match is_attribute_bound {
Truthiness::AlwaysTrue => {
return Place::bound(annotation_ty);
}
Truthiness::Ambiguous => {
return Place::possibly_unbound(annotation_ty);
}
Truthiness::AlwaysFalse => unreachable!(
"If the attribute assignments are all invisible, inference of their types should be skipped"
),
if is_attribute_bound {
return Place::bound(annotation_ty);
}
unreachable!(
"If the attribute assignments are all invisible, inference of their types should be skipped"
);
}
DefinitionKind::Assignment(assign) => {
match assign.target_kind() {
@ -1995,10 +1985,10 @@ impl<'db> ClassLiteral<'db> {
}
}
match is_attribute_bound {
Truthiness::AlwaysTrue => Place::bound(union_of_inferred_types.build()),
Truthiness::Ambiguous => Place::possibly_unbound(union_of_inferred_types.build()),
Truthiness::AlwaysFalse => Place::Unbound,
if is_attribute_bound {
Place::bound(union_of_inferred_types.build())
} else {
Place::Unbound
}
}

2
crates/ty_python_semantic/src/types/infer.rs
View file

@ -5665,7 +5665,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
// If we're inferring types of deferred expressions, always treat them as public symbols
if self.is_deferred() {
let place = if let Some(place_id) = place_table.place_id_by_expr(expr) {
place_from_bindings(db, use_def.end_of_scope_bindings(place_id))
place_from_bindings(db, use_def.all_reachable_bindings(place_id))
} else {
assert!(
self.deferred_state.in_string_annotation(),

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

@ -1650,8 +1650,8 @@ mod tests {
panic!("expected one positional-or-keyword parameter");
};
assert_eq!(name, "a");
// Parameter resolution deferred; we should see B
assert_eq!(annotated_type.unwrap().display(&db).to_string(), "B");
// Parameter resolution deferred:
assert_eq!(annotated_type.unwrap().display(&db).to_string(), "A | B");
}
#[test]