Skip to content

Commit ab3f52d

Browse files
committed
[ty] Prefer declared base class attribute over inferred attribute on subclass
1 parent 150ea92 commit ab3f52d

File tree

5 files changed

+153
-82
lines changed

5 files changed

+153
-82
lines changed

crates/ty_python_semantic/resources/mdtest/attributes.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -901,24 +901,21 @@ reveal_type(Derived().redeclared_with_wider_type) # revealed: str | int | None
901901
reveal_type(Derived.overwritten_in_subclass_body) # revealed: Unknown | None
902902
reveal_type(Derived().overwritten_in_subclass_body) # revealed: Unknown | None | str
903903

904-
# TODO: Both of these should be `str`
905904
reveal_type(Derived.overwritten_in_subclass_method) # revealed: str
906-
reveal_type(Derived().overwritten_in_subclass_method) # revealed: str | Unknown | None
905+
reveal_type(Derived().overwritten_in_subclass_method) # revealed: str
907906

908907
reveal_type(Derived().pure_attribute) # revealed: str | None
909908

910909
# TODO: This should be `str`
911910
reveal_type(Derived().pure_overwritten_in_subclass_body) # revealed: Unknown | None | str
912911

913-
# TODO: This should be `str`
914-
reveal_type(Derived().pure_overwritten_in_subclass_method) # revealed: Unknown | None
912+
reveal_type(Derived().pure_overwritten_in_subclass_method) # revealed: str
915913

916914
# TODO: Both of these should be `Unknown | Literal["intermediate", "base"]`
917915
reveal_type(Derived.undeclared) # revealed: Unknown | Literal["intermediate"]
918916
reveal_type(Derived().undeclared) # revealed: Unknown | Literal["intermediate"]
919917

920-
# TODO: This should be `Unknown | Literal["intermediate", "base"]`
921-
reveal_type(Derived().pure_undeclared) # revealed: Unknown | Literal["intermediate"]
918+
reveal_type(Derived().pure_undeclared) # revealed: Unknown | Literal["intermediate", "base"]
922919
```
923920

924921
## Accessing attributes on class objects

crates/ty_python_semantic/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ mod db;
3434
mod dunder_all;
3535
pub mod lint;
3636
pub(crate) mod list;
37+
mod member;
3738
mod module_name;
3839
mod module_resolver;
3940
mod node_key;
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
use crate::{place::PlaceAndQualifiers, types::Type};
2+
3+
/// The return type of certain member-lookup operations. Contains information
4+
/// about the type, type qualifiers, boundness/declaredness, and additional
5+
/// metadata (e.g. whether or not the member was declared)
6+
#[derive(Debug, Clone, PartialEq, Eq, salsa::Update, get_size2::GetSize)]
7+
pub(crate) struct Member<'db> {
8+
/// Type, qualifiers, and boundness information of this member
9+
pub(crate) inner: PlaceAndQualifiers<'db>,
10+
11+
/// Whether or not this member was explicitly declared (e.g. `attr: int = 1`
12+
/// on the class body or `self.attr: int = 1` in a class method), or if the
13+
/// type was inferred (e.g. `attr = 1` on the class body or `self.attr = 1`
14+
/// in a class method).
15+
pub(crate) is_declared: bool,
16+
}
17+
18+
impl Default for Member<'_> {
19+
fn default() -> Self {
20+
Member::undeclared(PlaceAndQualifiers::default())
21+
}
22+
}
23+
24+
impl<'db> Member<'db> {
25+
pub(crate) fn undeclared(inner: PlaceAndQualifiers<'db>) -> Self {
26+
Self {
27+
inner,
28+
is_declared: false,
29+
}
30+
}
31+
32+
pub(crate) fn declared(inner: PlaceAndQualifiers<'db>) -> Self {
33+
Self {
34+
inner,
35+
is_declared: true,
36+
}
37+
}
38+
39+
pub(crate) fn is_unbound(&self) -> bool {
40+
self.inner.place.is_unbound()
41+
}
42+
43+
/// Represents the absence of a member.
44+
pub(crate) fn unbound() -> Self {
45+
Self::undeclared(PlaceAndQualifiers::default())
46+
}
47+
48+
#[must_use]
49+
pub(crate) fn map_type(self, f: impl FnOnce(Type<'db>) -> Type<'db>) -> Self {
50+
Self {
51+
inner: self.inner.map_type(f),
52+
is_declared: self.is_declared,
53+
}
54+
}
55+
}

crates/ty_python_semantic/src/place.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use ruff_db::files::File;
22

33
use crate::dunder_all::dunder_all_names;
4+
use crate::member::Member;
45
use crate::module_resolver::{KnownModule, file_to_module};
56
use crate::semantic_index::definition::{Definition, DefinitionState};
67
use crate::semantic_index::place::{PlaceExprRef, ScopedPlaceId};
@@ -232,13 +233,9 @@ pub(crate) fn place<'db>(
232233
)
233234
}
234235

235-
/// Infer the public type of a class symbol (its type as seen from outside its scope) in the given
236+
/// Infer the public type of a class member/symbol (its type as seen from outside its scope) in the given
236237
/// `scope`.
237-
pub(crate) fn class_symbol<'db>(
238-
db: &'db dyn Db,
239-
scope: ScopeId<'db>,
240-
name: &str,
241-
) -> PlaceAndQualifiers<'db> {
238+
pub(crate) fn class_member<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Member<'db> {
242239
place_table(db, scope)
243240
.symbol_id(name)
244241
.map(|symbol_id| {
@@ -252,10 +249,10 @@ pub(crate) fn class_symbol<'db>(
252249

253250
if !place_and_quals.place.is_unbound() && !place_and_quals.is_init_var() {
254251
// Trust the declared type if we see a class-level declaration
255-
return place_and_quals;
252+
return Member::declared(place_and_quals);
256253
}
257254

258-
if let PlaceAndQualifiers {
255+
let place = if let PlaceAndQualifiers {
259256
place: Place::Type(ty, _),
260257
qualifiers,
261258
} = place_and_quals
@@ -275,7 +272,9 @@ pub(crate) fn class_symbol<'db>(
275272
}
276273
} else {
277274
Place::Unbound.into()
278-
}
275+
};
276+
277+
Member::undeclared(place)
279278
})
280279
.unwrap_or_default()
281280
}

0 commit comments

Comments
 (0)