Skip to content

Commit 538ee52

Browse files
kinto0meta-codesync[bot]
authored andcommitted
Remove definition_at fallback from get_type_at
Summary: Remove the `definition_at` method and its usage as a fallback in `get_type_at`. All definition contexts are now handled explicitly through `identifier_at` pattern matching, making the scope-trace-based `definition_at_position` lookup unnecessary. The KeywordArgument handler now constructs keys directly from the refined parameter range, with a guard to return None when refinement fails (i.e., when the text at the definition range doesn't match the keyword argument name). Reviewed By: stroxler Differential Revision: D95331715 fbshipit-source-id: e01f2fdd79eca9c03a33da845f5e3bfda7ceafa4
1 parent 1631365 commit 538ee52

File tree

2 files changed

+16
-16
lines changed

2 files changed

+16
-16
lines changed

pyrefly/lib/state/lsp.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -841,19 +841,7 @@ impl<'a> Transaction<'a> {
841841
}
842842
}
843843

844-
fn definition_at(&self, handle: &Handle, position: TextSize) -> Option<Key> {
845-
self.get_bindings(handle)?
846-
.definition_at_position(position)
847-
.cloned()
848-
}
849-
850844
pub fn get_type_at(&self, handle: &Handle, position: TextSize) -> Option<Type> {
851-
// TODO(grievejia): Remove the usage of `definition_at()`: it doesn't reliably detect all
852-
// definitions.
853-
if let Some(key) = self.definition_at(handle, position) {
854-
return self.get_type(handle, &key);
855-
}
856-
857845
match self.identifier_at(handle, position) {
858846
Some(IdentifierWithContext {
859847
identifier: id,
@@ -991,8 +979,20 @@ impl<'a> Transaction<'a> {
991979
)
992980
.first()
993981
.and_then(|item| {
994-
self.definition_at(handle, item.definition_range.start())
995-
.and_then(|key| self.get_type(handle, &key))
982+
let code_at_range = item.module.code_at(item.definition_range);
983+
// If refinement failed, definition_range points to the callee itself,
984+
// not a matching parameter. In that case, return None.
985+
if code_at_range != identifier.id.as_str() {
986+
return None;
987+
}
988+
let name = Name::new(code_at_range);
989+
let id = Identifier::new(name.clone(), item.definition_range);
990+
let key = Key::Definition(ShortIdentifier::new(&id));
991+
let bindings = self.get_bindings(handle)?;
992+
if !bindings.is_valid_key(&key) {
993+
return None;
994+
}
995+
self.get_type(handle, &key)
996996
}),
997997
Some(IdentifierWithContext {
998998
identifier: _,

pyrefly/lib/test/lsp/hover_type.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,8 +546,8 @@ Hover Result: `int`
546546
);
547547
}
548548

549-
// todo(kylei): go-to definition currently finds the implementation in case of overload. it needs to be made smarter
550-
// for us to know the hover type
549+
// todo(kylei): When the callee's implementation uses *args/**kwargs, we can't refine the
550+
// keyword argument to a specific parameter. Ideally we'd resolve through the matched overload.
551551
#[test]
552552
fn kwarg_with_overload() {
553553
let code = r#"

0 commit comments

Comments
 (0)