Skip to content

Commit d59282e

Browse files
authored
[ty] render docstrings in hover (#19882)
This PR has several components: * Introduce a Docstring String wrapper type that has render_plaintext and render_markdown methods, to force docstring handlers to pick a rendering format * Implement [PEP-257](https://peps.python.org/pep-0257/) docstring trimming for it * The markdown rendering just renders the content in a plaintext codeblock for now (followup work) * Introduce a `DefinitionsOrTargets` type representing the partial evaluation of `GotoTarget::get_definition_targets` to ideally stop at getting `ResolvedDefinitions` * Add `declaration_targets`, `definition_targets`, and `docstring` methods to `DefinitionsOrTargets` for the 3 usecases we have for this operation * `docstring` is of course the key addition here, it uses the same basic logic that `signature_help` was using: first check the goto-declaration for docstrings, then check the goto-definition for docstrings. * Refactor `signature_help` to use the new APIs instead of implementing it itself * Not fixed in this PR: an issue I found where `signature_help` will erroneously cache docs between functions that have the same type (hover docs don't have this bug) * A handful of new tests and additions to tests to add docstrings in various places and see which get caught Examples of it working with stdlib, third party, and local definitions: <img width="597" height="120" alt="Screenshot 2025-08-12 at 2 13 55 PM" src="https://github.com/user-attachments/assets/eae54efd-882e-4b50-b5b4-721595224232" /> <img width="598" height="281" alt="Screenshot 2025-08-12 at 2 14 06 PM" src="https://github.com/user-attachments/assets/5c9740d5-a06b-4c22-9349-da6eb9a9ba5a" /> <img width="327" height="180" alt="Screenshot 2025-08-12 at 2 14 18 PM" src="https://github.com/user-attachments/assets/3b5647b9-2cdd-4c5b-bb7d-da23bff1bcb5" /> Notably modules don't work yet (followup work): <img width="224" height="83" alt="Screenshot 2025-08-12 at 2 14 37 PM" src="https://github.com/user-attachments/assets/7e9dcb70-a10e-46d9-a85c-9fe52c3b7e7b" /> Notably we don't show docs for an item if you hover its actual definition (followup work, but also, not the most important): <img width="324" height="69" alt="Screenshot 2025-08-12 at 2 16 54 PM" src="https://github.com/user-attachments/assets/d4ddcdd8-c3fc-4120-ac93-cefdf57933b4" />
1 parent e12747a commit d59282e

File tree

11 files changed

+1025
-220
lines changed

11 files changed

+1025
-220
lines changed

crates/ty_ide/src/docstring.rs

Lines changed: 440 additions & 25 deletions
Large diffs are not rendered by default.

crates/ty_ide/src/goto.rs

Lines changed: 163 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::docstring::Docstring;
12
pub use crate::goto_declaration::goto_declaration;
23
pub use crate::goto_definition::goto_definition;
34
pub use crate::goto_type_definition::goto_type_definition;
@@ -146,6 +147,94 @@ pub(crate) enum GotoTarget<'a> {
146147
},
147148
}
148149

