From b52be9565610c30cd37af682493d201add58255e Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Wed, 8 Jan 2025 14:29:43 -0800 Subject: [PATCH] [red-knot] handle synthetic 'self' argument in call-binding diagnostics --- .../mdtest/call/callable_instance.md | 29 ++++++++++++ .../src/types/call/arguments.rs | 5 +- .../src/types/call/bind.rs | 46 +++++++++++++------ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/callable_instance.md b/crates/red_knot_python_semantic/resources/mdtest/call/callable_instance.md index 746aee725f547..8f5aab67b5aa4 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/call/callable_instance.md +++ b/crates/red_knot_python_semantic/resources/mdtest/call/callable_instance.md @@ -70,3 +70,32 @@ def _(flag: bool): # error: "Object of type `Literal[1] | Literal[__call__]` is not callable (due to union element `Literal[1]`)" reveal_type(a()) # revealed: Unknown | int ``` + +## Call binding errors + +### Wrong argument type + +```py +class C: + def __call__(self, x: int) -> int: + return 1 + +c = C() + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter 2 (`x`) of function `__call__`; expected type `int`" +reveal_type(c("foo")) # revealed: int +``` + +### Wrong argument type on `self` + +```py +class C: + # TODO this definition should also be an error; `C` must be assignable to type of `self` + def __call__(self: int) -> int: + return 1 + +c = C() + +# error: 13 [invalid-argument-type] "Object of type `C` cannot be assigned to parameter 1 (`self`) of function `__call__`; expected type `int`" +reveal_type(c()) # revealed: int +``` diff --git a/crates/red_knot_python_semantic/src/types/call/arguments.rs b/crates/red_knot_python_semantic/src/types/call/arguments.rs index b0067e41a0154..8cc2027c878c2 100644 --- a/crates/red_knot_python_semantic/src/types/call/arguments.rs +++ b/crates/red_knot_python_semantic/src/types/call/arguments.rs @@ -16,7 +16,7 @@ impl<'a, 'db> CallArguments<'a, 'db> { /// Prepend an extra positional argument. pub(crate) fn with_self(&self, self_ty: Type<'db>) -> Self { let mut arguments = Vec::with_capacity(self.0.len() + 1); - arguments.push(Argument::Positional(self_ty)); + arguments.push(Argument::Synthetic(self_ty)); arguments.extend_from_slice(&self.0); Self(arguments) } @@ -48,6 +48,8 @@ impl<'a, 'db> FromIterator> for CallArguments<'a, 'db> { #[derive(Clone, Debug)] pub(crate) enum Argument<'a, 'db> { + /// The synthetic `self` or `cls` argument, which doesn't appear explicitly at the call site. + Synthetic(Type<'db>), /// A positional argument. Positional(Type<'db>), /// A starred positional argument (e.g. `*args`). @@ -61,6 +63,7 @@ pub(crate) enum Argument<'a, 'db> { impl<'db> Argument<'_, 'db> { fn ty(&self) -> Type<'db> { match self { + Self::Synthetic(ty) => *ty, Self::Positional(ty) => *ty, Self::Variadic(ty) => *ty, Self::Keyword { name: _, ty } => *ty, diff --git a/crates/red_knot_python_semantic/src/types/call/bind.rs b/crates/red_knot_python_semantic/src/types/call/bind.rs index ebd15be09a42d..75aa633d1471c 100644 --- a/crates/red_knot_python_semantic/src/types/call/bind.rs +++ b/crates/red_knot_python_semantic/src/types/call/bind.rs @@ -24,9 +24,24 @@ pub(crate) fn bind_call<'db>( let mut errors = vec![]; let mut next_positional = 0; let mut first_excess_positional = None; + let mut num_synthetic_args = 0; + let get_argument_index = |argument_index: usize, num_synthetic_args: usize| { + if argument_index >= num_synthetic_args { + // Adjust the argument index to skip synthetic args, which don't appear at the call + // site and thus won't be in the Call node arguments list. + Some(argument_index - num_synthetic_args) + } else { + // we are erroring on a synthetic argument, we'll just emit the diagnostic on the + // entire Call node, since there's no argument node for this argument at the call site + None + } + }; for (argument_index, argument) in arguments.iter().enumerate() { let (index, parameter, argument_ty, positional) = match argument { - Argument::Positional(ty) => { + Argument::Positional(ty) | Argument::Synthetic(ty) => { + if matches!(argument, Argument::Synthetic(_)) { + num_synthetic_args += 1; + } let Some((index, parameter)) = parameters .get_positional(next_positional) .map(|param| (next_positional, param)) @@ -46,7 +61,7 @@ pub(crate) fn bind_call<'db>( else { errors.push(CallBindingError::UnknownArgument { argument_name: ast::name::Name::new(name), - argument_index, + argument_index: get_argument_index(argument_index, num_synthetic_args), }); continue; }; @@ -62,7 +77,7 @@ pub(crate) fn bind_call<'db>( if !argument_ty.is_assignable_to(db, expected_ty) { errors.push(CallBindingError::InvalidArgumentType { parameter: ParameterContext::new(parameter, index, positional), - argument_index, + argument_index: get_argument_index(argument_index, num_synthetic_args), expected_ty, provided_ty: *argument_ty, }); @@ -74,7 +89,7 @@ pub(crate) fn bind_call<'db>( parameter_tys[index].replace(union); } else { errors.push(CallBindingError::ParameterAlreadyAssigned { - argument_index, + argument_index: get_argument_index(argument_index, num_synthetic_args), parameter: ParameterContext::new(parameter, index, positional), }); } @@ -82,7 +97,10 @@ pub(crate) fn bind_call<'db>( } if let Some(first_excess_argument_index) = first_excess_positional { errors.push(CallBindingError::TooManyPositionalArguments { - first_excess_argument_index, + first_excess_argument_index: get_argument_index( + first_excess_argument_index, + num_synthetic_args, + ), expected_positional_count: parameters.positional().count(), provided_positional_count: next_positional, }); @@ -243,7 +261,7 @@ pub(crate) enum CallBindingError<'db> { /// parameter. InvalidArgumentType { parameter: ParameterContext, - argument_index: usize, + argument_index: Option, expected_ty: Type<'db>, provided_ty: Type<'db>, }, @@ -252,17 +270,17 @@ pub(crate) enum CallBindingError<'db> { /// A call argument can't be matched to any parameter. UnknownArgument { argument_name: ast::name::Name, - argument_index: usize, + argument_index: Option, }, /// More positional arguments are provided in the call than can be handled by the signature. TooManyPositionalArguments { - first_excess_argument_index: usize, + first_excess_argument_index: Option, expected_positional_count: usize, provided_positional_count: usize, }, /// Multiple arguments were provided for a single parameter. ParameterAlreadyAssigned { - argument_index: usize, + argument_index: Option, parameter: ParameterContext, }, } @@ -372,11 +390,11 @@ impl<'db> CallBindingError<'db> { } } - fn get_node(node: ast::AnyNodeRef, argument_index: usize) -> ast::AnyNodeRef { - // If we have a Call node, report the diagnostic on the correct argument node; - // otherwise, report it on the entire provided node. - match node { - ast::AnyNodeRef::ExprCall(call_node) => { + fn get_node(node: ast::AnyNodeRef, argument_index: Option) -> ast::AnyNodeRef { + // If we have a Call node and an argument index, report the diagnostic on the correct + // argument node; otherwise, report it on the entire provided node. + match (node, argument_index) { + (ast::AnyNodeRef::ExprCall(call_node), Some(argument_index)) => { match call_node .arguments .arguments_source_order()