From fc5321e0007fb96de01b51682d9374247a9dfe5c Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Thu, 21 Aug 2025 16:19:52 -0400 Subject: [PATCH] [ty] fix GotoTargets for keyword args in nested function calls (#20013) While implementing similar logic for initializers I noticed that this code appeared to be walking the ancestors in the wrong direction, and so if you have nested function calls it would always grab the outermost one instead of the closest-ancestor. The four copies of the test are because there's something really evil in our caching that can't seem to be demonstrated in our cursor testing framework, which I'm filing a followup for. --- crates/ty_ide/src/find_node.rs | 8 +- crates/ty_ide/src/goto_definition.rs | 152 +++++++++++++++++++++++++++ crates/ty_ide/src/selection_range.rs | 3 +- 3 files changed, 158 insertions(+), 5 deletions(-) diff --git a/crates/ty_ide/src/find_node.rs b/crates/ty_ide/src/find_node.rs index a56be5f7b9..74c1de060b 100644 --- a/crates/ty_ide/src/find_node.rs +++ b/crates/ty_ide/src/find_node.rs @@ -116,10 +116,10 @@ impl<'a> CoveringNode<'a> { Ok(self) } - /// Returns an iterator over the ancestor nodes, starting from the root - /// and ending with the covering node. - pub(crate) fn ancestors(&self) -> impl Iterator> + '_ { - self.nodes.iter().copied() + /// Returns an iterator over the ancestor nodes, starting with the node itself + /// and walking towards the root. + pub(crate) fn ancestors(&self) -> impl DoubleEndedIterator> + '_ { + self.nodes.iter().copied().rev() } /// Finds the index of the node that fully covers the range and diff --git a/crates/ty_ide/src/goto_definition.rs b/crates/ty_ide/src/goto_definition.rs index aaa02cd2a2..3c5edac5ec 100644 --- a/crates/ty_ide/src/goto_definition.rs +++ b/crates/ty_ide/src/goto_definition.rs @@ -630,6 +630,158 @@ class MyClass: ... "); } + /// goto-definition on a nested call using a keyword arg where both funcs have that arg name + /// + /// In this case they ultimately have different signatures. + #[test] + fn goto_definition_nested_keyword_arg1() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +def my_func(ab, y, z = None): ... +def my_other_func(ab, y): ... + +my_other_func(my_func(ab=5, y=2), 0) +my_func(my_other_func(ab=5, y=2), 0) +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r" + info[goto-definition]: Definition + --> main.py:2:13 + | + 2 | def my_func(ab, y, z = None): ... + | ^^ + 3 | def my_other_func(ab, y): ... + | + info: Source + --> main.py:5:23 + | + 3 | def my_other_func(ab, y): ... + 4 | + 5 | my_other_func(my_func(ab=5, y=2), 0) + | ^^ + 6 | my_func(my_other_func(ab=5, y=2), 0) + | + "); + } + + /// goto-definition on a nested call using a keyword arg where both funcs have that arg name + /// + /// In this case they ultimately have different signatures. + #[test] + fn goto_definition_nested_keyword_arg2() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +def my_func(ab, y, z = None): ... +def my_other_func(ab, y): ... + +my_other_func(my_func(ab=5, y=2), 0) +my_func(my_other_func(ab=5, y=2), 0) +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r" + info[goto-definition]: Definition + --> main.py:3:19 + | + 2 | def my_func(ab, y, z = None): ... + 3 | def my_other_func(ab, y): ... + | ^^ + 4 | + 5 | my_other_func(my_func(ab=5, y=2), 0) + | + info: Source + --> main.py:6:23 + | + 5 | my_other_func(my_func(ab=5, y=2), 0) + 6 | my_func(my_other_func(ab=5, y=2), 0) + | ^^ + | + "); + } + + /// goto-definition on a nested call using a keyword arg where both funcs have that arg name + /// + /// In this case they have identical signatures. + #[test] + fn goto_definition_nested_keyword_arg3() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +def my_func(ab, y): ... +def my_other_func(ab, y): ... + +my_other_func(my_func(ab=5, y=2), 0) +my_func(my_other_func(ab=5, y=2), 0) +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r" + info[goto-definition]: Definition + --> main.py:2:13 + | + 2 | def my_func(ab, y): ... + | ^^ + 3 | def my_other_func(ab, y): ... + | + info: Source + --> main.py:5:23 + | + 3 | def my_other_func(ab, y): ... + 4 | + 5 | my_other_func(my_func(ab=5, y=2), 0) + | ^^ + 6 | my_func(my_other_func(ab=5, y=2), 0) + | + "); + } + + /// goto-definition on a nested call using a keyword arg where both funcs have that arg name + /// + /// In this case they have identical signatures. + #[test] + fn goto_definition_nested_keyword_arg4() { + let test = CursorTest::builder() + .source( + "main.py", + r#" +def my_func(ab, y): ... +def my_other_func(ab, y): ... + +my_other_func(my_func(ab=5, y=2), 0) +my_func(my_other_func(ab=5, y=2), 0) +"#, + ) + .build(); + + assert_snapshot!(test.goto_definition(), @r" + info[goto-definition]: Definition + --> main.py:3:19 + | + 2 | def my_func(ab, y): ... + 3 | def my_other_func(ab, y): ... + | ^^ + 4 | + 5 | my_other_func(my_func(ab=5, y=2), 0) + | + info: Source + --> main.py:6:23 + | + 5 | my_other_func(my_func(ab=5, y=2), 0) + 6 | my_func(my_other_func(ab=5, y=2), 0) + | ^^ + | + "); + } + impl CursorTest { fn goto_definition(&self) -> String { let Some(targets) = goto_definition(&self.db, self.cursor.file, self.cursor.offset) diff --git a/crates/ty_ide/src/selection_range.rs b/crates/ty_ide/src/selection_range.rs index 9603ff39e7..139b8d433c 100644 --- a/crates/ty_ide/src/selection_range.rs +++ b/crates/ty_ide/src/selection_range.rs @@ -14,7 +14,8 @@ pub fn selection_range(db: &dyn Db, file: File, offset: TextSize) -> Vec