150+
/// The resolved definitions for a `GotoTarget`
151+
#[derive(Debug, Clone)]
152+
pub(crate) enum DefinitionsOrTargets<'db> {
153+
/// We computed actual Definitions we can do followup queries on.
154+
Definitions(Vec<ResolvedDefinition<'db>>),
155+
/// We directly computed a navigation.
156+
///
157+
/// We can't get docs or usefully compute goto-definition for this.
158+
Targets(crate::NavigationTargets),
159+
}
160+
161+
impl<'db> DefinitionsOrTargets<'db> {
162+
/// Get the "goto-declaration" interpretation of this definition
163+
///
164+
/// In this case it basically returns exactly what was found.
165+
pub(crate) fn declaration_targets(
166+
self,
167+
db: &'db dyn crate::Db,
168+
) -> Option<crate::NavigationTargets> {
169+
match self {
170+
DefinitionsOrTargets::Definitions(definitions) => {
171+
definitions_to_navigation_targets(db, None, definitions)
172+
}
173+
DefinitionsOrTargets::Targets(targets) => Some(targets),
174+
}
175+
}
176+
177+
/// Get the "goto-definition" interpretation of this definition
178+
///
179+
/// In this case we apply stub-mapping to try to find the "real" implementation
180+
/// if the definition we have is found in a stub file.
181+
pub(crate) fn definition_targets(
182+
self,
183+
db: &'db dyn crate::Db,
184+
) -> Option<crate::NavigationTargets> {
185+
match self {
186+
DefinitionsOrTargets::Definitions(definitions) => {
187+
definitions_to_navigation_targets(db, Some(&StubMapper::new(db)), definitions)
188+
}
189+
DefinitionsOrTargets::Targets(targets) => Some(targets),
190+
}
191+
}
192+
193+
/// Get the docstring for this definition
194+
///
195+
/// Typically documentation only appears on implementations and not stubs,
196+
/// so this will check both the goto-declarations and goto-definitions (in that order)
197+
/// and return the first one found.
198+
pub(crate) fn docstring(self, db: &'db dyn crate::Db) -> Option<Docstring> {
199+
let definitions = match self {
200+
DefinitionsOrTargets::Definitions(definitions) => definitions,
201+
// Can't find docs for these
202+
// (make more cases DefinitionOrTargets::Definitions to get more docs!)
203+
DefinitionsOrTargets::Targets(_) => return None,
204+
};
205+
for definition in &definitions {
206+
// TODO: get docstrings for modules
207+
let ResolvedDefinition::Definition(definition) = definition else {
208+
continue;
209+
};
210+
// First try to get the docstring from the original definition
211+
let original_docstring = definition.docstring(db);
212+
213+
// If we got a docstring from the original definition, use it
214+
if let Some(docstring) = original_docstring {
215+
return Some(Docstring::new(docstring));
216+
}
217+
}
218+
219+
// If the definition is located within a stub file and no docstring
220+
// is present, try to map the symbol to an implementation file and extract
221+
// the docstring from that location.
222+
let stub_mapper = StubMapper::new(db);
223+
224+
// Try to find the corresponding implementation definition
225+
for mapped_definition in stub_mapper.map_definitions(definitions) {
226+
// TODO: get docstrings for modules
227+
if let ResolvedDefinition::Definition(impl_definition) = mapped_definition {
228+
if let Some(impl_docstring) = impl_definition.docstring(db) {
229+
return Some(Docstring::new(impl_docstring));
230+
}
231+
}
232+
}
233+
234+
None
235+
}
236+
}
237+
149238
impl GotoTarget<'_> {
150239
pub(crate) fn inferred_type<'db>(&self, model: &SemanticModel<'db>) -> Option<Type<'db>> {
151240
let ty = match self {
@@ -173,79 +262,86 @@ impl GotoTarget<'_> {
173262
Some(ty)
174263
}
175264

176-
/// Gets the navigation ranges for this goto target.
177-
/// If a stub mapper is provided, definitions from stub files will be mapped to
178-
/// their corresponding source file implementations. The `alias_resolution`
179-
/// parameter controls whether import aliases (i.e. "x" in "from a import b as x") are
180-
/// resolved or returned as is. We want to resolve them in some cases (like
181-
/// "goto declaration") but not in others (like find references or rename).
182-
pub(crate) fn get_definition_targets(
265+
/// Gets the definitions for this goto target.
266+
///
267+
/// The `alias_resolution` parameter controls whether import aliases
268+
/// (i.e. "x" in "from a import b as x") are resolved or returned as is.
269+
/// We want to resolve them in some cases (like "goto declaration") but not in others
270+
/// (like find references or rename).
271+
///
272+
///
273+
/// Ideally this would always return `DefinitionsOrTargets::Definitions`
274+
/// as this is more useful for doing stub mapping (goto-definition) and
275+
/// retrieving docstrings. However for now some cases are stubbed out
276+
/// as just returning a raw `NavigationTarget`.
277+
pub(crate) fn get_definition_targets<'db>(
183278
&self,
184279
file: ruff_db::files::File,
185-
db: &dyn crate::Db,
186-
stub_mapper: Option<&StubMapper>,
280+
db: &'db dyn crate::Db,
187281
alias_resolution: ImportAliasResolution,
188-
) -> Option<crate::NavigationTargets> {
282+
) -> Option<DefinitionsOrTargets<'db>> {
189283
use crate::NavigationTarget;
190284
use ruff_python_ast as ast;
191285

