diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index 99b2b6a270..12bab34fb8 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/crates/ty_python_semantic/resources/mdtest/attributes.md @@ -43,7 +43,6 @@ reveal_type(c_instance.declared_only) # revealed: Unknown reveal_type(c_instance.declared_and_bound) # revealed: bool -# error: [possibly-unbound-attribute] reveal_type(c_instance.possibly_undeclared_unbound) # revealed: str # This assignment is fine, as we infer `Unknown | Literal[1, "a"]` for `inferred_from_value`. @@ -265,7 +264,7 @@ class C: # TODO: Mypy and pyright do not support this, but it would be great if we could # infer `Unknown | str` here (`Weird` is not a possible type for the `w` attribute). -reveal_type(C().w) # revealed: Unknown +reveal_type(C().w) # revealed: Unknown | Weird ``` #### Attributes defined in tuple unpackings @@ -342,10 +341,7 @@ class C: for self.z in NonIterable(): pass -# Iterable might be empty -# error: [possibly-unbound-attribute] reveal_type(C().x) # revealed: Unknown | int -# error: [possibly-unbound-attribute] reveal_type(C().y) # revealed: Unknown | str ``` @@ -453,8 +449,8 @@ reveal_type(c_instance.g) # revealed: Unknown #### Conditionally declared / bound attributes -Attributes are possibly unbound if they, or the method to which they are added are conditionally -declared / bound. +We currently treat implicit instance attributes to be bound, even if they are only conditionally +defined: ```py def flag() -> bool: @@ -472,13 +468,9 @@ class C: c_instance = C() -# error: [possibly-unbound-attribute] reveal_type(c_instance.a1) # revealed: str | None -# error: [possibly-unbound-attribute] reveal_type(c_instance.a2) # revealed: str | None -# error: [possibly-unbound-attribute] reveal_type(c_instance.b1) # revealed: Unknown | Literal[1] -# error: [possibly-unbound-attribute] reveal_type(c_instance.b2) # revealed: Unknown | Literal[1] ``` @@ -620,8 +612,10 @@ reveal_type(C(True).a) # revealed: Unknown | Literal[1] # error: [unresolved-attribute] reveal_type(C(True).b) # revealed: Unknown reveal_type(C(True).c) # revealed: Unknown | Literal[3] | str -# TODO: this attribute is possibly unbound -reveal_type(C(True).d) # revealed: Unknown | Literal[5] +# Ideally, this would just be `Unknown | Literal[5]`, but we currently do not +# attempt to analyze control flow within methods more closely. All reachable +# attribute assignments are considered, so `self.x = 4` is also included: +reveal_type(C(True).d) # revealed: Unknown | Literal[4, 5] # error: [unresolved-attribute] reveal_type(C(True).e) # revealed: Unknown ``` @@ -1289,6 +1283,10 @@ def _(flag: bool): ### Possibly unbound/undeclared instance attribute +We currently treat implicit instance attributes to be bound, even if they are only conditionally +defined within a method. If the class-level definition or the whole method is only conditionally +available, we emit a `possibly-unbound-attribute` diagnostic. + #### Possibly unbound and undeclared ```py @@ -1320,10 +1318,8 @@ def _(flag: bool): else: self.y = "b" - # error: [possibly-unbound-attribute] reveal_type(Foo().x) # revealed: Unknown | Literal[1] - # error: [possibly-unbound-attribute] Foo().x = 2 reveal_type(Foo().y) # revealed: Unknown | Literal["a", "b"] diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 2fafc9d3ef..385bc60b94 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -62,10 +62,6 @@ impl<'db> Place<'db> { Place::Type(ty.into(), Boundness::Bound) } - pub(crate) fn possibly_unbound(ty: impl Into>) -> 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 diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 5dc9aa3cad..e111f9aac5 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -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, )) }) diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 48b06ac56d..14451b3ed2 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -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() diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 7c4e0be72c..5ec7cb3a4a 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -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 } } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index f7702b66f3..1635458fb5 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -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(), diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index a9cd30ecc4..5678d5c217 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -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]