Skip to content

[ty] Return all attribute definitions for goto definition #24332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MichaReiser wants to merge 2 commits into
base: main
Choose a base branch
from
+183 −24
Open

[ty] Return all attribute definitions for goto definition #24332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a1df94
[ty] Return all attribute definitions for goto
MichaReiser Mar 31, 2026
89cb39f
Add rename test
MichaReiser Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions crates/ty_ide/src/goto_definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,96 @@ p = Point<CURSOR>(1, 2)
"#);
}

#[test]
fn goto_definition_attribute_redeclarations() {
let test = CursorTest::builder()
.source(
"main.py",
r#"
class Test:
a: str
a: str

test = Test()

test.a<CURSOR>
"#,
)
.build();

assert_snapshot!(test.goto_definition(), @r#"
info[goto-definition]: Go to definition
--> main.py:8:6
|
6 | test = Test()
7 |
8 | test.a
| ^ Clicking here
|
info: Found 2 definitions
--> main.py:3:5
|
2 | class Test:
3 | a: str
| -
4 | a: str
| -
Comment on lines +2074 to +2077
Copy link
Copy Markdown
Contributor

@sharkdp sharkdp Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, interesting. It seems a bit strange to me, but as you pointed out, this is now consistent with what we do for non-class scopes. And it seems like the right choice for renaming, and of course for properties.

5 |
6 | test = Test()
|
"#);
}

#[test]
fn goto_definition_property_getter_and_setter() {
let test = CursorTest::builder()
.source(
"main.py",
r#"
class Test:
@property
def a(self) -> str:
return ""

@a.setter
def a(self, value: str) -> None:
pass

test = Test()

test.a<CURSOR>
"#,
)
.build();

assert_snapshot!(test.goto_definition(), @r#"
info[goto-definition]: Go to definition
--> main.py:13:6
|
11 | test = Test()
12 |
13 | test.a
| ^ Clicking here
|
info: Found 2 definitions
--> main.py:4:9
|
2 | class Test:
3 | @property
4 | def a(self) -> str:
| -
5 | return ""
|
::: main.py:8:9
|
7 | @a.setter
8 | def a(self, value: str) -> None:
| -
9 | pass
|
"#);
}

/// Goto-definition works when accessing type attributes on class objects.
#[test]
fn goto_definition_for_type_attributes_on_class_objects() {
Expand Down
55 changes: 55 additions & 0 deletions crates/ty_ide/src/rename.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2591,6 +2591,61 @@ result = func(10, y=20)
"#);
}

#[test]
fn rename_property_from_assignment_usage() {
let test = CursorTest::builder()
.source(
"lib.py",
r#"
class Foo:
@property
def my_property(self) -> int:
return 42

@my_property.setter
def my_property(self, value: int) -> None:
pass
"#,
)
.source(
"main.py",
r#"
from lib import Foo

print(Foo().my_property)
Foo().my_<CURSOR>property = 56
"#,
)
.build();

assert_snapshot!(test.rename("better_name"), @"
info[rename]: Rename symbol (found 5 locations)
--> main.py:4:13
|
2 | from lib import Foo
3 |
4 | print(Foo().my_property)
| ^^^^^^^^^^^
5 | Foo().my_property = 56
Comment on lines +2622 to +2629
Copy link
Copy Markdown
Member Author

@MichaReiser MichaReiser Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already worked before because rename tests if the definitions of the renamed symbol and the current symbol's definitions intersect. But I felt it's still worth adding a test for it.

| -----------
|
::: lib.py:4:9
|
2 | class Foo:
3 | @property
4 | def my_property(self) -> int:
| -----------
5 | return 42
6 |
7 | @my_property.setter
| -----------
8 | def my_property(self, value: int) -> None:
| -----------
9 | pass
|
");
}

// TODO: This should rename all attribute usages
// Note: Pylance only renames the assignment in `__init__`.
#[test]
Expand Down
62 changes: 38 additions & 24 deletions crates/ty_python_semantic/src/types/ide_support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,32 +347,39 @@ fn definitions_for_attribute_in_class_hierarchy<'db>(
// Look for class-level declarations and bindings
if let Some(place_id) = class_place_table.symbol_id(attribute_name) {
let use_def = use_def_map(db, class_scope);
let mut ancestor_resolved = Vec::new();

// Check declarations first
// Declarations take precedence over bindings, but attribute go-to-definition
// should return all co-definitions for the chosen class scope.
for decl in use_def.reachable_symbol_declarations(place_id) {
if let Some(def) = decl.declaration.definition() {
resolved.extend(resolve_definition(
ancestor_resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
break 'scopes;
}
}

// If no declarations found, check bindings
for binding in use_def.reachable_symbol_bindings(place_id) {
if let Some(def) = binding.binding.definition() {
resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
break 'scopes;
if ancestor_resolved.is_empty() {
for binding in use_def.reachable_symbol_bindings(place_id) {
if let Some(def) = binding.binding.definition() {
ancestor_resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
}
}
}

if !ancestor_resolved.is_empty() {
resolved.extend(ancestor_resolved);
break 'scopes;
}
}

// Look for instance attributes in method scopes (e.g., self.x = 1)
Expand All @@ -385,32 +392,39 @@ fn definitions_for_attribute_in_class_hierarchy<'db>(
.member_id_by_instance_attribute_name(attribute_name)
{
let use_def = index.use_def_map(function_scope_id);
let mut scope_resolved = Vec::new();

// Check declarations first
// Declarations take precedence over bindings, but return all
// co-definitions from the chosen method scope.
for decl in use_def.reachable_member_declarations(place_id) {
if let Some(def) = decl.declaration.definition() {
resolved.extend(resolve_definition(
scope_resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
break 'scopes;
}
}

// If no declarations found, check bindings
for binding in use_def.reachable_member_bindings(place_id) {
if let Some(def) = binding.binding.definition() {
resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
break 'scopes;
if scope_resolved.is_empty() {
for binding in use_def.reachable_member_bindings(place_id) {
if let Some(def) = binding.binding.definition() {
scope_resolved.extend(resolve_definition(
db,
def,
Some(attribute_name),
ImportAliasResolution::ResolveAliases,
));
}
}
}

if !scope_resolved.is_empty() {
resolved.extend(scope_resolved);
break 'scopes;
}
}
}
}
Expand Down
Loading