192286
match self {
193287
GotoTarget::Expression(expression) => match expression {
194-
ast::ExprRef::Name(name) => definitions_to_navigation_targets(
195-
db,
196-
stub_mapper,
288+
ast::ExprRef::Name(name) => Some(DefinitionsOrTargets::Definitions(
197289
definitions_for_name(db, file, name),
198-
),
199-
ast::ExprRef::Attribute(attribute) => definitions_to_navigation_targets(
200-
db,
201-
stub_mapper,
290+
)),
291+
ast::ExprRef::Attribute(attribute) => Some(DefinitionsOrTargets::Definitions(
202292
ty_python_semantic::definitions_for_attribute(db, file, attribute),
203-
),
293+
)),
204294
_ => None,
205295
},
206296

207297
// For already-defined symbols, they are their own definitions
208298
GotoTarget::FunctionDef(function) => {
209299
let range = function.name.range;
210-
Some(crate::NavigationTargets::single(NavigationTarget {
211-
file,
212-
focus_range: range,
213-
full_range: function.range(),
214-
}))
300+
Some(DefinitionsOrTargets::Targets(
301+
crate::NavigationTargets::single(NavigationTarget {
302+
file,
303+
focus_range: range,
304+
full_range: function.range(),
305+
}),
306+
))
215307
}
216308

217309
GotoTarget::ClassDef(class) => {
218310
let range = class.name.range;
219-
Some(crate::NavigationTargets::single(NavigationTarget {
220-
file,
221-
focus_range: range,
222-
full_range: class.range(),
223-
}))
311+
Some(DefinitionsOrTargets::Targets(
312+
crate::NavigationTargets::single(NavigationTarget {
313+
file,
314+
focus_range: range,
315+
full_range: class.range(),
316+
}),
317+
))
224318
}
225319

226320
GotoTarget::Parameter(parameter) => {
227321
let range = parameter.name.range;
228-
Some(crate::NavigationTargets::single(NavigationTarget {
229-
file,
230-
focus_range: range,
231-
full_range: parameter.range(),
232-
}))
322+
Some(DefinitionsOrTargets::Targets(
323+
crate::NavigationTargets::single(NavigationTarget {
324+
file,
325+
focus_range: range,
326+
full_range: parameter.range(),
327+
}),
328+
))
233329
}
234330

235331
// For import aliases (offset within 'y' or 'z' in "from x import y as z")
236332
GotoTarget::ImportSymbolAlias {
237333
alias, import_from, ..
238334
} => {
239335
let symbol_name = alias.name.as_str();
240-
let definitions = definitions_for_imported_symbol(
241-
db,
242-
file,
243-
import_from,
244-
symbol_name,
245-
alias_resolution,
246-
);
247-
248-
definitions_to_navigation_targets(db, stub_mapper, definitions)
336+
Some(DefinitionsOrTargets::Definitions(
337+
definitions_for_imported_symbol(
338+
db,
339+
file,
340+
import_from,
341+
symbol_name,
342+
alias_resolution,
343+
),
344+
))
249345
}
250346

251347
GotoTarget::ImportModuleComponent {
@@ -260,42 +356,42 @@ impl GotoTarget<'_> {
260356
let target_module_name = components[..=*component_index].join(".");
261357

262358
// Try to resolve the module
263-
resolve_module_to_navigation_target(db, stub_mapper, &target_module_name)
359+
definitions_for_module(db, &target_module_name)
264360
}
265361

266362
// Handle import aliases (offset within 'z' in "import x.y as z")
267363
GotoTarget::ImportModuleAlias { alias } => {
268364
if alias_resolution == ImportAliasResolution::ResolveAliases {
269365
let full_module_name = alias.name.as_str();
270366
// Try to resolve the module
271-
resolve_module_to_navigation_target(db, stub_mapper, full_module_name)
367+
definitions_for_module(db, full_module_name)
272368
} else {
273369
let alias_range = alias.asname.as_ref().unwrap().range;
274-
Some(crate::NavigationTargets::single(NavigationTarget {
275-
file,
276-
focus_range: alias_range,
277-
full_range: alias.range(),
278-
}))
370+
Some(DefinitionsOrTargets::Targets(
371+
crate::NavigationTargets::single(NavigationTarget {
372+
file,
373+
focus_range: alias_range,
374+
full_range: alias.range(),
375+
}),
376+
))
279377
}
280378
}
281379

282380
// Handle keyword arguments in call expressions
283381
GotoTarget::KeywordArgument {
284382
keyword,
285383
call_expression,
286-
} => {
287-
let definitions =
288-
definitions_for_keyword_argument(db, file, keyword, call_expression);
289-
definitions_to_navigation_targets(db, stub_mapper, definitions)
290-
}
384+
} => Some(DefinitionsOrTargets::Definitions(
385+
definitions_for_keyword_argument(db, file, keyword, call_expression),
386+
)),
291387

