[ty] Resolve overloads for hovers by Gankra · Pull Request #21417 · astral-sh/ruff

Gankra · 2025-11-13T01:21:57Z

This is a very conservative minimal implementation of applying overloads to resolve a callable-type-being-called down to a single function signature on hover. If we ever encounter a situation where the answer doesn't simplify down to a single function call, we bail out to preserve prettier printing of non-raw-Signatures.

The resulting Signatures are still a bit bare, I'm going to try to improve that in a followup to improve our Signature printing in general.

Fixes astral-sh/ty#73

Gankra · 2025-11-13T01:24:32Z

Most of these snapshot changes are bad and the consequence of this logic firing too eagerly. I need to cleanup that part, but I wanted to get the Types and Semantics part up sooner than later for correction.

astral-sh-bot · 2025-11-13T01:24:36Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

astral-sh-bot · 2025-11-13T01:25:31Z

`mypy_primer` results

No ecosystem changes detected ✅

No memory usage changes detected ✅

Gankra · 2025-11-13T20:40:23Z

crates/ty_python_semantic/src/types/ide_support.rs

+            binding.matching_overloads().map(|(_, overload)| {
+                overload
+                    .signature
+                    .display_with(db, DisplaySettings::default().multiline())
+                    .to_string()
+            })


I spent an unconscionable amount of time trying to have this return a Type or something more useful than a String but the types on Binding are haunted by the overloads when you try to render them (meaning printing them still returns the full overload set!).

Stringifying the raw Signature is the only thing that seems to be safe.

(An early version returned a full CallSignatureDetails but this usecase has no use of that so I dropped it as a waste of effort. Also this version that just returns a String is easier to iterate on (I kept trying to Not have a String and had to refactor like 4 calls and 2 types over and over as I experimented between Type vs Vec<Type> vs CallSignatureDetails vs...))

Gankra · 2025-11-13T20:46:50Z

crates/ty_ide/src/hover.rs


-pub struct DisplayHover<'a> {
-    db: &'a dyn Db,
-    hover: &'a Hover<'a>,
+pub struct DisplayHover<'db, 'a> {
+    db: &'db dyn Db,
+    hover: &'a Hover<'db>,
    kind: MarkupKind,
 }


This is debris from versions where this was being asked to store CallSignatureDetails, which is actually invariant over 'db. Leaving it like this just simplifies any potential future work.

MichaReiser

I'll do a full review on Monday but I've a question why we limit overload matching to hover only

MichaReiser · 2025-11-14T18:42:59Z

crates/ty_ide/src/hover.rs

-    // TODO: Render the symbol's signature instead of just its type.
    let mut contents = Vec::new();
