mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-30 13:51:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] No boundness analysis for implicit instance attributes (#20128)

Browse source 
## Summary

With this PR, we stop performing boundness analysis for implicit
instance attributes:

```py
class C:
    def __init__(self):
        if False:   
            self.x = 1

C().x  # would previously show an error, with this PR we pretend the attribute exists
```

This PR is potentially just a temporary measure until we find a better
fix. But I have already invested a lot of time trying to find the root
cause of https://github.com/astral-sh/ty/issues/758 (and [this
example](https://github.com/astral-sh/ty/issues/758#issuecomment-3206108262),
which I'm not entirely sure is related) and I still don't understand
what is going on. This PR fixes the performance problems in both of
these problems (in a rather crude way).

The impact of the proposed change on the ecosystem is small, and the
three new diagnostics are arguably true positives (previously hidden
because we considered the code unreachable, based on e.g. `assert`ions
that depended on implicit instance attributes). So this seems like a
reasonable fix for now.

Note that we still support cases like these:

```py
class D:
    if False:  # or any other expression that statically evaluates to `False`
        x: int = 1

D().x  # still an error


class E:
    if False:  # or any other expression that statically evaluates to `False`
        def f(self):
            self.x = 1

E().x  # still an error
```

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

## Test Plan

Updated tests, benchmark results
This commit is contained in:
David Peter 2025-08-28 16:25:07 +02:00 • committed by GitHub
parent c2bc15bc15
commit b3c4005289
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 21 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -820,6 +820,8 @@ impl ReachabilityConstraints {
}
fn analyze_single(db: &dyn Db, predicate: &Predicate) -> Truthiness {
let _span = tracing::trace_span!("analyze_single", ?predicate).entered();
match predicate.node {
PredicateNode::Expression(test_expr) => {
static_expression_truthiness(db, test_expr).negate_if(!predicate.is_positive)

12
crates/ty_python_semantic/src/semantic_index/use_def.rs
View file

@ -598,18 +598,6 @@ impl<'db> UseDefMap<'db> {
.is_always_false()
}
pub(crate) fn declaration_reachability(
&self,
db: &dyn crate::Db,
declaration: &DeclarationWithConstraint<'db>,
) -> Truthiness {
self.reachability_constraints.evaluate(
db,
&self.predicates,
declaration.reachability_constraint,
)
}
pub(crate) fn binding_reachability(
&self,
db: &dyn crate::Db,

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

@ -6910,10 +6910,10 @@ bitflags! {
const NOT_REQUIRED = 1 << 4;
/// `typing_extensions.ReadOnly`
const READ_ONLY = 1 << 5;
/// An implicit instance attribute which is possibly unbound according
/// to local control flow within the method it is defined in. This flag
/// overrules the `Boundness` information on `PlaceAndQualifiers`.
const POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE = 1 << 6;
/// A non-standard type qualifier that marks implicit instance attributes, i.e.
/// instance attributes that are only implicitly defined via `self.x = …` in
/// the body of a class method.
const IMPLICIT_INSTANCE_ATTRIBUTE = 1 << 6;
}
}
@ -8663,14 +8663,6 @@ impl Truthiness {
if condition { self.negate() } else { self }
}
pub(crate) fn and(self, other: Self) -> Self {
match (self, other) {
(Truthiness::AlwaysTrue, Truthiness::AlwaysTrue) => Truthiness::AlwaysTrue,
(Truthiness::AlwaysFalse, _) | (_, Truthiness::AlwaysFalse) => Truthiness::AlwaysFalse,
_ => Truthiness::Ambiguous,
}
}
pub(crate) fn or(self, other: Self) -> Self {
match (self, other) {
(Truthiness::AlwaysFalse, Truthiness::AlwaysFalse) => Truthiness::AlwaysFalse,

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

@ -2686,7 +2686,7 @@ impl<'db> ClassLiteral<'db> {
// that attribute. We include `Unknown` in that union to account for the fact that the
// attribute might be externally modified.
let mut union_of_inferred_types = UnionBuilder::new(db);
let mut qualifiers = TypeQualifiers::empty();
let mut qualifiers = TypeQualifiers::IMPLICIT_INSTANCE_ATTRIBUTE;
let mut is_attribute_bound = false;
@ -2736,20 +2736,11 @@ impl<'db> ClassLiteral<'db> {
// self.name: <annotation>
// self.name: <annotation> = …
let reachability = use_def_map(db, method_scope)
.declaration_reachability(db, &attribute_declaration);
if reachability.is_always_false() {
continue;
}
let annotation = declaration_type(db, declaration);
let mut annotation =
Place::bound(annotation.inner).with_qualifiers(annotation.qualifiers);
let annotation = Place::bound(annotation.inner).with_qualifiers(
annotation.qualifiers | TypeQualifiers::IMPLICIT_INSTANCE_ATTRIBUTE,
);
if reachability.is_ambiguous() {
annotation.qualifiers |= TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE;
}
if let Some(all_qualifiers) = annotation.is_bare_final() {
if let Some(value) = assignment.value(&module) {
// If we see an annotated assignment with a bare `Final` as in
@ -2781,8 +2772,6 @@ impl<'db> ClassLiteral<'db> {
continue;
}
let method_map = use_def_map(db, method_scope);
// The attribute assignment inherits the reachability of the method which contains it
let is_method_reachable =
if let Some(method_def) = method_scope.node(db).as_function(&module) {
@ -2802,53 +2791,16 @@ impl<'db> ClassLiteral<'db> {
continue;
}
// Storage for the implicit `DefinitionState::Undefined` binding. If present, it
// will be the first binding in the `attribute_assignments` iterator.
let mut unbound_binding = None;
for attribute_assignment in attribute_assignments {
if let DefinitionState::Undefined = attribute_assignment.binding {
// Store the implicit unbound binding here so that we can delay the
// computation of `unbound_reachability` to the point when we actually
// need it. This is an optimization for the common case where the
// `unbound` binding is the only binding of the `name` attribute,
// i.e. if there is no `self.name = …` assignment in this method.
unbound_binding = Some(attribute_assignment);
continue;
}
let DefinitionState::Defined(binding) = attribute_assignment.binding else {
continue;
};
match method_map
.binding_reachability(db, &attribute_assignment)
.and(is_method_reachable)
{
Truthiness::AlwaysTrue => {
is_attribute_bound = true;
}
Truthiness::Ambiguous => {
is_attribute_bound = true;
qualifiers |= TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE;
}
Truthiness::AlwaysFalse => {
continue;
}
}
// There is at least one attribute assignment that may be reachable, so if `unbound_reachability` is
// always false then this attribute is considered bound.
// TODO: this is incomplete logic since the attributes bound after termination are considered reachable.
let unbound_reachability = unbound_binding
.as_ref()
.map(|binding| method_map.binding_reachability(db, binding))
.unwrap_or(Truthiness::AlwaysFalse);
if unbound_reachability
.negate()
.and(is_method_reachable)
.is_always_true()
{
if !is_method_reachable.is_always_false() {
is_attribute_bound = true;
}

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

@ -7199,10 +7199,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
let fallback_place = value_type.member(db, &attr.id);
// Exclude non-definitely-bound places for purposes of reachability
// analysis. We currently do not perform boundness analysis for implicit
// instance attributes, so we exclude them here as well.
if !fallback_place.place.is_definitely_bound()
|| fallback_place
.qualifiers
.contains(TypeQualifiers::POSSIBLY_UNBOUND_IMPLICIT_ATTRIBUTE)
.contains(TypeQualifiers::IMPLICIT_INSTANCE_ATTRIBUTE)
{
self.all_definitely_bound = false;
}