292388
// For exception variables, they are their own definitions (like parameters)
293389
GotoTarget::ExceptVariable(except_handler) => {
294390
if let Some(name) = &except_handler.name {
295391
let range = name.range;
296-
Some(crate::NavigationTargets::single(NavigationTarget::new(
297-
file, range,
298-
)))
392+
Some(DefinitionsOrTargets::Targets(
393+
crate::NavigationTargets::single(NavigationTarget::new(file, range)),
394+
))
299395
} else {
300396
None
301397
}
@@ -305,9 +401,9 @@ impl GotoTarget<'_> {
305401
GotoTarget::PatternMatchRest(pattern_mapping) => {
306402
if let Some(rest_name) = &pattern_mapping.rest {
307403
let range = rest_name.range;
308-
Some(crate::NavigationTargets::single(NavigationTarget::new(
309-
file, range,
310-
)))
404+
Some(DefinitionsOrTargets::Targets(
405+
crate::NavigationTargets::single(NavigationTarget::new(file, range)),
406+
))
311407
} else {
312408
None
313409
}
@@ -317,9 +413,9 @@ impl GotoTarget<'_> {
317413
GotoTarget::PatternMatchAsName(pattern_as) => {
318414
if let Some(name) = &pattern_as.name {
319415
let range = name.range;
320-
Some(crate::NavigationTargets::single(NavigationTarget::new(
321-
file, range,
322-
)))
416+
Some(DefinitionsOrTargets::Targets(
417+
crate::NavigationTargets::single(NavigationTarget::new(file, range)),
418+
))
323419
} else {
324420
None
325421
}
@@ -656,11 +752,10 @@ pub(crate) fn find_goto_target(
656752
}
657753

658754
/// Helper function to resolve a module name and create a navigation target.
659-
fn resolve_module_to_navigation_target(
660-
db: &dyn crate::Db,
661-
stub_mapper: Option<&StubMapper>,
755+
fn definitions_for_module<'db>(
756+
db: &'db dyn crate::Db,
662757
module_name_str: &str,
663-
) -> Option<crate::NavigationTargets> {
758+
) -> Option<DefinitionsOrTargets<'db>> {
664759
use ty_python_semantic::{ModuleName, resolve_module};
665760

666761
if let Some(module_name) = ModuleName::new(module_name_str) {
@@ -670,7 +765,7 @@ fn resolve_module_to_navigation_target(
670765
module_file,
671766
TextRange::default(),
672767
))];
673-
return definitions_to_navigation_targets(db, stub_mapper, definitions);
768+
return Some(DefinitionsOrTargets::Definitions(definitions));
674769
}
675770
}
676771
}

crates/ty_ide/src/goto_declaration.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,9 @@ pub fn goto_declaration(
1818
let module = parsed_module(db, file).load(db);
1919
let goto_target = find_goto_target(&module, offset)?;
2020

21-
let declaration_targets = goto_target.get_definition_targets(
22-
file,
23-
db,
24-
None,
25-
ImportAliasResolution::ResolveAliases,
26-
)?;
21+
let declaration_targets = goto_target
22+
.get_definition_targets(file, db, ImportAliasResolution::ResolveAliases)?
23+
.declaration_targets(db)?;
2724

2825
Some(RangedValue {
2926
range: FileRange::new(file, goto_target.range()),

crates/ty_ide/src/goto_definition.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::goto::find_goto_target;
2-
use crate::stub_mapping::StubMapper;
32
use crate::{Db, NavigationTargets, RangedValue};
43
use ruff_db::files::{File, FileRange};
54
use ruff_db::parsed::parsed_module;
@@ -20,15 +19,9 @@ pub fn goto_definition(
2019
let module = parsed_module(db, file).load(db);
2120
let goto_target = find_goto_target(&module, offset)?;
2221

23-
// Create a StubMapper to map from stub files to source files
24-
let stub_mapper = StubMapper::new(db);
25-
26-
let definition_targets = goto_target.get_definition_targets(
27-
file,
28-
db,
29-
Some(&stub_mapper),
30-
ImportAliasResolution::ResolveAliases,
31-
)?;
22+
let definition_targets = goto_target
23+
.get_definition_targets(file, db, ImportAliasResolution::ResolveAliases)?
24+
.definition_targets(db)?;
3225

3326
Some(RangedValue {
3427
range: FileRange::new(file, goto_target.range()),

0 commit comments

Comments
 (0)