From ebd8ad31580797a113ce6ca621d7d752cf05466e Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 17 Sep 2026 20:46:58 +0200 Subject: [PATCH] [ty] Find parameter references across files (#28670) --- crates/ty_ide/src/find_references.rs | 110 +++++++++++++++++++-------- crates/ty_ide/src/references.rs | 93 +++++----------------- 2 files changed, 97 insertions(+), 106 deletions(-) diff --git a/crates/ty_ide/src/find_references.rs b/crates/ty_ide/src/find_references.rs index f7bd4450c1..6b5ae9ab10 100644 --- a/crates/ty_ide/src/find_references.rs +++ b/crates/ty_ide/src/find_references.rs @@ -1546,20 +1546,26 @@ result = func(value=42) let test = CursorTest::builder() .source( "utils.py", - " + r#" def func(value: int): return value * 2 - -result = func(value=42) -", +"#, ) .source( "caller.py", - " + r#" +from utils import func + +result = func(value=42) +"#, + ) + .source( + "other.py", + r#" from utils import func result = func(value=1) -", +"#, ) .build(); @@ -1567,6 +1573,11 @@ result = func(value=1) info[references]: Found 4 references --> caller.py:4:15 | + 4 | result = func(value=42) + | ----- + | + ::: other.py:4:15 + | 4 | result = func(value=1) | ----- | @@ -1576,9 +1587,6 @@ result = func(value=1) | ----- 3 | return value * 2 | ----- - 4 | - 5 | result = func(value=42) - | ----- "); } @@ -1650,39 +1658,79 @@ instance = ExampleClass(old_name="test") } #[test] - fn multi_file_nested_function_parameter_references_do_not_include_keyword_argument_labels() { + fn multi_file_nested_function_parameter_references_include_keyword_argument_labels() { let test = CursorTest::builder() .source( - "outer.py", - " -def outer(): - def inner(value: int): - return value * 2 - return inner -", + "lib.py", + r#" +class Handler: + def __init__(self) -> None: + def process(*, value: int) -> int: + return value + self.process = process +"#, ) .source( "caller.py", - " -from outer import outer + r#" +from lib import Handler -func = outer() -result = func(value=10) -", +Handler().process(value=42) +"#, ) .build(); - // TODO(parameter-keyword-references): Nested callable owners are intentionally excluded by - // the external-visibility heuristic (perf/signal tradeoff). - // Ideal output would also include `caller.py` at `func(value=10)` on `value`. assert_snapshot!(test.references(), @" - info[references]: Found 2 references - --> outer.py:3:15 + info[references]: Found 3 references + --> caller.py:4:19 | - 3 | def inner(value: int): - | ----- - 4 | return value * 2 - | ----- + 4 | Handler().process(value=42) + | ----- + | + ::: lib.py:4:24 + | + 4 | def process(*, value: int) -> int: + | ----- + 5 | return value + | ----- + "); + } + + #[test] + fn multi_file_nested_class_parameter_references_include_keyword_argument_labels() { + let test = CursorTest::builder() + .source( + "lib.py", + r#" +class Outer: + class Inner: + def method(self, *, value: int) -> int: + return value +"#, + ) + .source( + "caller.py", + r#" +from lib import Outer + +Outer.Inner().method(value=42) +"#, + ) + .build(); + + assert_snapshot!(test.references(), @" + info[references]: Found 3 references + --> caller.py:4:22 + | + 4 | Outer.Inner().method(value=42) + | ----- + | + ::: lib.py:4:29 + | + 4 | def method(self, *, value: int) -> int: + | ----- + 5 | return value + | ----- "); } diff --git a/crates/ty_ide/src/references.rs b/crates/ty_ide/src/references.rs index d6a3b43275..5619fa278e 100644 --- a/crates/ty_ide/src/references.rs +++ b/crates/ty_ide/src/references.rs @@ -15,7 +15,7 @@ use crate::{Db, ReferenceKind, ReferenceTarget}; use rayon::prelude::*; use ruff_db::files::File; use ruff_db::parsed::parsed_module; -use ruff_python_ast::find_node::{CoveringNode, covering_node}; +use ruff_python_ast::find_node::CoveringNode; use ruff_python_ast::token::Tokens; use ruff_python_ast::{ self as ast, AnyNodeRef, @@ -117,7 +117,11 @@ pub(crate) fn references( let is_externally_visible_symbol = has_any_external_visible_definitions(db, &target_definitions); - let is_parameter = parameter_owner_is_externally_visible(db, &target_definitions); + let is_parameter = target_definitions.iter().any(|resolved| { + resolved + .definition() + .is_some_and(|definition| definition.kind(db).is_parameter_def()) + }); let search = LocalReferenceSearch { target_text, @@ -184,12 +188,20 @@ fn references_for_search( } let other_file = ProgramFile::new(db, other_file, program); - if has_fixture_target || is_externally_visible_symbol { + // A parameter's declaration and body uses must also be searched when the + // request starts from a keyword argument in another file. + if has_fixture_target + || is_externally_visible_symbol + || search.target_definitions.iter().any(|resolved| { + resolved + .definition() + .is_some_and(|definition| definition.program_file(db) == other_file) + }) + { references_for_file(db, other_file, search, mode) } else { - // Parameters are local by scope, but they can have cross-file references via keyword - // argument labels (e.g. `f(param=...)`). Handle this case with a narrow scan that only - // considers keyword arguments. + // Outside their defining files, parameters can be referenced via keyword + // argument labels (e.g. `f(param=...)`). Only consider keyword arguments here. references_for_keyword_arguments_in_file(db, other_file, search, mode) } }) @@ -336,75 +348,6 @@ pub(crate) fn has_any_external_visible_definitions( }) } -/// Determine whether a parameter's owning callable is externally visible. -/// -/// Parameters are local by scope, but their keyword-argument labels can appear across files -/// when the owning callable is visible outside of the current module. -fn parameter_owner_is_externally_visible( - db: &dyn Db, - target_definitions: &Definitions<'_>, -) -> bool { - target_definitions - .iter() - .any(|target| parameter_owner_is_externally_visible_for_target(db, target)) -} - -fn parameter_owner_is_externally_visible_for_target( - db: &dyn Db, - resolved: &ResolvedDefinition, -) -> bool { - let Some(definition) = resolved.definition() else { - return false; - }; - let parsed = parsed_module(db, definition.python_file(db)); - let target = definition.focus_range(db, &parsed.load(db)); - let module = parsed.load(db); - - let covering = covering_node(module.syntax().into(), target.range()); - let Ok(parameter_covering) = - covering.find_last(|node| matches!(node, AnyNodeRef::Parameter(_))) - else { - return false; - }; - - let mut owner: Option> = None; - let mut seen_owner = false; - let mut class_ancestor_found = false; - - // Heuristic: treat parameters as externally visible only when they belong to a top-level - // function or a method on a top-level class. Nested functions/classes are excluded to avoid - // broad, low-signal workspace scans. - for ancestor in parameter_covering.ancestors() { - if !seen_owner { - if matches!( - ancestor, - AnyNodeRef::StmtFunctionDef(_) | AnyNodeRef::ExprLambda(_) - ) { - owner = Some(ancestor); - seen_owner = true; - } - continue; - } - - match ancestor { - AnyNodeRef::StmtFunctionDef(_) | AnyNodeRef::ExprLambda(_) => { - // Nested functions or lambdas are not externally visible. - return false; - } - AnyNodeRef::StmtClassDef(_) => { - if class_ancestor_found { - // Nested classes are treated as not externally visible for now. - return false; - } - class_ancestor_found = true; - } - _ => {} - } - } - - matches!(owner, Some(AnyNodeRef::StmtFunctionDef(_))) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum OccurrenceKind { /// An identifier that references a symbol.