[red-knot] Better diagnostics for method calls

crates/red_knot_python_semantic/resources/mdtest/call/methods.md

@@ -239,19 +239,19 @@ method_wrapper(None, C)
 method_wrapper(None)
 
 # Passing something that is not assignable to `type` as the `owner` argument is an
-# error: [invalid-argument-type] "Object of type `Literal[1]` cannot be assigned to parameter 2 (`owner`); expected type `type`"
+# error: [invalid-argument-type] "Object of type `Literal[1]` cannot be assigned to parameter 2 (`owner`) of method wrapper `__get__` of function `f`; expected type `type`"
 method_wrapper(None, 1)
 
 # Passing `None` as the `owner` argument when `instance` is `None` is an
-# error: [invalid-argument-type] "Object of type `None` cannot be assigned to parameter 2 (`owner`); expected type `type`"
+# error: [invalid-argument-type] "Object of type `None` cannot be assigned to parameter 2 (`owner`) of method wrapper `__get__` of function `f`; expected type `type`"
 method_wrapper(None, None)
 
 # Calling `__get__` without any arguments is an
 # error: [missing-argument] "No argument provided for required parameter `instance`"
 method_wrapper()
 
 # Calling `__get__` with too many positional arguments is an
-# error: [too-many-positional-arguments] "Too many positional arguments: expected 2, got 3"
+# error: [too-many-positional-arguments] "Too many positional arguments to method wrapper `__get__` of function `f`: expected 2, got 3"
 method_wrapper(C(), C, "one too many")
 ```
 
@@ -298,7 +298,7 @@ class D:
         # This function is wrongly annotated, it should be `type[D]` instead of `D`
         pass
 
-# error: [invalid-argument-type] "Object of type `Literal[D]` cannot be assigned to parameter 1 (`cls`); expected type `D`"
+# error: [invalid-argument-type] "Object of type `Literal[D]` cannot be assigned to parameter 1 (`cls`) of bound method `f`; expected type `D`"
 D.f()
 ```
 

crates/red_knot_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -425,15 +425,15 @@ wrapper_descriptor(f, None)
 wrapper_descriptor(f, C())
 
 # Calling it with something that is not a `FunctionType` as the first argument is an
-# error: [invalid-argument-type] "Object of type `Literal[1]` cannot be assigned to parameter 1 (`self`); expected type `FunctionType`"
+# error: [invalid-argument-type] "Object of type `Literal[1]` cannot be assigned to parameter 1 (`self`) of wrapper descriptor `FunctionType.__get__`; expected type `FunctionType`"
 wrapper_descriptor(1, None, type(f))
 
 # Calling it with something that is not a `type` as the `owner` argument is an
-# error: [invalid-argument-type] "Object of type `Literal[f]` cannot be assigned to parameter 3 (`owner`); expected type `type`"
+# error: [invalid-argument-type] "Object of type `Literal[f]` cannot be assigned to parameter 3 (`owner`) of wrapper descriptor `FunctionType.__get__`; expected type `type`"
 wrapper_descriptor(f, None, f)
 
 # Calling it with too many positional arguments is an
-# error: [too-many-positional-arguments] "Too many positional arguments: expected 3, got 4"
+# error: [too-many-positional-arguments] "Too many positional arguments to wrapper descriptor `FunctionType.__get__`: expected 3, got 4"
 wrapper_descriptor(f, None, type(f), "one too many")
 ```
 

crates/red_knot_python_semantic/resources/mdtest/diagnostics/invalid_argument_type.md

@@ -182,3 +182,16 @@ class C:
 c = C()
 c("wrong")  # error: [invalid-argument-type]
 ```
+
+## Calls to methods
+
+Tests that we also see a reference to a function if the callable is a bound method.
+
+```py
+class C:
+    def square(self, x: int) -> int:
+        return x * x
+
+c = C()
+c.square("hello")  # error: [invalid-argument-type]
+```

crates/red_knot_python_semantic/resources/mdtest/snapshots/invalid_argument_type.md_-_Invalid_argument_type_diagnostics_-_Calls_to_methods.snap

