diff --git a/crates/ty_ide/src/goto_definition.rs b/crates/ty_ide/src/goto_definition.rs index cedc966babd4b..b7147e6274d6c 100644 --- a/crates/ty_ide/src/goto_definition.rs +++ b/crates/ty_ide/src/goto_definition.rs @@ -2041,6 +2041,96 @@ p = Point(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 + "#, + ) + .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 + | - + 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 + "#, + ) + .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() { diff --git a/crates/ty_ide/src/rename.rs b/crates/ty_ide/src/rename.rs index e7ddbab2d38f0..72a25c19ccba5 100644 --- a/crates/ty_ide/src/rename.rs +++ b/crates/ty_ide/src/rename.rs @@ -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_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 + | ----------- + | + ::: 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] diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index d3aa6b61a8364..96a182043f3aa 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -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) @@ -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; + } } } }