[ty] No boundness analysis for implicit instance attributes

Author: sharkdp

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -571,8 +571,8 @@ class C:
         if (2 + 3) < 4:
             self.x: str = "a"
 
-# error: [unresolved-attribute]
-reveal_type(C().x)  # revealed: Unknown
+# TODO: this would ideally raise an `unresolved-attribute` error
+reveal_type(C().x)  # revealed: str
 ```
 
 ```py
@@ -600,9 +600,10 @@ class C:
         def set_e(self, e: str) -> None:
             self.e = e
 
-reveal_type(C(True).a)  # revealed: Unknown | Literal[1]
-# error: [unresolved-attribute]
-reveal_type(C(True).b)  # revealed: Unknown
+# TODO: this would ideally be `Unknown | Literal[1]`
+reveal_type(C(True).a)  # revealed: Unknown | Literal[1, "a"]
+# TODO: this would ideally raise an `unresolved-attribute` error
+reveal_type(C(True).b)  # revealed: Unknown | Literal[2]
 reveal_type(C(True).c)  # revealed: Unknown | Literal[3] | str
 # Ideally, this would just be `Unknown | Literal[5]`, but we currently do not
 # attempt to analyze control flow within methods more closely. All reachable

crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs

@@ -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)

crates/ty_python_semantic/src/semantic_index/use_def.rs

@@ -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,

crates/ty_python_semantic/src/types.rs

@@ -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,

crates/ty_python_semantic/src/types/class.rs

@@ -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;
                 }
 

crates/ty_python_semantic/src/types/infer.rs

@@ -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;
         }