@@ -0,0 +1,41 @@
+---
+source: crates/red_knot_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: invalid_argument_type.md - Invalid argument type diagnostics - Calls to methods
+mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/invalid_argument_type.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | class C:
+2 |     def square(self, x: int) -> int:
+3 |         return x * x
+4 | 
+5 | c = C()
+6 | c.square("hello")  # error: [invalid-argument-type]
+```
+
+# Diagnostics
+
+```
+error: lint:invalid-argument-type
+ --> /src/mdtest_snippet.py:6:10
+  |
+5 | c = C()
+6 | c.square("hello")  # error: [invalid-argument-type]
+  |          ^^^^^^^ Object of type `Literal["hello"]` cannot be assigned to parameter 2 (`x`) of bound method `square`; expected type `int`
+  |
+ ::: /src/mdtest_snippet.py:2:22
+  |
+1 | class C:
+2 |     def square(self, x: int) -> int:
+  |                      ------ info: parameter declared in function definition here
+3 |         return x * x
+  |
+
+```

crates/red_knot_python_semantic/src/types/call/bind.rs

@@ -5,7 +5,7 @@ use crate::types::diagnostic::{
     TOO_MANY_POSITIONAL_ARGUMENTS, UNKNOWN_ARGUMENT,
 };
 use crate::types::signatures::Parameter;
-use crate::types::{todo_type, UnionType};
+use crate::types::{todo_type, CallableType, UnionType};
 use ruff_db::diagnostic::{SecondaryDiagnosticMessage, Span};
 use ruff_python_ast as ast;
 use ruff_text_size::Ranged;
@@ -137,6 +137,13 @@ pub(crate) fn bind_call<'db>(
     }
 }
 
+/// Describes a callable for the purposes of diagnostics.
+#[derive(Debug)]
+pub(crate) struct CallableDescriptor<'a> {
+    name: &'a str,
+    kind: &'a str,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct CallBinding<'db> {
     /// Type of the callable object (function, class...)
@@ -200,18 +207,43 @@ impl<'db> CallBinding<'db> {
         }
     }
 
-    fn callable_name(&self, db: &'db dyn Db) -> Option<&str> {
+    fn callable_descriptor(&self, db: &'db dyn Db) -> Option<CallableDescriptor> {
         match self.callable_ty {
-            Type::FunctionLiteral(function) => Some(function.name(db)),
-            Type::ClassLiteral(class_type) => Some(class_type.class().name(db)),
+            Type::FunctionLiteral(function) => Some(CallableDescriptor {
+                kind: "function",
+                name: function.name(db),
+            }),
+            Type::ClassLiteral(class_type) => Some(CallableDescriptor {
+                kind: "class",
+                name: class_type.class().name(db),
+            }),
+            Type::Callable(CallableType::BoundMethod(bound_method)) => Some(CallableDescriptor {
+                kind: "bound method",
+                name: bound_method.function(db).name(db),
+            }),
+            Type::Callable(CallableType::MethodWrapperDunderGet(function)) => {
+                Some(CallableDescriptor {
+                    kind: "method wrapper `__get__` of function",
+                    name: function.name(db),
+                })
+            }
+            Type::Callable(CallableType::WrapperDescriptorDunderGet) => Some(CallableDescriptor {
+                kind: "wrapper descriptor",
+                name: "FunctionType.__get__",
+            }),
             _ => None,
         }
     }
 
     pub(crate) fn report_diagnostics(&self, context: &InferContext<'db>, node: ast::AnyNodeRef) {
-        let callable_name = self.callable_name(context.db());
+        let callable_descriptor = self.callable_descriptor(context.db());
         for error in &self.errors {
-            error.report_diagnostic(context, node, self.callable_ty, callable_name);
+            error.report_diagnostic(
+                context,
+                node,
+                self.callable_ty,
+                callable_descriptor.as_ref(),
+            );
         }
     }
 
@@ -304,12 +336,46 @@ pub(crate) enum CallBindingError<'db> {
 }
 
 impl<'db> CallBindingError<'db> {
+    fn parameter_span_from_index(
+        db: &'db dyn Db,
+        callable_ty: Type<'db>,
+        parameter_index: usize,
+    ) -> Option<Span> {
+        match callable_ty {
+            Type::FunctionLiteral(function) => {
+                let function_scope = function.body_scope(db);
+                let mut span = Span::from(function_scope.file(db));
+                let node = function_scope.node(db);
+                if let Some(func_def) = node.as_function() {
+                    let range = func_def
+                        .parameters
+                        .iter()
+                        .nth(parameter_index)
+                        .map(|param| param.range())
+                        .unwrap_or(func_def.parameters.range);
+                    span = span.with_range(range);
+                    Some(span)
+                } else {
+                    None
+                }
+            }
+            Type::Callable(CallableType::BoundMethod(bound_method)) => {
+                Self::parameter_span_from_index(
+                    db,
+                    Type::FunctionLiteral(bound_method.function(db)),
+                    parameter_index,
+                )
+            }
+            _ => None,
+        }
+    }
+
     pub(super) fn report_diagnostic(
         &self,
         context: &InferContext<'db>,
         node: ast::AnyNodeRef,
         callable_ty: Type<'db>,
-        callable_name: Option<&str>,
+        callable_descriptor: Option<&CallableDescriptor>,
     ) {
         match self {
             Self::InvalidArgumentType {
@@ -319,23 +385,13 @@ impl<'db> CallBindingError<'db> {
                 provided_ty,
             } => {
                 let mut messages = vec![];
-                if let Some(func_lit) = callable_ty.into_function_literal() {
-                    let func_lit_scope = func_lit.body_scope(context.db());
-                    let mut span = Span::from(func_lit_scope.file(context.db()));
-                    let node = func_lit_scope.node(context.db());
-                    if let Some(func_def) = node.as_function() {
-                        let range = func_def
-                            .parameters
-                            .iter()
-                            .nth(parameter.index)
-                            .map(|param| param.range())
-                            .unwrap_or(func_def.parameters.range);
-                        span = span.with_range(range);
-                        messages.push(SecondaryDiagnosticMessage::new(
-                            span,
-                            "parameter declared in function definition here",
-                        ));
-                    }
+                if let Some(span) =
+                    Self::parameter_span_from_index(context.db(), callable_ty, parameter.index)
+                {
+                    messages.push(SecondaryDiagnosticMessage::new(
+                        span,
+                        "parameter declared in function definition here",
+                    ));
                 }
 
                 let provided_ty_display = provided_ty.display(context.db());
@@ -346,8 +402,8 @@ impl<'db> CallBindingError<'db> {
                     format_args!(
                         "Object of type `{provided_ty_display}` cannot be assigned to \
                         parameter {parameter}{}; expected type `{expected_ty_display}`",
-                        if let Some(callable_name) = callable_name {
-                            format!(" of function `{callable_name}`")
+                        if let Some(CallableDescriptor { kind, name }) = callable_descriptor {
+                            format!(" of {kind} `{name}`")
                         } else {
                             String::new()
                         }
@@ -367,8 +423,8 @@ impl<'db> CallBindingError<'db> {
                     format_args!(
                         "Too many positional arguments{}: expected \
                         {expected_positional_count}, got {provided_positional_count}",
-                        if let Some(callable_name) = callable_name {
-                            format!(" to function `{callable_name}`")
+                        if let Some(CallableDescriptor { kind, name }) = callable_descriptor {
+                            format!(" to {kind} `{name}`")
                         } else {
                             String::new()
                         }
@@ -383,8 +439,8 @@ impl<'db> CallBindingError<'db> {
                     node,
                     format_args!(
                         "No argument{s} provided for required parameter{s} {parameters}{}",
-                        if let Some(callable_name) = callable_name {
-                            format!(" of function `{callable_name}`")
+                        if let Some(CallableDescriptor { kind, name }) = callable_descriptor {
+                            format!(" of {kind} `{name}`")
                         } else {
                             String::new()
                         }
@@ -401,8 +457,8 @@ impl<'db> CallBindingError<'db> {
                     Self::get_node(node, *argument_index),
                     format_args!(
                         "Argument `{argument_name}` does not match any known parameter{}",
-                        if let Some(callable_name) = callable_name {
-                            format!(" of function `{callable_name}`")
+                        if let Some(CallableDescriptor { kind, name }) = callable_descriptor {
+                            format!(" of {kind} `{name}`")
                         } else {
                             String::new()
                         }
@@ -419,8 +475,8 @@ impl<'db> CallBindingError<'db> {
                     Self::get_node(node, *argument_index),
                     format_args!(
                         "Multiple values provided for parameter {parameter}{}",
-                        if let Some(callable_name) = callable_name {
-                            format!(" of function `{callable_name}`")
+                        if let Some(CallableDescriptor { kind, name }) = callable_descriptor {
+                            format!(" of {kind} `{name}`")
                         } else {
                             String::new()
                         }