diff --git a/crates/ty_ide/src/find_references.rs b/crates/ty_ide/src/find_references.rs index e62608e555..4a2da0e598 100644 --- a/crates/ty_ide/src/find_references.rs +++ b/crates/ty_ide/src/find_references.rs @@ -2101,9 +2101,21 @@ func_alias() ) .build(); - // TODO: this should also highlight the RHS subpkg in the import + // The references correctly identify: + // 1. The RHS of the import (`from .subpkg import subpkg`) + // 2. The usage (`x = subpkg`) + // 3. The original definition in the subpackage (`subpkg: int = 10`) assert_snapshot!(test.references(), @r" info[references]: Reference 1 + --> mypackage/__init__.py:2:21 + | + 2 | from .subpkg import subpkg + | ^^^^^^ + 3 | + 4 | x = subpkg + | + + info[references]: Reference 2 --> mypackage/__init__.py:4:5 | 2 | from .subpkg import subpkg @@ -2111,6 +2123,13 @@ func_alias() 4 | x = subpkg | ^^^^^^ | + + info[references]: Reference 3 + --> mypackage/subpkg/__init__.py:2:1 + | + 2 | subpkg: int = 10 + | ^^^^^^ + | "); } diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index 217f8b420b..f669f07b09 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -19,7 +19,7 @@ use ty_python_semantic::types::ide_support::{ }; use ty_python_semantic::{ HasDefinition, HasType, ImportAliasResolution, SemanticModel, definitions_for_imported_symbol, - definitions_for_name, + definitions_for_name, definitions_for_name_in_scope, }; #[derive(Clone, Debug)] @@ -392,10 +392,24 @@ impl GotoTarget<'_> { GotoTarget::Expression(expression) => { definitions_for_expression(model, *expression, alias_resolution) } - // For already-defined symbols, they are their own definitions - GotoTarget::FunctionDef(function) => Some(vec![ResolvedDefinition::Definition( - function.definition(model), - )]), + // For function definitions, get all definitions with the same name in the scope. + // This is important for overloaded functions where multiple definitions share the same name. + GotoTarget::FunctionDef(function) => { + let definition = function.definition(model); + let db = model.db(); + let file = definition.file(db); + let file_scope = definition.file_scope(db); + let Some(name) = definition.name(db) else { + return Some(Definitions(vec![ResolvedDefinition::Definition(definition)])); + }; + Some(definitions_for_name_in_scope( + db, + file, + file_scope, + &name, + alias_resolution, + )) + } GotoTarget::ClassDef(class) => Some(vec![ResolvedDefinition::Definition( class.definition(model), diff --git a/crates/ty_ide/src/references.rs b/crates/ty_ide/src/references.rs index 5bec65a356..0a35aa6542 100644 --- a/crates/ty_ide/src/references.rs +++ b/crates/ty_ide/src/references.rs @@ -364,12 +364,13 @@ impl LocalReferencesFinder<'_> { /// Check if `Vec` match our target definitions fn navigation_targets_match(&self, current_targets: &NavigationTargets) -> bool { - // Since we're comparing the same symbol, all definitions should be equivalent - // We only need to check against the first target definition - if let Some(first_target) = self.target_definitions.iter().next() { + // Check if any current target matches any of our target definitions. + // This is important for overloaded functions where each overload is a separate + // definition but they all refer to the same logical symbol. + for target_def in self.target_definitions { for current_target in current_targets { - if current_target.file == first_target.file - && current_target.focus_range == first_target.focus_range + if current_target.file == target_def.file + && current_target.focus_range == target_def.focus_range { return true; } diff --git a/crates/ty_ide/src/rename.rs b/crates/ty_ide/src/rename.rs index fe51f06615..28a1d505b4 100644 --- a/crates/ty_ide/src/rename.rs +++ b/crates/ty_ide/src/rename.rs @@ -1486,20 +1486,26 @@ result = func(10, y=20) ) .build(); - // TODO(submodule-imports): this is incorrect, we should rename the `subpkg` int - // and the RHS of the import statement (but *not* rename the LHS). + // The rename correctly identifies: + // 1. The RHS of the import (`from .subpkg import subpkg`) + // 2. The usage (`x = subpkg`) + // 3. The original definition in the subpackage (`subpkg: int = 10`) // - // However us being cautious here *would* be good as the rename will actually - // result in a `subpkg` variable still existing in this code, as the import's LHS - // `DefinitionKind::ImportFromSubmodule` would stop being overwritten by the RHS! + // Note: The LHS submodule name is NOT renamed, which is correct. assert_snapshot!(test.rename("mypkg"), @r" - info[rename]: Rename symbol (found 1 locations) - --> mypackage/__init__.py:4:5 + info[rename]: Rename symbol (found 3 locations) + --> mypackage/__init__.py:2:21 | 2 | from .subpkg import subpkg + | ^^^^^^ 3 | 4 | x = subpkg - | ^^^^^^ + | ------ + | + ::: mypackage/subpkg/__init__.py:2:1 + | + 2 | subpkg: int = 10 + | ------ | "); } @@ -1534,23 +1540,31 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r#" - info[rename]: Rename symbol (found 3 locations) - --> lib.py:5:5 - | - 4 | @overload - 5 | def test() -> None: ... - | ^^^^ - 6 | @overload - 7 | def test(a: str) -> str: ... - | - ::: main.py:2:17 - | - 2 | from lib import test - | ---- - 3 | - 4 | test("test") - | ---- - | + info[rename]: Rename symbol (found 6 locations) + --> lib.py:5:5 + | + 4 | @overload + 5 | def test() -> None: ... + | ^^^^ + 6 | @overload + 7 | def test(a: str) -> str: ... + | ---- + 8 | @overload + 9 | def test(a: int) -> int: ... + | ---- + 10 | + 11 | def test(a: Any) -> Any: + | ---- + 12 | return a + | + ::: main.py:2:17 + | + 2 | from lib import test + | ---- + 3 | + 4 | test("test") + | ---- + | "#); } @@ -1586,23 +1600,31 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r#" - info[rename]: Rename symbol (found 2 locations) - --> lib.py:6:9 - | - 4 | class Test: - 5 | @overload - 6 | def test() -> None: ... - | ^^^^ - 7 | @overload - 8 | def test(a: str) -> str: ... - | - ::: main.py:4:8 - | - 2 | from lib import Test - 3 | - 4 | Test().test("test") - | ---- - | + info[rename]: Rename symbol (found 5 locations) + --> lib.py:6:9 + | + 4 | class Test: + 5 | @overload + 6 | def test() -> None: ... + | ^^^^ + 7 | @overload + 8 | def test(a: str) -> str: ... + | ---- + 9 | @overload + 10 | def test(a: int) -> int: ... + | ---- + 11 | + 12 | def test(a: Any) -> Any: + | ---- + 13 | return a + | + ::: main.py:4:8 + | + 2 | from lib import Test + 3 | + 4 | Test().test("test") + | ---- + | "#); } @@ -1636,23 +1658,31 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r#" - info[rename]: Rename symbol (found 3 locations) - --> main.py:2:17 - | - 2 | from lib import test - | ^^^^ - 3 | - 4 | test("test") - | ---- - | - ::: lib.py:5:5 - | - 4 | @overload - 5 | def test() -> None: ... - | ---- - 6 | @overload - 7 | def test(a: str) -> str: ... - | + info[rename]: Rename symbol (found 6 locations) + --> main.py:2:17 + | + 2 | from lib import test + | ^^^^ + 3 | + 4 | test("test") + | ---- + | + ::: lib.py:5:5 + | + 4 | @overload + 5 | def test() -> None: ... + | ---- + 6 | @overload + 7 | def test(a: str) -> str: ... + | ---- + 8 | @overload + 9 | def test(a: int) -> int: ... + | ---- + 10 | + 11 | def test(a: Any) -> Any: + | ---- + 12 | return a + | "#); } @@ -1728,7 +1758,7 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r" - info[rename]: Rename symbol (found 4 locations) + info[rename]: Rename symbol (found 5 locations) --> lib.py:4:9 | 2 | class Foo: @@ -1740,6 +1770,7 @@ result = func(10, y=20) 7 | @my_property.setter | ----------- 8 | def my_property(self, value: int) -> None: + | ----------- 9 | pass | ::: main.py:4:13 @@ -1754,8 +1785,6 @@ result = func(10, y=20) "); } - // TODO: this should rename the name of the function decorated with - // `@my_property.deleter` as well as the getter function name #[test] fn rename_property_with_deleter() { let test = CursorTest::builder() @@ -1784,7 +1813,7 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r" - info[rename]: Rename symbol (found 4 locations) + info[rename]: Rename symbol (found 5 locations) --> lib.py:4:9 | 2 | class Foo: @@ -1796,6 +1825,7 @@ result = func(10, y=20) 7 | @my_property.deleter | ----------- 8 | def my_property(self) -> None: + | ----------- 9 | pass | ::: main.py:4:13 @@ -1809,10 +1839,6 @@ result = func(10, y=20) | "); } - - // TODO: this should rename the name of the functions decorated with - // `@my_property.deleter` and `@my_property.deleter` as well as the - // getter function name #[test] fn rename_property_with_setter_and_deleter() { let test = CursorTest::builder() @@ -1846,7 +1872,7 @@ result = func(10, y=20) .build(); assert_snapshot!(test.rename("better_name"), @r" - info[rename]: Rename symbol (found 6 locations) + info[rename]: Rename symbol (found 8 locations) --> lib.py:4:9 | 2 | class Foo: @@ -1858,11 +1884,13 @@ result = func(10, y=20) 7 | @my_property.setter | ----------- 8 | def my_property(self, value: int) -> None: + | ----------- 9 | pass 10 | 11 | @my_property.deleter | ----------- 12 | def my_property(self) -> None: + | ----------- 13 | pass | ::: main.py:4:13 diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index cf386839c9..f81b386862 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -30,8 +30,8 @@ pub use suppression::create_suppression_fix; pub use types::DisplaySettings; pub use types::ide_support::{ ImportAliasResolution, ResolvedDefinition, definitions_for_attribute, definitions_for_bin_op, - definitions_for_imported_symbol, definitions_for_name, definitions_for_unary_op, - map_stub_definition, + definitions_for_imported_symbol, definitions_for_name, definitions_for_name_in_scope, + definitions_for_unary_op, map_stub_definition, }; pub mod ast_node_ref; diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 2659e75493..7235438662 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs @@ -33,7 +33,7 @@ pub struct Definition<'db> { pub file: File, /// The scope in which the definition occurs. - pub(crate) file_scope: FileScopeId, + pub file_scope: FileScopeId, /// The place ID of the definition. pub(crate) place: ScopedPlaceId, diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index a74cc82f7e..a619925b4e 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -190,6 +190,53 @@ pub fn definitions_for_name<'db>( } } +/// Returns all definitions for a name in a specific scope. +/// This is useful when you have a Definition and want to find all other definitions +/// with the same name in its scope (e.g., for overloaded functions). +pub fn definitions_for_name_in_scope<'db>( + db: &'db dyn Db, + file: ruff_db::files::File, + file_scope_id: crate::semantic_index::scope::FileScopeId, + name_str: &str, + alias_resolution: ImportAliasResolution, +) -> Vec> { + let index = semantic_index(db, file); + let place_table = index.place_table(file_scope_id); + + let Some(symbol_id) = place_table.symbol_id(name_str) else { + return vec![]; + }; + + let use_def_map = index.use_def_map(file_scope_id); + let mut all_definitions = FxIndexSet::default(); + + // Get all definitions (both bindings and declarations) for this symbol + let bindings = use_def_map.all_reachable_symbol_bindings(symbol_id); + let declarations = use_def_map.all_reachable_symbol_declarations(symbol_id); + + for binding in bindings { + if let Some(def) = binding.binding.definition() { + all_definitions.insert(def); + } + } + + for declaration in declarations { + if let Some(def) = declaration.declaration.definition() { + all_definitions.insert(def); + } + } + + // Resolve import definitions to their targets + let mut resolved_definitions = Vec::new(); + + for definition in &all_definitions { + let resolved = resolve_definition(db, *definition, Some(name_str), alias_resolution); + resolved_definitions.extend(resolved); + } + + resolved_definitions +} + fn is_float_or_complex_annotation(db: &dyn Db, ty: UnionType, name: &str) -> bool { let float_or_complex_ty = match name { "float" => UnionType::from_elements(