diff --git a/crates/ty_ide/src/goto.rs b/crates/ty_ide/src/goto.rs index 8ac83bf965..101726363a 100644 --- a/crates/ty_ide/src/goto.rs +++ b/crates/ty_ide/src/goto.rs @@ -4,12 +4,13 @@ pub use crate::goto_type_definition::goto_type_definition; use crate::find_node::covering_node; use crate::stub_mapping::StubMapper; -use ruff_db::parsed::ParsedModuleRef; +use ruff_db::parsed::{ParsedModuleRef, parsed_module}; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_parser::TokenKind; use ruff_text_size::{Ranged, TextRange, TextSize}; use ty_python_semantic::types::Type; -use ty_python_semantic::{HasType, SemanticModel}; +use ty_python_semantic::types::definitions_for_keyword_argument; +use ty_python_semantic::{HasType, SemanticModel, definitions_for_name}; #[derive(Clone, Copy, Debug)] pub(crate) enum GotoTarget<'a> { @@ -150,15 +151,19 @@ impl GotoTarget<'_> { use ruff_python_ast as ast; match self { - // For names, find the definitions of the symbol - GotoTarget::Expression(expression) => { - if let ast::ExprRef::Name(name) = expression { - Self::get_name_definition_targets(name, file, db, stub_mapper) - } else { - // For other expressions, we can't find definitions - None - } - } + GotoTarget::Expression(expression) => match expression { + ast::ExprRef::Name(name) => definitions_to_navigation_targets( + db, + stub_mapper, + definitions_for_name(db, file, name), + ), + ast::ExprRef::Attribute(attribute) => definitions_to_navigation_targets( + db, + stub_mapper, + ty_python_semantic::definitions_for_attribute(db, file, attribute), + ), + _ => None, + }, // For already-defined symbols, they are their own definitions GotoTarget::FunctionDef(function) => { @@ -195,41 +200,31 @@ impl GotoTarget<'_> { None } - // TODO: Handle attribute and method accesses (y in `x.y` expressions) - // TODO: Handle keyword arguments in call expression + // Handle keyword arguments in call expressions + GotoTarget::KeywordArgument(keyword) => { + // Find the call expression that contains this keyword + let module = parsed_module(db, file).load(db); + + // Use the keyword's range to find the containing call expression + let covering_node = covering_node(module.syntax().into(), keyword.range()) + .find_first(|node| matches!(node, AnyNodeRef::ExprCall(_))) + .ok()?; + + if let AnyNodeRef::ExprCall(call_expr) = covering_node.node() { + let definitions = + definitions_for_keyword_argument(db, file, keyword, call_expr); + return definitions_to_navigation_targets(db, stub_mapper, definitions); + } + + None + } + // TODO: Handle multi-part module names in import statements // TODO: Handle imported symbol in y in `from x import y as z` statement // TODO: Handle string literals that map to TypedDict fields _ => None, } } - - /// Get navigation targets for definitions associated with a name expression - fn get_name_definition_targets( - name: &ruff_python_ast::ExprName, - file: ruff_db::files::File, - db: &dyn crate::Db, - stub_mapper: Option<&StubMapper>, - ) -> Option { - use ty_python_semantic::definitions_for_name; - - // Get all definitions for this name - let mut definitions = definitions_for_name(db, file, name); - - // Apply stub mapping if a mapper is provided - if let Some(mapper) = stub_mapper { - definitions = mapper.map_definitions(definitions); - } - - if definitions.is_empty() { - return None; - } - - // Convert definitions to navigation targets - let targets = convert_resolved_definitions_to_targets(db, definitions); - - Some(crate::NavigationTargets::unique(targets)) - } } impl Ranged for GotoTarget<'_> { @@ -279,18 +274,35 @@ fn convert_resolved_definitions_to_targets( full_range: full_range.range(), } } - ty_python_semantic::ResolvedDefinition::ModuleFile(module_file) => { - // For module files, navigate to the beginning of the file + ty_python_semantic::ResolvedDefinition::FileWithRange(file_range) => { + // For file ranges, navigate to the specific range within the file crate::NavigationTarget { - file: module_file, - focus_range: ruff_text_size::TextRange::default(), // Start of file - full_range: ruff_text_size::TextRange::default(), // Start of file + file: file_range.file(), + focus_range: file_range.range(), + full_range: file_range.range(), } } }) .collect() } +/// Shared helper to map and convert resolved definitions into navigation targets. +fn definitions_to_navigation_targets<'db>( + db: &dyn crate::Db, + stub_mapper: Option<&StubMapper<'db>>, + mut definitions: Vec>, +) -> Option { + if let Some(mapper) = stub_mapper { + definitions = mapper.map_definitions(definitions); + } + if definitions.is_empty() { + None + } else { + let targets = convert_resolved_definitions_to_targets(db, definitions); + Some(crate::NavigationTargets::unique(targets)) + } +} + pub(crate) fn find_goto_target( parsed: &ParsedModuleRef, offset: TextSize, diff --git a/crates/ty_ide/src/goto_declaration.rs b/crates/ty_ide/src/goto_declaration.rs index c1f01d94b2..e403340eb4 100644 --- a/crates/ty_ide/src/goto_declaration.rs +++ b/crates/ty_ide/src/goto_declaration.rs @@ -611,7 +611,107 @@ def another_helper(): } #[test] - fn goto_declaration_builtin_type() { + fn goto_declaration_instance_attribute() { + let test = cursor_test( + " + class C: + def __init__(self): + self.x: int = 1 + + c = C() + y = c.x + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:4:21 + | + 2 | class C: + 3 | def __init__(self): + 4 | self.x: int = 1 + | ^^^^^^ + 5 | + 6 | c = C() + | + info: Source + --> main.py:7:17 + | + 6 | c = C() + 7 | y = c.x + | ^^^ + | + "); + } + + #[test] + fn goto_declaration_instance_attribute_no_annotation() { + let test = cursor_test( + " + class C: + def __init__(self): + self.x = 1 + + c = C() + y = c.x + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:4:21 + | + 2 | class C: + 3 | def __init__(self): + 4 | self.x = 1 + | ^^^^^^ + 5 | + 6 | c = C() + | + info: Source + --> main.py:7:17 + | + 6 | c = C() + 7 | y = c.x + | ^^^ + | + "); + } + + #[test] + fn goto_declaration_method_call_to_definition() { + let test = cursor_test( + " + class C: + def foo(self): + return 42 + + c = C() + res = c.foo() + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:3:21 + | + 2 | class C: + 3 | def foo(self): + | ^^^ + 4 | return 42 + | + info: Source + --> main.py:7:19 + | + 6 | c = C() + 7 | res = c.foo() + | ^^^^^ + | + "); + } + + #[test] + fn goto_declaration_module_attribute() { let test = cursor_test( r#" x: int = 42 @@ -721,6 +821,152 @@ def function(): "#); } + #[test] + fn goto_declaration_inherited_attribute() { + let test = cursor_test( + " + class A: + x = 10 + + class B(A): + pass + + b = B() + y = b.x + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:3:17 + | + 2 | class A: + 3 | x = 10 + | ^ + 4 | + 5 | class B(A): + | + info: Source + --> main.py:9:17 + | + 8 | b = B() + 9 | y = b.x + | ^^^ + | + "); + } + + #[test] + fn goto_declaration_property_getter_setter() { + let test = cursor_test( + " + class C: + def __init__(self): + self._value = 0 + + @property + def value(self): + return self._value + + c = C() + c.value = 42 + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:7:21 + | + 6 | @property + 7 | def value(self): + | ^^^^^ + 8 | return self._value + | + info: Source + --> main.py:11:13 + | + 10 | c = C() + 11 | c.value = 42 + | ^^^^^^^ + | + "); + } + + #[test] + fn goto_declaration_function_doc_attribute() { + let test = cursor_test( + r#" + def my_function(): + """This is a docstring.""" + return 42 + + doc = my_function.__doc__ + "#, + ); + + // Should navigate to the __doc__ property in the FunctionType class in typeshed + let result = test.goto_declaration(); + + assert!( + !result.contains("No goto target found"), + "Should find builtin __doc__ attribute" + ); + assert!( + !result.contains("No declarations found"), + "Should find builtin __doc__ declarations" + ); + + // Should navigate to a typeshed file containing the __doc__ attribute + assert!( + result.contains("types.pyi") || result.contains("builtins.pyi"), + "Should navigate to typeshed file with __doc__ definition" + ); + assert!( + result.contains("__doc__"), + "Should find the __doc__ attribute definition" + ); + assert!( + result.contains("info[goto-declaration]: Declaration"), + "Should be a goto-declaration result" + ); + } + + #[test] + fn goto_declaration_protocol_instance_attribute() { + let test = cursor_test( + " + from typing import Protocol + + class Drawable(Protocol): + def draw(self) -> None: ... + name: str + + def use_drawable(obj: Drawable): + obj.name + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:6:17 + | + 4 | class Drawable(Protocol): + 5 | def draw(self) -> None: ... + 6 | name: str + | ^^^^ + 7 | + 8 | def use_drawable(obj: Drawable): + | + info: Source + --> main.py:9:17 + | + 8 | def use_drawable(obj: Drawable): + 9 | obj.name + | ^^^^^^^^ + | + "); + } + #[test] fn goto_declaration_generic_method_class_type() { let test = cursor_test( @@ -756,6 +1002,94 @@ class MyClass: "); } + #[test] + fn goto_declaration_keyword_argument_simple() { + let test = cursor_test( + " + def my_function(x, y, z=10): + return x + y + z + + result = my_function(1, y=2, z=3) + ", + ); + + assert_snapshot!(test.goto_declaration(), @r" + info[goto-declaration]: Declaration + --> main.py:2:32 + | + 2 | def my_function(x, y, z=10): + | ^ + 3 | return x + y + z + | + info: Source + --> main.py:5:37 + | + 3 | return x + y + z + 4 | + 5 | result = my_function(1, y=2, z=3) + | ^ + | + "); + } + + #[test] + fn goto_declaration_keyword_argument_overloaded() { + let test = cursor_test( + r#" + from typing import overload + + @overload + def process(data: str, format: str) -> str: ... + + @overload + def process(data: int, format: int) -> int: ... + + def process(data, format): + return data + + # Call the overloaded function + result = process("hello", format="json") + "#, + ); + + // Should navigate to the parameter in both matching overloads + assert_snapshot!(test.goto_declaration(), @r#" + info[goto-declaration]: Declaration + --> main.py:5:36 + | + 4 | @overload + 5 | def process(data: str, format: str) -> str: ... + | ^^^^^^ + 6 | + 7 | @overload + | + info: Source + --> main.py:14:39 + | + 13 | # Call the overloaded function + 14 | result = process("hello", format="json") + | ^^^^^^ + | + + info[goto-declaration]: Declaration + --> main.py:8:36 + | + 7 | @overload + 8 | def process(data: int, format: int) -> int: ... + | ^^^^^^ + 9 | + 10 | def process(data, format): + | + info: Source + --> main.py:14:39 + | + 13 | # Call the overloaded function + 14 | result = process("hello", format="json") + | ^^^^^^ + | + "#); + } + impl CursorTest { fn goto_declaration(&self) -> String { let Some(targets) = goto_declaration(&self.db, self.cursor.file, self.cursor.offset) diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index 63717874f9..06d8d6217c 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -17,8 +17,8 @@ pub use program::{ pub use python_platform::PythonPlatform; pub use semantic_model::{Completion, CompletionKind, HasType, NameKind, SemanticModel}; pub use site_packages::{PythonEnvironment, SitePackagesPaths, SysPrefixPathOrigin}; -pub use types::definitions_for_name; pub use types::ide_support::ResolvedDefinition; +pub use types::{definitions_for_attribute, definitions_for_name}; pub use util::diagnostics::add_inferred_python_version_hint_to_diagnostic; pub mod ast_node_ref; diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 5a780db7c4..54677ee7e5 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -49,7 +49,7 @@ use crate::types::generics::{ }; pub use crate::types::ide_support::{ CallSignatureDetails, Member, all_members, call_signature_details, definition_kind_for_name, - definitions_for_name, + definitions_for_attribute, definitions_for_keyword_argument, definitions_for_name, }; use crate::types::infer::infer_unpack_types; use crate::types::mro::{Mro, MroError, MroIterator}; diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index ddc673bdc5..b34064f603 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -13,12 +13,16 @@ use crate::types::call::CallArguments; use crate::types::signatures::Signature; use crate::types::{ClassBase, ClassLiteral, DynamicType, KnownClass, KnownInstanceType, Type}; use crate::{Db, HasType, NameKind, SemanticModel}; -use ruff_db::files::File; +use ruff_db::files::{File, FileRange}; +use ruff_db::parsed::parsed_module; use ruff_python_ast as ast; use ruff_python_ast::name::Name; -use ruff_text_size::TextRange; +use ruff_text_size::{Ranged, TextRange}; use rustc_hash::FxHashSet; +pub use resolve_definition::ResolvedDefinition; +use resolve_definition::{find_symbol_in_scope, resolve_definition}; + pub(crate) fn all_declarations_and_bindings<'db>( db: &'db dyn Db, scope_id: ScopeId<'db>, @@ -366,7 +370,7 @@ pub fn definition_kind_for_name<'db>( let name_str = name.id.as_str(); // Get the scope for this name expression - let file_scope = index.try_expression_scope_id(&ast::Expr::Name(name.clone()))?; + let file_scope = index.expression_scope_id(&ast::ExprRef::from(name)); // Get the place table for this scope let place_table = index.place_table(file_scope); @@ -399,9 +403,7 @@ pub fn definitions_for_name<'db>( let name_str = name.id.as_str(); // Get the scope for this name expression - let Some(file_scope) = index.try_expression_scope_id(&ast::Expr::Name(name.clone())) else { - return Vec::new(); - }; + let file_scope = index.expression_scope_id(&ast::ExprRef::from(name)); let mut all_definitions = Vec::new(); @@ -503,6 +505,183 @@ pub fn definitions_for_name<'db>( } } +/// Returns all resolved definitions for an attribute expression `x.y`. +/// This function duplicates much of the functionality in the semantic +/// analyzer, but it has somewhat different behavior so we've decided +/// to keep it separate for now. One key difference is that this function +/// doesn't model the descriptor protocol when accessing attributes. +/// For "go to definition", we want to get the type of the descriptor object +/// rather than "invoking" its `__get__` or `__set__` method. +/// If this becomes a maintenance burden in the future, it may be worth +/// changing the corresponding logic in the semantic analyzer to conditionally +/// handle this case through the use of mode flags. +pub fn definitions_for_attribute<'db>( + db: &'db dyn Db, + file: File, + attribute: &ast::ExprAttribute, +) -> Vec> { + let name_str = attribute.attr.as_str(); + let model = SemanticModel::new(db, file); + + let mut resolved = Vec::new(); + + // Determine the type of the LHS + let lhs_ty = attribute.value.inferred_type(&model); + let tys = match lhs_ty { + Type::Union(union) => union.elements(db).to_vec(), + _ => vec![lhs_ty], + }; + + // Expand intersections for each subtype into their components + let expanded_tys = tys + .into_iter() + .flat_map(|ty| match ty { + Type::Intersection(intersection) => intersection.positive(db).iter().copied().collect(), + _ => vec![ty], + }) + .collect::>(); + + for ty in expanded_tys { + // Handle modules + if let Type::ModuleLiteral(module_literal) = ty { + if let Some(module_file) = module_literal.module(db).file() { + let module_scope = global_scope(db, module_file); + for def in find_symbol_in_scope(db, module_scope, name_str) { + resolved.extend(resolve_definition(db, def, Some(name_str))); + } + } + continue; + } + + // First, transform the type to its meta type, unless it's already a class-like type. + let meta_type = match ty { + Type::ClassLiteral(_) | Type::SubclassOf(_) | Type::GenericAlias(_) => ty, + _ => ty.to_meta_type(db), + }; + let class_literal = match meta_type { + Type::ClassLiteral(class_literal) => class_literal, + Type::SubclassOf(subclass) => match subclass.subclass_of().into_class() { + Some(cls) => cls.class_literal(db).0, + None => continue, + }, + _ => continue, + }; + + // Walk the MRO: include class and its ancestors, but stop when we find a match + 'scopes: for ancestor in class_literal + .iter_mro(db, None) + .filter_map(ClassBase::into_class) + .map(|cls| cls.class_literal(db).0) + { + let class_scope = ancestor.body_scope(db); + let class_place_table = crate::semantic_index::place_table(db, class_scope); + + // Look for class-level declarations and bindings + if let Some(place_id) = class_place_table.place_id_by_name(name_str) { + let use_def = use_def_map(db, class_scope); + + // Check declarations first + for decl in use_def.all_reachable_declarations(place_id) { + if let Some(def) = decl.declaration.definition() { + resolved.extend(resolve_definition(db, def, Some(name_str))); + break 'scopes; + } + } + + // If no declarations found, check bindings + for binding in use_def.all_reachable_bindings(place_id) { + if let Some(def) = binding.binding.definition() { + resolved.extend(resolve_definition(db, def, Some(name_str))); + break 'scopes; + } + } + } + + // Look for instance attributes in method scopes (e.g., self.x = 1) + let file = class_scope.file(db); + let index = semantic_index(db, file); + + for function_scope_id in attribute_scopes(db, class_scope) { + let place_table = index.place_table(function_scope_id); + + if let Some(place_id) = place_table.place_id_by_instance_attribute_name(name_str) { + let use_def = index.use_def_map(function_scope_id); + + // Check declarations first + for decl in use_def.all_reachable_declarations(place_id) { + if let Some(def) = decl.declaration.definition() { + resolved.extend(resolve_definition(db, def, Some(name_str))); + break 'scopes; + } + } + + // If no declarations found, check bindings + for binding in use_def.all_reachable_bindings(place_id) { + if let Some(def) = binding.binding.definition() { + resolved.extend(resolve_definition(db, def, Some(name_str))); + break 'scopes; + } + } + } + } + + // TODO: Add support for metaclass attribute lookups + } + } + + resolved +} + +/// Returns definitions for a keyword argument in a call expression. +/// This resolves the keyword argument to the corresponding parameter(s) in the callable's signature(s). +pub fn definitions_for_keyword_argument<'db>( + db: &'db dyn Db, + file: File, + keyword: &ast::Keyword, + call_expr: &ast::ExprCall, +) -> Vec> { + let model = SemanticModel::new(db, file); + let func_type = call_expr.func.inferred_type(&model); + + let Some(keyword_name) = keyword.arg.as_ref() else { + return Vec::new(); + }; + let keyword_name_str = keyword_name.as_str(); + + let mut resolved_definitions = Vec::new(); + + if let Some(Type::Callable(callable_type)) = func_type.into_callable(db) { + let signatures = callable_type.signatures(db); + + // For each signature, find the parameter with the matching name + for signature in signatures { + if let Some((_param_index, _param)) = + signature.parameters().keyword_by_name(keyword_name_str) + { + if let Some(function_definition) = signature.definition() { + let function_file = function_definition.file(db); + let module = parsed_module(db, function_file).load(db); + let def_kind = function_definition.kind(db); + + if let DefinitionKind::Function(function_ast_ref) = def_kind { + let function_node = function_ast_ref.node(&module); + + if let Some(parameter_range) = + find_parameter_range(&function_node.parameters, keyword_name_str) + { + resolved_definitions.push(ResolvedDefinition::FileWithRange( + FileRange::new(function_file, parameter_range), + )); + } + } + } + } + } + } + + resolved_definitions +} + /// Details about a callable signature for IDE support. #[derive(Debug, Clone)] pub struct CallSignatureDetails<'db> { @@ -526,7 +705,7 @@ pub struct CallSignatureDetails<'db> { pub definition: Option>, /// Mapping from argument indices to parameter indices. This helps - /// determine which parameter corresponds to which argument position. + /// identify which argument corresponds to which parameter. pub argument_to_parameter_mapping: Vec>, } @@ -573,14 +752,27 @@ pub fn call_signature_details<'db>( } } +/// Find the text range of a specific parameter in function parameters by name. +/// Only searches for parameters that can be addressed by name in keyword arguments. +fn find_parameter_range(parameters: &ast::Parameters, parameter_name: &str) -> Option { + // Check regular positional and keyword-only parameters + parameters + .args + .iter() + .chain(¶meters.kwonlyargs) + .find(|param| param.parameter.name.as_str() == parameter_name) + .map(|param| param.parameter.name.range()) +} + mod resolve_definition { //! Resolves an Import, `ImportFrom` or `StarImport` definition to one or more //! "resolved definitions". This is done recursively to find the original //! definition targeted by the import. - use ruff_db::files::File; + use ruff_db::files::{File, FileRange}; use ruff_db::parsed::parsed_module; use ruff_python_ast as ast; + use ruff_text_size::TextRange; use rustc_hash::FxHashSet; use crate::semantic_index::definition::{Definition, DefinitionKind}; @@ -588,16 +780,17 @@ mod resolve_definition { use crate::semantic_index::{global_scope, place_table, use_def_map}; use crate::{Db, ModuleName, resolve_module}; - /// Represents the result of resolving an import to either a specific definition or a module file. + /// Represents the result of resolving an import to either a specific definition or + /// a specific range within a file. /// This enum helps distinguish between cases where an import resolves to: /// - A specific definition within a module (e.g., `from os import path` -> definition of `path`) - /// - An entire module file (e.g., `import os` -> the `os` module file itself) + /// - A specific range within a file, sometimes an empty range at the top of the file #[derive(Debug, Clone, PartialEq, Eq)] pub enum ResolvedDefinition<'db> { /// The import resolved to a specific definition within a module Definition(Definition<'db>), - /// The import resolved to an entire module file - ModuleFile(File), + /// The import resolved to a file with a specific range + FileWithRange(FileRange), } /// Resolve import definitions to their targets. @@ -657,7 +850,10 @@ mod resolve_definition { // For simple imports like "import os", we want to navigate to the module itself. // Return the module file directly instead of trying to find definitions within it. - vec![ResolvedDefinition::ModuleFile(module_file)] + vec![ResolvedDefinition::FileWithRange(FileRange::new( + module_file, + TextRange::default(), + ))] } DefinitionKind::ImportFrom(import_from_def) => { @@ -767,6 +963,3 @@ mod resolve_definition { definitions } } - -pub use resolve_definition::ResolvedDefinition; -use resolve_definition::{find_symbol_in_scope, resolve_definition};