mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:24:57 +00:00
[ty] resolve docstrings for modules (#19898)
This also reintroduces the `ResolvedDefinition::Module` variant because reverse-engineering it in several places is a bit confusing. In an ideal world we wouldn't have `ResolvedDefinition::FileWithRange` as it kinda kills the ability to do richer analysis, so I want to chip away at its scope wherever I can (currently it's used to point at asname parts of import statements when doing `ImportAliasResolution::PreserveAliases`, and also keyword arguments). This also makes a kind of odd change to allow a hover to *only* produce a docstring. This works around an oddity where hovering over a module name in an import fails to resolve to a `ty` even though hovering over uses of that imported name *does*. The two fixed tests reflect the two interesting cases here.
This commit is contained in:
parent
9f6146a13d
commit
f0b03c3e86
4 changed files with 93 additions and 49 deletions
|
@ -7,7 +7,6 @@ use std::borrow::Cow;
|
|||
|
||||
use crate::find_node::covering_node;
|
||||
use crate::stub_mapping::StubMapper;
|
||||
use ruff_db::files::FileRange;
|
||||
use ruff_db::parsed::ParsedModuleRef;
|
||||
use ruff_python_ast::{self as ast, AnyNodeRef};
|
||||
use ruff_python_parser::TokenKind;
|
||||
|
@ -203,15 +202,8 @@ impl<'db> DefinitionsOrTargets<'db> {
|
|||
DefinitionsOrTargets::Targets(_) => return None,
|
||||
};
|
||||
for definition in &definitions {
|
||||
// TODO: get docstrings for modules
|
||||
let ResolvedDefinition::Definition(definition) = definition else {
|
||||
continue;
|
||||
};
|
||||
// First try to get the docstring from the original definition
|
||||
let original_docstring = definition.docstring(db);
|
||||
|
||||
// If we got a docstring from the original definition, use it
|
||||
if let Some(docstring) = original_docstring {
|
||||
if let Some(docstring) = definition.docstring(db) {
|
||||
return Some(Docstring::new(docstring));
|
||||
}
|
||||
}
|
||||
|
@ -222,12 +214,9 @@ impl<'db> DefinitionsOrTargets<'db> {
|
|||
let stub_mapper = StubMapper::new(db);
|
||||
|
||||
// Try to find the corresponding implementation definition
|
||||
for mapped_definition in stub_mapper.map_definitions(definitions) {
|
||||
// TODO: get docstrings for modules
|
||||
if let ResolvedDefinition::Definition(impl_definition) = mapped_definition {
|
||||
if let Some(impl_docstring) = impl_definition.docstring(db) {
|
||||
return Some(Docstring::new(impl_docstring));
|
||||
}
|
||||
for definition in stub_mapper.map_definitions(definitions) {
|
||||
if let Some(docstring) = definition.docstring(db) {
|
||||
return Some(Docstring::new(docstring));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -703,6 +692,10 @@ fn convert_resolved_definitions_to_targets(
|
|||
full_range: full_range.range(),
|
||||
}
|
||||
}
|
||||
ty_python_semantic::ResolvedDefinition::Module(file) => {
|
||||
// For modules, navigate to the start of the file
|
||||
crate::NavigationTarget::new(file, TextRange::default())
|
||||
}
|
||||
ty_python_semantic::ResolvedDefinition::FileWithRange(file_range) => {
|
||||
// For file ranges, navigate to the specific range within the file
|
||||
crate::NavigationTarget::new(file_range.file(), file_range.range())
|
||||
|
@ -761,11 +754,9 @@ fn definitions_for_module<'db>(
|
|||
if let Some(module_name) = ModuleName::new(module_name_str) {
|
||||
if let Some(resolved_module) = resolve_module(db, &module_name) {
|
||||
if let Some(module_file) = resolved_module.file(db) {
|
||||
let definitions = vec![ResolvedDefinition::FileWithRange(FileRange::new(
|
||||
module_file,
|
||||
TextRange::default(),
|
||||
))];
|
||||
return Some(DefinitionsOrTargets::Definitions(definitions));
|
||||
return Some(DefinitionsOrTargets::Definitions(vec![
|
||||
ResolvedDefinition::Module(module_file),
|
||||
]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -20,7 +20,7 @@ pub fn hover(db: &dyn Db, file: File, offset: TextSize) -> Option<RangedValue<Ho
|
|||
}
|
||||
|
||||
let model = SemanticModel::new(db, file);
|
||||
let ty = goto_target.inferred_type(&model)?;
|
||||
let ty = goto_target.inferred_type(&model).map(HoverContent::Type);
|
||||
let docs = goto_target
|
||||
.get_definition_targets(
|
||||
file,
|
||||
|
@ -29,12 +29,20 @@ pub fn hover(db: &dyn Db, file: File, offset: TextSize) -> Option<RangedValue<Ho
|
|||
)
|
||||
.and_then(|definitions| definitions.docstring(db))
|
||||
.map(HoverContent::Docstring);
|
||||
tracing::debug!("Inferred type of covering node is {}", ty.display(db));
|
||||
|
||||
if let Some(HoverContent::Type(ty)) = ty {
|
||||
tracing::debug!("Inferred type of covering node is {}", ty.display(db));
|
||||
}
|
||||
|
||||
// TODO: Render the symbol's signature instead of just its type.
|
||||
let mut contents = vec![HoverContent::Type(ty)];
|
||||
let mut contents = Vec::new();
|
||||
contents.extend(ty);
|
||||
contents.extend(docs);
|
||||
|
||||
if contents.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(RangedValue {
|
||||
range: FileRange::new(file, goto_target.range()),
|
||||
value: Hover { contents },
|
||||
|
@ -630,9 +638,21 @@ mod tests {
|
|||
|
||||
assert_snapshot!(test.hover(), @r"
|
||||
<module 'lib'>
|
||||
---------------------------------------------
|
||||
The cool lib_py module!
|
||||
|
||||
Wow this module rocks.
|
||||
|
||||
---------------------------------------------
|
||||
```python
|
||||
<module 'lib'>
|
||||
```
|
||||
---
|
||||
```text
|
||||
The cool lib_py module!
|
||||
|
||||
Wow this module rocks.
|
||||
|
||||
```
|
||||
---------------------------------------------
|
||||
info[hover]: Hovered content is
|
||||
|
@ -672,7 +692,31 @@ mod tests {
|
|||
)
|
||||
.unwrap();
|
||||
|
||||
assert_snapshot!(test.hover(), @"Hover provided no content");
|
||||
assert_snapshot!(test.hover(), @r"
|
||||
The cool lib_py module!
|
||||
|
||||
Wow this module rocks.
|
||||
|
||||
---------------------------------------------
|
||||
```text
|
||||
The cool lib_py module!
|
||||
|
||||
Wow this module rocks.
|
||||
|
||||
```
|
||||
---------------------------------------------
|
||||
info[hover]: Hovered content is
|
||||
--> main.py:2:20
|
||||
|
|
||||
2 | import lib
|
||||
| ^^-
|
||||
| | |
|
||||
| | Cursor offset
|
||||
| source
|
||||
3 |
|
||||
4 | lib
|
||||
|
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
@ -113,7 +113,14 @@ impl<'db> Definition<'db> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Extract a docstring from a function or class body.
|
||||
/// Get the module-level docstring for the given file
|
||||
pub(crate) fn module_docstring(db: &dyn Db, file: File) -> Option<String> {
|
||||
let module = parsed_module(db, file).load(db);
|
||||
docstring_from_body(module.suite())
|
||||
.map(|docstring_expr| docstring_expr.value.to_str().to_owned())
|
||||
}
|
||||
|
||||
/// Extract a docstring from a function, module, or class body.
|
||||
fn docstring_from_body(body: &[ast::Stmt]) -> Option<&ast::ExprStringLiteral> {
|
||||
let stmt = body.first()?;
|
||||
// Require the docstring to be a standalone expression.
|
||||
|
|
|
@ -973,12 +973,11 @@ mod resolve_definition {
|
|||
use ruff_db::files::{File, FileRange};
|
||||
use ruff_db::parsed::{ParsedModuleRef, parsed_module};
|
||||
use ruff_python_ast as ast;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
use rustc_hash::FxHashSet;
|
||||
use tracing::trace;
|
||||
|
||||
use crate::module_resolver::file_to_module;
|
||||
use crate::semantic_index::definition::{Definition, DefinitionKind};
|
||||
use crate::semantic_index::definition::{Definition, DefinitionKind, module_docstring};
|
||||
use crate::semantic_index::scope::{NodeWithScopeKind, ScopeId};
|
||||
use crate::semantic_index::{global_scope, place_table, semantic_index, use_def_map};
|
||||
use crate::{Db, ModuleName, resolve_module, resolve_real_module};
|
||||
|
@ -992,6 +991,8 @@ mod resolve_definition {
|
|||
pub enum ResolvedDefinition<'db> {
|
||||
/// The import resolved to a specific definition within a module
|
||||
Definition(Definition<'db>),
|
||||
/// The import resolved to an entire module
|
||||
Module(File),
|
||||
/// The import resolved to a file with a specific range
|
||||
FileWithRange(FileRange),
|
||||
}
|
||||
|
@ -1000,9 +1001,18 @@ mod resolve_definition {
|
|||
fn file(&self, db: &'db dyn Db) -> File {
|
||||
match self {
|
||||
ResolvedDefinition::Definition(definition) => definition.file(db),
|
||||
ResolvedDefinition::Module(file) => *file,
|
||||
ResolvedDefinition::FileWithRange(file_range) => file_range.file(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn docstring(&self, db: &'db dyn Db) -> Option<String> {
|
||||
match self {
|
||||
ResolvedDefinition::Definition(definition) => definition.docstring(db),
|
||||
ResolvedDefinition::Module(file) => module_docstring(db, *file),
|
||||
ResolvedDefinition::FileWithRange(_) => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve import definitions to their targets.
|
||||
|
@ -1070,10 +1080,7 @@ mod resolve_definition {
|
|||
|
||||
// For simple imports like "import os", we want to navigate to the module itself.
|
||||
// Return the module file directly instead of trying to find definitions within it.
|
||||
vec![ResolvedDefinition::FileWithRange(FileRange::new(
|
||||
module_file,
|
||||
TextRange::default(),
|
||||
))]
|
||||
vec![ResolvedDefinition::Module(module_file)]
|
||||
}
|
||||
|
||||
DefinitionKind::ImportFrom(import_from_def) => {
|
||||
|
@ -1283,24 +1290,19 @@ mod resolve_definition {
|
|||
}
|
||||
trace!("Built Definition Path: {path:?}");
|
||||
}
|
||||
ResolvedDefinition::FileWithRange(file_range) => {
|
||||
return if file_range.range() == TextRange::default() {
|
||||
trace!(
|
||||
"Found module mapping: {} => {}",
|
||||
stub_file.path(db),
|
||||
real_file.path(db)
|
||||
);
|
||||
// This is just a reference to a module, no need to do paths
|
||||
Some(vec![ResolvedDefinition::FileWithRange(FileRange::new(
|
||||
real_file,
|
||||
TextRange::default(),
|
||||
))])
|
||||
} else {
|
||||
// Not yet implemented -- in this case we want to recover something like a Definition
|
||||
// and build a Definition Path, but this input is a bit too abstract for now.
|
||||
trace!("Found arbitrary FileWithRange by stub mapping, giving up");
|
||||
None
|
||||
};
|
||||
ResolvedDefinition::Module(_) => {
|
||||
trace!(
|
||||
"Found module mapping: {} => {}",
|
||||
stub_file.path(db),
|
||||
real_file.path(db)
|
||||
);
|
||||
return Some(vec![ResolvedDefinition::Module(real_file)]);
|
||||
}
|
||||
ResolvedDefinition::FileWithRange(_) => {
|
||||
// Not yet implemented -- in this case we want to recover something like a Definition
|
||||
// and build a Definition Path, but this input is a bit too abstract for now.
|
||||
trace!("Found arbitrary FileWithRange while stub mapping, giving up");
|
||||
return None;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue