cleanup implementation

crates/ty_ide/src/goto.rs

@@ -15,8 +15,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
 use ty_python_semantic::ResolvedDefinition;
 use ty_python_semantic::types::Type;
 use ty_python_semantic::types::ide_support::{
-    CallSignatureDetails, call_signature_details, call_signature_details_typed,
-    definitions_for_keyword_argument,
+    call_signature_details, call_type_simplified_by_overloads, definitions_for_keyword_argument,
 };
 use ty_python_semantic::{
     HasDefinition, HasType, ImportAliasResolution, SemanticModel, definitions_for_imported_symbol,
@@ -327,20 +326,16 @@ impl GotoTarget<'_> {
         Some(ty)
     }
 
-    pub(crate) fn signature<'db>(
+    /// Try to get a simplified display of this callable type by resolving overloads
+    pub(crate) fn call_type_simplified_by_overloads(
         &self,
-        model: &SemanticModel<'db>,
-    ) -> Option<Vec<CallSignatureDetails<'db>>> {
+        model: &SemanticModel,
+    ) -> Option<String> {
         if let GotoTarget::Call { call, .. } = self {
-            let signature_details = call_signature_details(model.db(), model, call);
-            if signature_details.len() > 1 {
-                let signature_details = call_signature_details_typed(model.db(), model, call);
-                if !signature_details.is_empty() {
-                    return Some(signature_details);
-                }
-            }
+            call_type_simplified_by_overloads(model.db(), model, call)
+        } else {
+            None
         }
-        None
     }
 
     /// Gets the definitions for this goto target.

crates/ty_ide/src/hover.rs

@@ -6,7 +6,6 @@ use ruff_db::parsed::parsed_module;
 use ruff_text_size::{Ranged, TextSize};
 use std::fmt;
 use std::fmt::Formatter;
-use ty_python_semantic::types::ide_support::CallSignatureDetails;
 use ty_python_semantic::types::{KnownInstanceType, Type, TypeVarVariance};
 use ty_python_semantic::{DisplaySettings, SemanticModel};
 
@@ -31,7 +30,7 @@ pub fn hover(db: &dyn Db, file: File, offset: TextSize) -> Option<RangedValue<Ho
         .map(HoverContent::Docstring);
 
     let mut contents = Vec::new();