-    if let Some(ty) = ty {
+    if let Some(signature) = goto_target.call_type_simplified_by_overloads(&model) {


Is there a reason we only apply overload simplification for hover mode? For example, TypeScript jumps to the matching overload if you use "go to definition," which matches my expectation. https://www.typescriptlang.org/play/?#code/GYVwdgxgLglg9mABFApgZygCgIYC5FggC2ARigE4CU+hpFA3AFCiSwLLpZ4HFnkA0iEvgzkYYAObVEo8RKYto8JKgw4avCoJIB+EVDGTp2MAE9EAb0aMAvtcaqsARgAMggESP3le0A

I'm surprised that Pylance doesn't seem to do that

Hmm strictly speaking I don't think goto-definition should jump to an overload unless we can't find the actual implementation (in which case it's actually goto-declaration). goto-declaration should ideally jump to the overload though.

MichaReiser · 2025-11-17T08:10:33Z

crates/ty_python_semantic/src/types/ide_support.rs

+    let func_type = call_expr.func.inferred_type(model);
+
+    // Use into_callable to handle all the complex type conversions
+    let callable_type = func_type.try_upcast_to_callable(db)?;


Is my understanding correct that this adds another "todo" call-site to astral-sh/ty#1086?

I'm not sure if we'll be able to do astral-sh/ty#1086; commented on that issue.

dhruvmanila · 2025-11-17T08:30:38Z

crates/ty_python_semantic/src/types/ide_support.rs

+        .flat_map(|binding| {
+            binding.matching_overloads().map(|(_, overload)| {
+                overload
+                    .signature
+                    .display_with(db, DisplaySettings::default().multiline())
+                    .to_string()
+            })
+        })


I think I remember now facing a similar problem regarding using the signature_type on Binding struct that we saw during our call last week. For context, the signature_type would represent the FunctionLiteral which means the display would contain all the overloads and this is the main reason we need to use the display of Signature to display the matching overload which leads to requiring #21438.

I faced this issue while implementing #18452 and the way I resolved it is by capturing the required information (FunctionType, BoundMethodType, etc. and the matching overload index) in MatchingOverloadLiteral and implementing a .get method to return the matching OverloadLiteral.

I think we could possibly use a similar approach to get the actual OverloadLiteral that needs to be displayed as it does implement the Display trait. You might find it useful to go through this part of the code:

ruff/crates/ty_python_semantic/src/types/call/bind.rs

Lines 2125 to 2162 in dcc451d

// TODO: This should probably be adapted to handle more

// types of callables[1]. At present, it just handles

// standard function and method calls.

//

// [1]: https://github.com/astral-sh/ty/issues/274#issuecomment-2881856028

let function_type_and_kind = match self.signature_type {

Type::FunctionLiteral(function) => Some((FunctionKind::Function, function)),

Type::BoundMethod(bound_method) => Some((

FunctionKind::BoundMethod,

bound_method.function(context.db()),

)),

Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet(

function,

)) => Some((FunctionKind::MethodWrapper, function)),

_ => None,

};

// If there is a single matching overload, the diagnostics should be reported

// directly for that overload.

if let Some(matching_overload_index) = self.matching_overload_index {

let callable_description =

CallableDescription::new(context.db(), self.signature_type);

let matching_overload =

function_type_and_kind.map(|(kind, function)| MatchingOverloadLiteral {

index: matching_overload_index,

kind,

function,

});

self.overloads[matching_overload_index].report_diagnostics(

context,

node,

self.signature_type,

callable_description.as_ref(),

union_diag,

matching_overload.as_ref(),

);

return;

}

Another approach could be to expand the DisplaySettings to include the matching_overload_index: MatchingOverloadIndex which can then be used directly to filter the overloads at the display level. The None variant would display all the overloads.

MichaReiser · 2025-11-20T09:10:31Z

@Gankra what's the status of this PR?

Gankra · 2025-11-20T17:01:45Z

I think this PR could be landed as-is as an incremental strict improvement. I've been locally experimenting with alternative approaches that would e.g. empower goto-declaration as well, but I keep running into issues around the fact that randomly places in the code (like type printing) don't want to talk about A Specific Overload (connected to the fact that definitions-hanging-off-types is a fundamental hack).

MichaReiser · 2025-11-20T17:31:29Z

I'm fine with handling go-to separately. But we might want to address @dhruvmanila's feedback before landing?

Gankra · 2025-11-20T17:38:52Z

Yeah dhruv's suggestions are what I was experimenting with locally. Hover doesn't really benefit from it (it really just needs a String, and I'm computing that String fine), but goto-decl would benefit.

MichaReiser

Alright. I'm fine with this, given that you're already working on the follow ups. If you don't end up implementing all of them, would you mind opening follow up issues in that case.

Gankra · 2025-11-20T17:44:18Z

Or well, I suppose dhruv's suggestions would ideally obsolete the more hacky stuff in #21438

But yeah it's all kinda the same work.

Resolve overloads for hovers

89236a3

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

Gankra added 2 commits November 12, 2025 23:23

only apply the rule if non-trivial

3a10f87

cleanup implementation

dcc451d

Gankra marked this pull request as ready for review November 13, 2025 20:33

Gankra requested review from AlexWaygood, MichaReiser, carljm, dcreager and sharkdp as code owners November 13, 2025 20:33

Gankra commented Nov 13, 2025

View reviewed changes

Gankra mentioned this pull request Nov 13, 2025

[ty] print function names for bare Signatures opportunistically #21438

Closed

carljm removed their request for review November 13, 2025 23:31

MichaReiser requested review from dhruvmanila and removed request for AlexWaygood, dcreager and sharkdp November 14, 2025 18:39

MichaReiser reviewed Nov 14, 2025

View reviewed changes

MichaReiser reviewed Nov 17, 2025

View reviewed changes

dhruvmanila reviewed Nov 17, 2025

View reviewed changes

MichaReiser approved these changes Nov 20, 2025

View reviewed changes

Gankra merged commit 6e84f4f into main Nov 20, 2025
41 checks passed

Gankra deleted the gankra/overloads branch November 20, 2025 17:45

	// TODO: This should probably be adapted to handle more
	// types of callables[1]. At present, it just handles
	// standard function and method calls.
	//
	// [1]: https://github.com/astral-sh/ty/issues/274#issuecomment-2881856028
	let function_type_and_kind = match self.signature_type {
	Type::FunctionLiteral(function) => Some((FunctionKind::Function, function)),
	Type::BoundMethod(bound_method) => Some((
	FunctionKind::BoundMethod,
	bound_method.function(context.db()),
	)),
	Type::KnownBoundMethod(KnownBoundMethodType::FunctionTypeDunderGet(
	function,
	)) => Some((FunctionKind::MethodWrapper, function)),
	_ => None,
	};

	// If there is a single matching overload, the diagnostics should be reported
	// directly for that overload.
	if let Some(matching_overload_index) = self.matching_overload_index {
	let callable_description =
	CallableDescription::new(context.db(), self.signature_type);
	let matching_overload =
	function_type_and_kind.map(\|(kind, function)\| MatchingOverloadLiteral {
	index: matching_overload_index,
	kind,
	function,
	});
	self.overloads[matching_overload_index].report_diagnostics(
	context,
	node,
	self.signature_type,
	callable_description.as_ref(),
	union_diag,
	matching_overload.as_ref(),
	);
	return;
	}

Comments

Conversation

Gankra commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gankra commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astral-sh-bot bot commented Nov 13, 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!

astral-sh-bot bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Gankra Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Gankra Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Gankra Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Gankra Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

carljm Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Nov 20, 2025

Uh oh!

Gankra commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser commented Nov 20, 2025

Uh oh!

Gankra commented Nov 20, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Gankra commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Gankra commented Nov 13, 2025 •

edited

Loading

Gankra commented Nov 13, 2025 •

edited

Loading

astral-sh-bot bot commented Nov 13, 2025 •

edited

Loading

astral-sh-bot bot commented Nov 13, 2025 •

edited

Loading

`mypy_primer` results

carljm Nov 17, 2025 •

edited

Loading

dhruvmanila Nov 17, 2025 •

edited

Loading

Gankra commented Nov 20, 2025 •

edited

Loading

Gankra commented Nov 20, 2025 •

edited

Loading