[ty] render docstrings in hover by Gankra · Pull Request #19882 · astral-sh/ruff

Gankra · 2025-08-12T18:18:04Z

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 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:

Notably modules don't work yet (followup work):

Notably we don't show docs for an item if you hover its actual definition (followup work, but also, not the most important):

github-actions · 2025-08-12T18:19:55Z

Diagnostic diff on typing conformance tests

Changes were detected when running ty on typing conformance tests

--- old-output.txt	2025-08-13 14:57:30.986477337 +0000
+++ new-output.txt	2025-08-13 14:57:31.053477542 +0000
@@ -1,5 +1,5 @@
 WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(b5023)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(244b3)): execute: too many cycle iterations`
 _directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
 _directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
 _directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__`

github-actions · 2025-08-12T18:22:06Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

Gankra · 2025-08-12T18:28:19Z

crates/ty_ide/src/hover.rs

+        x = MyCla<CURSOR>ss(0)
+        "#,
+        );
+
+        assert_snapshot!(test.hover(), @r"
+        <class 'MyClass'>
+        ---------------------------------------------
+        This is such a great class!!


This result is a disappointment but a pre-existing issue that we don't really distinguish a class from an invocation of the initializer (future work).

Gankra · 2025-08-12T18:29:17Z

crates/ty_ide/src/hover.rs

            r#"
-            def test(a: int): ...
+            def test(ab: int):
+                """my cool test

-            test(a<CURSOR>= 123)
+                Args:
+                    ab: a nice little integer
+                """
+                return 0
+
+            test(a<CURSOR>b= 123)
            "#,


This doesn't change the result at all but I changed this test in case one day our hover gets smart enough to see and render these docs.

Gankra · 2025-08-12T18:30:26Z

crates/ty_ide/src/hover.rs


-            def foo(a, b): ...
+            def foo(a, b):
+                """The foo function"""
+                return 0

-            def bar(a, b): ...
+            def bar(a, b):
+                """The bar function"""
+                return 1

            if random.choice([True, False]):
                a = foo


Similarly this doesn't change anything, but I added docs just in case something got smarter

Gankra · 2025-08-12T18:31:18Z

crates/ty_ide/src/hover.rs


-        test.write_file("lib.py", "a = 10").unwrap();
+        test.write_file(
+            "lib.py",
+            r"
+        '''
+        The cool lib_py module!
+
+        Wow this module rocks.


Future work, this should render eventually.

Gankra · 2025-08-12T18:33:29Z

crates/ty_ide/src/hover.rs

+        let mut test = cursor_test(
+            r#"
+            import li<CURSOR>b
+
+            lib
+            "#,
+        );
+
+        test.write_file(
+            "lib.py",
+            r"
+        '''
+        The cool lib_py module!


This one's even worse and produces no result because apparently hovering an import doesn't yield the module (even though I fixed goto-def on it in a previous PR). I assume this is because hover currently insists it needs the type of the hovered item, not the def/decl.

Gankra · 2025-08-12T18:34:12Z

crates/ty_ide/src/hover.rs

+                '''
+                    My cool func
+
+                    Args:
+                        a: hopefully a string, right?!   
+                '''
                if a is not None:
                    print(a<CURSOR>)


Another "maybe one day" change

MichaReiser

This is excellent. Thank you for working on this. I only have a few detail comments that are at your own discretion.

MichaReiser · 2025-08-13T07:33:52Z

crates/ty_ide/src/docstring.rs

-    // NumPy-style docstrings
-    param_docs.extend(extract_numpy_style_params(docstring));
+        // Google-style docstrings
+        param_docs.extend(extract_google_style_params(&self.0));


I know this is pre-existing code, so I'm fine deferring it but I wonder if it makes sense to short-circuit if we parsed parameters by at least one style because most code will only use one convention.

We actually have a test checked in for the mixed behaviour, so i guess @UnboundVariable thought it was something that we'll see in the wild?

crates/ty_ide/src/docstring.rs

MichaReiser · 2025-08-13T07:40:08Z

crates/ty_ide/src/docstring.rs

+/// Normalizes tabs and trims a docstring as specified in PEP-0257
+///
+/// See: <https://peps.python.org/pep-0257/#handling-docstring-indentation>
+fn documentation_trim(docs: &str) -> String {


Yay, our second implementation of this algorithm (it makes sense for them to be different):

ruff/crates/ruff_python_formatter/src/string/docstring.rs

Lines 27 to 114 in 9ae698f

/// Format a docstring by trimming whitespace and adjusting the indentation.

///

/// Summary of changes we make:

/// * Normalize the string like all other strings

/// * Ignore docstring that have an escaped newline

/// * Trim all trailing whitespace, except for a chaperone space that avoids quotes or backslashes

/// in the last line.

/// * Trim leading whitespace on the first line, again except for a chaperone space

/// * If there is only content in the first line and after that only whitespace, collapse the

/// docstring into one line

/// * Adjust the indentation (see below)

///

/// # Docstring indentation

///

/// Unlike any other string, like black we change the indentation of docstring lines.

///

/// We want to preserve the indentation inside the docstring relative to the suite statement/block

/// indent that the docstring statement is in, but also want to apply the change of the outer

/// indentation in the docstring, e.g.

/// ```python

/// def sparkle_sky():

/// """Make a pretty sparkly sky.

/// * * ✨ *. .

/// * * ✨ .

/// . * . ✨ * . .

/// """

/// ```

/// should become

/// ```python

/// def sparkle_sky():

/// """Make a pretty sparkly sky.

/// * * ✨ *. .

/// * * ✨ .

/// . * . ✨ * . .

/// """

/// ```

/// We can't compute the full indentation here since we don't know what the block indent of

/// the doc comment will be yet and which we can only have added by formatting each line

/// separately with a hard line break. This means we need to strip shared indentation from

/// docstring while preserving the in-docstring bigger-than-suite-statement indentation. Example:

/// ```python

/// def f():

/// """first line

/// line a

/// line b

/// """

/// ```

/// The docstring indentation is 2, the block indents will change this to 4 (but we can't

/// determine this at this point). The indentation of line a is 2, so we trim ` line a`

/// to `line a`. For line b it's 5, so we trim it to `line b` and pad with 5-2=3 spaces to

/// ` line b`. The closing quotes, being on their own line, are stripped get only the

/// default indentation. Fully formatted:

/// ```python

/// def f():

/// """first line

/// line a

/// line b

/// """

/// ```

///

/// Tabs are counted by padding them to the next multiple of 8 according to

/// [`str.expandtabs`](https://docs.python.org/3/library/stdtypes.html#str.expandtabs).

///

/// Additionally, if any line in the docstring has less indentation than the docstring

/// (effectively a negative indentation wrt. to the current level), we pad all lines to the

/// level of the docstring with spaces.

/// ```python

/// def f():

/// """first line

/// line a

/// line b

/// line c

/// """

/// ```

/// Here line a is 3 columns negatively indented, so we pad all lines by an extra 3 spaces:

/// ```python

/// def f():

/// """first line

/// line a

/// line b

/// line c

/// """

/// ```

/// The indentation is rewritten to all-spaces when using [`IndentStyle::Space`].

/// The formatter preserves tab-indentations when using [`IndentStyle::Tab`], but doesn't convert

/// `indent-width * spaces` to tabs because doing so could break ASCII art and other docstrings

/// that use spaces for alignment.

pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> FormatResult<()> {

In retrospect I really should have checked because of course we do 😓

MichaReiser · 2025-08-13T07:43:23Z

crates/ty_ide/src/goto.rs

+    ) -> Option<crate::NavigationTargets> {
+        match self {
+            DefinitionsOrTargets::Definitions(definitions) => {
+                definitions_to_navigation_targets(db, Some(&StubMapper::new(db)), definitions)


Nit: The stub mapper type still feels a bit overkill to me. It feels like it could easily be replaced by an enum with two variants (map, don't map) and a method (that takes db as an argument) that does the mapping.

Maybe something for a follow up PR?

I agree, I have several followup refactors planned here.

crates/ty_ide/src/goto.rs

crates/ty_ide/src/hover.rs

* main: [ty] Add precise inference for indexing, slicing and unpacking `NamedTuple` instances (#19560) Add rule code to GitLab description (#19896) [ty] render docstrings in hover (#19882) [ty] simplify return type of place_from_declarations (#19884) [ty] Various minor cleanups to tuple internals (#19891) [ty] Improve `sys.version_info` special casing (#19894)

…aints * dcreager/inferrable: (65 commits) this was right after all mark typevars inferrable as we go fix tests fix inference of constrained typevars [ty] Add precise inference for indexing, slicing and unpacking `NamedTuple` instances (#19560) Add rule code to GitLab description (#19896) [ty] render docstrings in hover (#19882) [ty] simplify return type of place_from_declarations (#19884) [ty] Various minor cleanups to tuple internals (#19891) [ty] Improve `sys.version_info` special casing (#19894) Don't cache files with diagnostics (#19869) [ty] support recursive type aliases (#19805) [ty] Remove unsafe `salsa::Update` implementations in `tuple.rs` (#19880) [ty] Function argument inlay hints (#19269) [ty] Remove Salsa interning for `TypedDictType` (#19879) Fix `lint.future-annotations` link (#19876) [ty] Reduce memory usage of `TupleSpec` and `TupleType` (#19872) [ty] Track heap usage of salsa structs (#19790) Update salsa to pull in tracked struct changes (#19843) [ty] simplify CycleDetector::visit signature (#19873) ...

Gankra added 3 commits August 12, 2025 14:01

basic docstring support

89a4e67

add tests for new functionality

3ea87f8

put markdown hack on better abstraction level

8ab8309

Gankra requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners August 12, 2025 18:18

Gankra added server Related to the LSP server ty Multi-file analysis & type inference labels Aug 12, 2025

fixup

4d386cf

Gankra commented Aug 12, 2025

View reviewed changes

MichaReiser approved these changes Aug 13, 2025

View reviewed changes

fixups

b80b444

Gankra enabled auto-merge (squash) August 13, 2025 14:57

Gankra merged commit d59282e into main Aug 13, 2025
37 checks passed

Gankra deleted the gankra/dochover branch August 13, 2025 14:59

InSyncWithFoo mentioned this pull request Aug 13, 2025

Apply formatting to hover information astral-sh/ty#264

Closed

	/// Format a docstring by trimming whitespace and adjusting the indentation.
	///
	/// Summary of changes we make:
	/// * Normalize the string like all other strings
	/// * Ignore docstring that have an escaped newline
	/// * Trim all trailing whitespace, except for a chaperone space that avoids quotes or backslashes
	/// in the last line.
	/// * Trim leading whitespace on the first line, again except for a chaperone space
	/// * If there is only content in the first line and after that only whitespace, collapse the
	/// docstring into one line
	/// * Adjust the indentation (see below)
	///
	/// # Docstring indentation
	///
	/// Unlike any other string, like black we change the indentation of docstring lines.
	///
	/// We want to preserve the indentation inside the docstring relative to the suite statement/block
	/// indent that the docstring statement is in, but also want to apply the change of the outer
	/// indentation in the docstring, e.g.
	/// ```python
	/// def sparkle_sky():
	/// """Make a pretty sparkly sky.
	/// * * ✨ *. .
	/// * * ✨ .
	/// . * . ✨ * . .
	/// """
	/// ```
	/// should become
	/// ```python
	/// def sparkle_sky():
	/// """Make a pretty sparkly sky.
	/// * * ✨ *. .
	/// * * ✨ .
	/// . * . ✨ * . .
	/// """
	/// ```
	/// We can't compute the full indentation here since we don't know what the block indent of
	/// the doc comment will be yet and which we can only have added by formatting each line
	/// separately with a hard line break. This means we need to strip shared indentation from
	/// docstring while preserving the in-docstring bigger-than-suite-statement indentation. Example:
	/// ```python
	/// def f():
	/// """first line
	/// line a
	/// line b
	/// """
	/// ```
	/// The docstring indentation is 2, the block indents will change this to 4 (but we can't
	/// determine this at this point). The indentation of line a is 2, so we trim ` line a`
	/// to `line a`. For line b it's 5, so we trim it to `line b` and pad with 5-2=3 spaces to
	/// ` line b`. The closing quotes, being on their own line, are stripped get only the
	/// default indentation. Fully formatted:
	/// ```python
	/// def f():
	/// """first line
	/// line a
	/// line b
	/// """
	/// ```
	///
	/// Tabs are counted by padding them to the next multiple of 8 according to
	/// [`str.expandtabs`](https://docs.python.org/3/library/stdtypes.html#str.expandtabs).
	///
	/// Additionally, if any line in the docstring has less indentation than the docstring
	/// (effectively a negative indentation wrt. to the current level), we pad all lines to the
	/// level of the docstring with spaces.
	/// ```python
	/// def f():
	/// """first line
	/// line a
	/// line b
	/// line c
	/// """
	/// ```
	/// Here line a is 3 columns negatively indented, so we pad all lines by an extra 3 spaces:
	/// ```python
	/// def f():
	/// """first line
	/// line a
	/// line b
	/// line c
	/// """
	/// ```
	/// The indentation is rewritten to all-spaces when using [`IndentStyle::Space`].
	/// The formatter preserves tab-indentations when using [`IndentStyle::Tab`], but doesn't convert
	/// `indent-width * spaces` to tabs because doing so could break ASCII art and other docstrings
	/// that use spaces for alignment.
	pub(crate) fn format(normalized: &NormalizedString, f: &mut PyFormatter) -> FormatResult<()> {

Comments

Conversation

Gankra commented Aug 12, 2025

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading

`mypy_primer` results