Skip to content

Commit ddd1275

Browse files
kinto0meta-codesync[bot]
authored andcommitted
go-to definition / hover on methods of .pyi
Summary: D80145015 implemented .py preference on go-to definition, but it did not work for attributes. that's because when we solve a type `a`, `a.attribute` doesn't do another find, it simply goes to the definition of a from solving which is always the .pyi. asuka implemented a fix for that [here](#1520) for hover docstrings. the intution is to do a second find on attribute definition lookup and look at the AST to find the corresponding definition. this works pretty well, and this diff stack generalizes it to definition. Reviewed By: connernilsen Differential Revision: D86814372 fbshipit-source-id: 8b7ed56d74071db55188d448f1eb76f91bcc612d
1 parent 078a467 commit ddd1275

File tree

4 files changed

+166
-3
lines changed

4 files changed

+166
-3
lines changed

pyrefly/lib/state/lsp.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use pyrefly_python::module::TextRangeWithModule;
3434
use pyrefly_python::module_name::ModuleName;
3535
use pyrefly_python::module_path::ModulePath;
3636
use pyrefly_python::module_path::ModulePathDetails;
37+
use pyrefly_python::module_path::ModuleStyle;
3738
use pyrefly_python::short_identifier::ShortIdentifier;
3839
use pyrefly_python::symbol_kind::SymbolKind;
3940
use pyrefly_python::sys_info::SysInfo;
@@ -79,6 +80,7 @@ use crate::state::ide::IntermediateDefinition;
7980
use crate::state::ide::import_regular_import_edit;
8081
use crate::state::ide::insert_import_edit;
8182
use crate::state::ide::key_to_intermediate_definition;
83+
use crate::state::lsp_attributes::AttributeContext;
8284
use crate::state::require::Require;
8385
use crate::state::semantic_tokens::SemanticTokenBuilder;
8486
use crate::state::semantic_tokens::SemanticTokensLegends;
@@ -1154,6 +1156,19 @@ impl<'a> Transaction<'a> {
11541156
) -> Option<(TextRangeWithModule, Option<TextRange>)> {
11551157
match definition {
11561158
AttrDefinition::FullyResolved(text_range_with_module_info) => {
1159+
// If prefer_pyi is false and the current module is a .pyi file,
1160+
// try to find the corresponding .py file
1161+
if !preference.prefer_pyi
1162+
&& text_range_with_module_info.module.path().is_interface()
1163+
&& let Some((exec_module, exec_range)) = self
1164+
.search_corresponding_py_module_for_attribute(
1165+
handle,
1166+
attr_name,
1167+
&text_range_with_module_info,
1168+
)
1169+
{
1170+
return Some((TextRangeWithModule::new(exec_module, exec_range), None));
1171+
}
11571172
Some((text_range_with_module_info, docstring_range))
11581173
}
11591174
AttrDefinition::PartiallyResolvedImportedModuleAttribute { module_name } => {
@@ -1168,6 +1183,39 @@ impl<'a> Transaction<'a> {
11681183
}
11691184
}
11701185

1186+
/// Find the .py definition for a corresponding .pyi definition by importing
1187+
/// and parsing the AST, looking for classes/functions.
1188+
fn search_corresponding_py_module_for_attribute(
1189+
&self,
1190+
request_handle: &Handle,
1191+
attr_name: &Name,
1192+
pyi_definition: &TextRangeWithModule,
1193+
) -> Option<(Module, TextRange)> {
1194+
let context = AttributeContext::from_module(&pyi_definition.module, pyi_definition.range)?;
1195+
let executable_handle = self
1196+
.import_handle_prefer_executable(request_handle, pyi_definition.module.name(), None)
1197+
.finding()?;
1198+
if executable_handle.path().style() != ModuleStyle::Executable {
1199+
return None;
1200+
}
1201+
let _ = self.get_exports(&executable_handle);
1202+
let executable_module = self.get_module_info(&executable_handle)?;
1203+
let ast = self.get_ast(&executable_handle).unwrap_or_else(|| {
1204+
Ast::parse(
1205+
executable_module.contents(),
1206+
executable_module.source_type(),
1207+
)
1208+
.0
1209+
.into()
1210+
});
1211+
let def_range = crate::state::lsp_attributes::definition_from_executable_ast(
1212+
ast.as_ref(),
1213+
&context,
1214+
attr_name,
1215+
)?;
1216+
Some((executable_module, def_range))
1217+
}
1218+
11711219
fn key_to_export(
11721220
&self,
11731221
handle: &Handle,
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
use pyrefly_python::ast::Ast;
9+
use pyrefly_python::module::Module;
10+
use ruff_python_ast::ModModule;
11+
use ruff_python_ast::Stmt;
12+
use ruff_text_size::Ranged;
13+
use ruff_text_size::TextRange;
14+
15+
#[derive(Debug, Clone)]
16+
pub(crate) enum AttributeKind {
17+
Function,
18+
Class,
19+
}
20+
21+
#[derive(Debug, Clone)]
22+
pub(crate) struct AttributeContext {
23+
pub(crate) parent_classes: Vec<ruff_python_ast::name::Name>,
24+
pub(crate) kind: AttributeKind,
25+
}
26+
27+
impl AttributeContext {
28+
pub(crate) fn from_module(
29+
module: &Module,
30+
target_range: TextRange,
31+
) -> Option<AttributeContext> {
32+
let ast = Ast::parse(module.contents(), module.source_type()).0;
33+
let mut parents = Vec::new();
34+
Self::from_body(ast.body.as_slice(), &mut parents, target_range)
35+
}
36+
37+
fn from_body(
38+
body: &[Stmt],
39+
parents: &mut Vec<ruff_python_ast::name::Name>,
40+
target_range: TextRange,
41+
) -> Option<AttributeContext> {
42+
for stmt in body {
43+
match stmt {
44+
Stmt::ClassDef(class_def) => {
45+
parents.push(class_def.name.id.clone());
46+
if let Some(ctx) = Self::from_body(&class_def.body, parents, target_range) {
47+
return Some(ctx);
48+
}
49+
parents.pop();
50+
if class_def.range().contains_range(target_range) {
51+
return Some(AttributeContext {
52+
parent_classes: parents.clone(),
53+
kind: AttributeKind::Class,
54+
});
55+
}
56+
}
57+
Stmt::FunctionDef(_) if stmt.range().contains_range(target_range) => {
58+
return Some(AttributeContext {
59+
parent_classes: parents.clone(),
60+
kind: AttributeKind::Function,
61+
});
62+
}
63+
_ => {}
64+
}
65+
}
66+
None
67+
}
68+
}
69+
70+
pub(crate) fn definition_from_executable_ast(
71+
ast: &ModModule,
72+
context: &AttributeContext,
73+
attr_name: &ruff_python_ast::name::Name,
74+
) -> Option<TextRange> {
75+
let mut body = ast.body.as_slice();
76+
for class_name in &context.parent_classes {
77+
let class_def = body.iter().find_map(|stmt| match stmt {
78+
Stmt::ClassDef(class_def) if &class_def.name.id == class_name => Some(class_def),
79+
_ => None,
80+
})?;
81+
body = class_def.body.as_slice();
82+
}
83+
match context.kind {
84+
AttributeKind::Function => definition_from_function(body, attr_name),
85+
AttributeKind::Class => definition_from_class(body, attr_name),
86+
}
87+
}
88+
89+
fn definition_from_function(
90+
body: &[Stmt],
91+
attr_name: &ruff_python_ast::name::Name,
92+
) -> Option<TextRange> {
93+
for stmt in body {
94+
if let Stmt::FunctionDef(func_def) = stmt
95+
&& &func_def.name.id == attr_name
96+
{
97+
return Some(func_def.name.range);
98+
}
99+
}
100+
None
101+
}
102+
103+
fn definition_from_class(
104+
body: &[Stmt],
105+
attr_name: &ruff_python_ast::name::Name,
106+
) -> Option<TextRange> {
107+
for stmt in body {
108+
if let Stmt::ClassDef(class_def) = stmt
109+
&& &class_def.name.id == attr_name
110+
{
111+
return Some(class_def.name.range);
112+
}
113+
}
114+
None
115+
}

pyrefly/lib/state/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub mod ide;
1212
pub mod load;
1313
pub mod loader;
1414
pub mod lsp;
15+
pub mod lsp_attributes;
1516
pub mod memory;
1617
pub mod notebook;
1718
pub mod require;

pyrefly/lib/test/lsp/lsp_interaction/definition.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,8 @@ fn definition_in_builtins() {
170170
);
171171
}
172172

173-
// todo(kylei): definition of an attribute of a pyi should still point to py
174173
#[test]
175-
fn definition_on_attr_of_pyi() {
174+
fn definition_on_attr_of_pyi_goes_to_py() {
176175
let root = get_test_files_root();
177176
let mut interaction = LspInteraction::new();
178177
interaction.set_root(root.path().to_path_buf());
@@ -184,7 +183,7 @@ fn definition_on_attr_of_pyi() {
184183
interaction.server.definition(file, 7, 4);
185184
interaction
186185
.client
187-
.expect_definition_response_from_root("attributes_of_py/lib.pyi", 6, 8, 6, 9);
186+
.expect_definition_response_from_root("attributes_of_py/lib.py", 7, 8, 7, 9);
188187
interaction.shutdown();
189188
}
190189

0 commit comments

Comments
 (0)