-    if let Some(signature) = goto_target.signature(&model) {
+    if let Some(signature) = goto_target.call_type_simplified_by_overloads(&model) {
         contents.push(HoverContent::Signature(signature));
     } else if let Some(ty) = goto_target.inferred_type(&model) {
         tracing::debug!("Inferred type of covering node is {}", ty.display(db));
@@ -118,7 +117,7 @@ impl fmt::Display for DisplayHover<'_, '_> {
 
 #[derive(Debug, Clone)]
 pub enum HoverContent<'db> {
-    Signature(Vec<CallSignatureDetails<'db>>),
+    Signature(String),
     Type(Type<'db>, Option<TypeVarVariance>),
     Docstring(Docstring),
 }
@@ -142,14 +141,8 @@ pub(crate) struct DisplayHoverContent<'a, 'db> {
 impl fmt::Display for DisplayHoverContent<'_, '_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.content {
-            HoverContent::Signature(signatures) => {
-                for signature in signatures {
-                    self.kind
-                        .fenced_code_block(&signature.label, "python")
-                        .fmt(f)?;
-                    self.kind.horizontal_line().fmt(f)?;
-                }
-                Ok(())
+            HoverContent::Signature(signature) => {
+                self.kind.fenced_code_block(&signature, "python").fmt(f)
             }
             HoverContent::Type(ty, variance) => {
                 let variance = match variance {
@@ -972,8 +965,6 @@ def ab(a: str): ...
 
         assert_snapshot!(test.hover(), @r"
         (a: int) -> Unknown
-        ---------------------------------------------
-
         ---------------------------------------------
         the int overload
 
@@ -981,8 +972,6 @@ def ab(a: str): ...
         ```python
         (a: int) -> Unknown
         ```
-        ---
-
         ---
         ```text
         the int overload
@@ -1039,8 +1028,6 @@ def ab(a: str):
 
         assert_snapshot!(test.hover(), @r#"
         (a: str) -> Unknown
-        ---------------------------------------------
-
         ---------------------------------------------
         the int overload
 
@@ -1048,8 +1035,6 @@ def ab(a: str):
         ```python
         (a: str) -> Unknown
         ```
-        ---
-
         ---
         ```text
         the int overload
@@ -1105,18 +1090,20 @@ def ab(a: int):
             .build();
 
         assert_snapshot!(test.hover(), @r"
-        (a: int, b: int) -> Unknown
-        ---------------------------------------------
-
+        (
+            a: int,
+            b: int
+        ) -> Unknown
         ---------------------------------------------
         the two arg overload
 
         ---------------------------------------------
         ```python
-        (a: int, b: int) -> Unknown
+        (
+            a: int,
+            b: int
+        ) -> Unknown
         ```
-        ---
-
         ---
         ```text
         the two arg overload
@@ -1173,8 +1160,6 @@ def ab(a: int):
 
         assert_snapshot!(test.hover(), @r"
         (a: int) -> Unknown
-        ---------------------------------------------
-
         ---------------------------------------------
         the two arg overload
 
@@ -1182,8 +1167,6 @@ def ab(a: int):
         ```python
         (a: int) -> Unknown
         ```
-        ---
-
         ---
         ```text
         the two arg overload
@@ -1243,18 +1226,22 @@ def ab(a: int, *, c: int):
             .build();
 
         assert_snapshot!(test.hover(), @r"
-        (a: int, *, b: int) -> Unknown
-        ---------------------------------------------
-
+        (
+            a: int,
+            *,
+            b: int
+        ) -> Unknown
         ---------------------------------------------
         keywordless overload
 
         ---------------------------------------------
         ```python
-        (a: int, *, b: int) -> Unknown
+        (
+            a: int,
+            *,
+            b: int
+        ) -> Unknown
         ```
-        ---
-
         ---
         ```text
         keywordless overload
@@ -1314,18 +1301,22 @@ def ab(a: int, *, c: int):
             .build();
 
         assert_snapshot!(test.hover(), @r"
-        (a: int, *, c: int) -> Unknown
-        ---------------------------------------------
-
+        (
+            a: int,
+            *,
+            c: int
+        ) -> Unknown
         ---------------------------------------------
         keywordless overload
 
         ---------------------------------------------
         ```python
-        (a: int, *, c: int) -> Unknown
+        (
+            a: int,
+            *,
+            c: int
+        ) -> Unknown
         ```
-        ---
-
         ---
         ```text
         keywordless overload
@@ -1372,18 +1363,28 @@ def ab(a: int, *, c: int):
         );
 
         assert_snapshot!(test.hover(), @r#"
-        (a: int, b) -> Unknown
-        ---------------------------------------------
-
+        (
+            a: int,
+            b
+        ) -> Unknown
+        (
+            a: str,
+            b
+        ) -> Unknown
         ---------------------------------------------
         The first overload
 
         ---------------------------------------------
         ```python
-        (a: int, b) -> Unknown
+        (
+            a: int,
+            b
+        ) -> Unknown
+        (
+            a: str,
+            b
+        ) -> Unknown
         ```
-        ---
-
         ---
         ```text
         The first overload
@@ -1430,17 +1431,15 @@ def ab(a: int, *, c: int):
 
         assert_snapshot!(test.hover(), @r#"
         (a: int) -> Unknown
-        ---------------------------------------------
-
+        (a: str) -> Unknown
         ---------------------------------------------
         The first overload
 
         ---------------------------------------------
         ```python
         (a: int) -> Unknown
+        (a: str) -> Unknown
         ```
-        ---
-
         ---
         ```text
         The first overload

crates/ty_python_semantic/src/types/call/arguments.rs

@@ -67,6 +67,9 @@ impl<'a, 'db> CallArguments<'a, 'db> {
     }
 
     /// Like [`Self::from_arguments`] but fills as much typing info in as possible.
+    ///
+    /// This currently only exists for the LSP usecase, and shouldn't be used in normal
+    /// typechecking.
     pub(crate) fn from_arguments_typed(
         arguments: &'a ast::Arguments,
         mut infer_argument_type: impl FnMut(Option<&ast::Expr>, &ast::Expr) -> Type<'db>,

crates/ty_python_semantic/src/types/ide_support.rs

@@ -17,7 +17,7 @@ use crate::types::{
     ClassBase, ClassLiteral, DynamicType, KnownClass, KnownInstanceType, Type, TypeContext,
     TypeVarBoundOrConstraints, class::CodeGeneratorKind,
 };
-use crate::{Db, HasType, NameKind, SemanticModel};
+use crate::{Db, DisplaySettings, HasType, NameKind, SemanticModel};
 use ruff_db::files::{File, FileRange};
 use ruff_db::parsed::parsed_module;
 use ruff_python_ast::name::Name;
@@ -973,55 +973,63 @@ pub fn call_signature_details<'db>(
     }
 }
 
-/// Extract signature details from a function call expression using type info.
+/// Given a call expression that has overloads, and whose overload is resolved to a
+/// single option by its arguments, return the type of the Signature.
 ///
-/// Unlike [`call_signature_details`][] we reduce down to the exact match if possible.
-pub fn call_signature_details_typed<'db>(
+/// This is only used for simplifying complex call types, so if we ever detect that
+/// the given callable type *is* simple, or that our answer *won't* be simple, we
+/// bail at out and return None, so that the original type can be used.
+///
+/// We do this because `Type::Signature` intentionally loses a lot of context, and
+/// so it has a "worse" display than say `Type::FunctionLiteral` or `Type::BoundMethod`,
+/// which this analysis would naturally wipe away. The contexts this function
+/// succeeds in are those where we would print a complicated/ugly type anyway.
+pub fn call_type_simplified_by_overloads<'db>(
     db: &'db dyn Db,
     model: &SemanticModel<'db>,
     call_expr: &ast::ExprCall,
-) -> Vec<CallSignatureDetails<'db>> {
+) -> Option<String> {
     let func_type = call_expr.func.inferred_type(model);
 
     // Use into_callable to handle all the complex type conversions
-    if let Some(callable_type) = func_type.try_upcast_to_callable(db) {
-        // Really shove as much type info in as we can
-        let call_arguments =
-            CallArguments::from_arguments_typed(&call_expr.arguments, |_, splatted_value| {
-                splatted_value.inferred_type(model)
-            });
+    let callable_type = func_type.try_upcast_to_callable(db)?;
+    let bindings = callable_type.bindings(db);
 
-        // Extract signature details from all callable bindings
-        callable_type
-            .bindings(db)
-            .match_parameters(db, &call_arguments)
-            .check_types(db, &call_arguments, TypeContext::default(), &[])
-            // Only use the Ok
-            .iter()
-            .flatten()
-            // The first matching overload is the one to use
-            .filter_map(|binding| binding.matching_overloads().next())
-            .map(|(_, binding)| {
-                let argument_to_parameter_mapping = binding.argument_matches().to_vec();
-                let signature = binding.signature.clone();
-                let display_details = signature.display(db).to_string_parts();
-                let parameter_label_offsets = display_details.parameter_ranges;
-                let parameter_names = display_details.parameter_names;
-
-                CallSignatureDetails {
-                    definition: signature.definition(),
-                    signature,
-                    label: display_details.label,
-                    parameter_label_offsets,
-                    parameter_names,
-                    argument_to_parameter_mapping,
-                }
-            })
-            .collect()
-    } else {
-        // Type is not callable, return empty signatures
-        vec![]
+    // If the callable is trivial this analysis is useless, bail out
+    if let Some(binding) = bindings.single_element()
+        && binding.overloads().len() < 2
+    {
+        return None;
     }
+
+    // Hand the overload resolution system as much type info as we have
+    let args = CallArguments::from_arguments_typed(&call_expr.arguments, |_, splatted_value| {
+        splatted_value.inferred_type(model)
+    });
+
+    // Try to resolve overloads with the arguments/types we have
+    let mut resolved = bindings
+        .match_parameters(db, &args)
+        .check_types(db, &args, TypeContext::default(), &[])
+        // Only use the Ok
+        .iter()
+        .flatten()
+        .flat_map(|binding| {
+            binding.matching_overloads().map(|(_, overload)| {
+                overload
+                    .signature
+                    .display_with(db, DisplaySettings::default().multiline())
+                    .to_string()
+            })
+        })
+        .collect::<Vec<_>>();
+
+    // If at the end of this we still got multiple signatures (or no signatures), give up
+    if resolved.len() != 1 {
+        return None;
+    }
+
+    resolved.pop()
 }
 
 /// Returns the definitions of the binary operation along with its callable type.