Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] handle synthetic 'self' argument in call-binding diagnostics #15362

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -48,6 +48,8 @@ impl<'a, 'db> FromIterator<Argument<'a, 'db>> 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`).
Expand All @@ -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,
Expand Down
46 changes: 32 additions & 14 deletions crates/red_knot_python_semantic/src/types/call/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;
};
Expand All @@ -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,
});
Expand All @@ -74,15 +89,18 @@ 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),
});
}
}
}
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,
});
Expand Down Expand Up @@ -243,7 +261,7 @@ pub(crate) enum CallBindingError<'db> {
/// parameter.
InvalidArgumentType {
parameter: ParameterContext,
argument_index: usize,
argument_index: Option<usize>,
expected_ty: Type<'db>,
provided_ty: Type<'db>,
},
Expand All @@ -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<usize>,
},
/// 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<usize>,
expected_positional_count: usize,
provided_positional_count: usize,
},
/// Multiple arguments were provided for a single parameter.
ParameterAlreadyAssigned {
argument_index: usize,
argument_index: Option<usize>,
parameter: ParameterContext,
},
}
Expand Down Expand Up @@ -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<usize>) -> 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()
Expand Down
Loading