diff --git a/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md b/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md index cff18e7fb2..cc096c53dd 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md +++ b/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md @@ -40,9 +40,9 @@ class C: return 42 x = C() +# error: [invalid-argument-type] x -= 1 -# TODO: should error, once operand type check is implemented reveal_type(x) # revealed: int ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/function.md b/crates/red_knot_python_semantic/resources/mdtest/call/function.md index af7c1e2582..435c9bbe21 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/call/function.md +++ b/crates/red_knot_python_semantic/resources/mdtest/call/function.md @@ -64,3 +64,196 @@ def _(flag: bool): # error: [possibly-unresolved-reference] reveal_type(foo()) # revealed: int ``` + +## Wrong argument type + +### Positional argument, positional-or-keyword parameter + +```py +def f(x: int) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter 1 (`x`) of function `f`; expected type `int`" +reveal_type(f("foo")) # revealed: int +``` + +### Positional argument, positional-only parameter + +```py +def f(x: int, /) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter 1 (`x`) of function `f`; expected type `int`" +reveal_type(f("foo")) # revealed: int +``` + +### Positional argument, variadic parameter + +```py +def f(*args: int) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter `*args` of function `f`; expected type `int`" +reveal_type(f("foo")) # revealed: int +``` + +### Keyword argument, positional-or-keyword parameter + +```py +def f(x: int) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter `x` of function `f`; expected type `int`" +reveal_type(f(x="foo")) # revealed: int +``` + +### Keyword argument, keyword-only parameter + +```py +def f(*, x: int) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter `x` of function `f`; expected type `int`" +reveal_type(f(x="foo")) # revealed: int +``` + +### Keyword argument, keywords parameter + +```py +def f(**kwargs: int) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter `**kwargs` of function `f`; expected type `int`" +reveal_type(f(x="foo")) # revealed: int +``` + +### Correctly match keyword out-of-order + +```py +def f(x: int = 1, y: str = "foo") -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal[2]` cannot be assigned to parameter `y` of function `f`; expected type `str`" +# error: 20 [invalid-argument-type] "Object of type `Literal["bar"]` cannot be assigned to parameter `x` of function `f`; expected type `int`" +reveal_type(f(y=2, x="bar")) # revealed: int +``` + +## Too many positional arguments + +### One too many + +```py +def f() -> int: + return 1 + +# error: 15 [too-many-positional-arguments] "Too many positional arguments to function `f`: expected 0, got 1" +reveal_type(f("foo")) # revealed: int +``` + +### Two too many + +```py +def f() -> int: + return 1 + +# error: 15 [too-many-positional-arguments] "Too many positional arguments to function `f`: expected 0, got 2" +reveal_type(f("foo", "bar")) # revealed: int +``` + +### No too-many-positional if variadic is taken + +```py +def f(*args: int) -> int: + return 1 + +reveal_type(f(1, 2, 3)) # revealed: int +``` + +## Missing arguments + +### No defaults or variadic + +```py +def f(x: int) -> int: + return 1 + +# error: 13 [missing-argument] "No argument provided for required parameter `x` of function `f`" +reveal_type(f()) # revealed: int +``` + +### With default + +```py +def f(x: int, y: str = "foo") -> int: + return 1 + +# error: 13 [missing-argument] "No argument provided for required parameter `x` of function `f`" +reveal_type(f()) # revealed: int +``` + +### Defaulted argument is not required + +```py +def f(x: int = 1) -> int: + return 1 + +reveal_type(f()) # revealed: int +``` + +### With variadic + +```py +def f(x: int, *y: str) -> int: + return 1 + +# error: 13 [missing-argument] "No argument provided for required parameter `x` of function `f`" +reveal_type(f()) # revealed: int +``` + +### Variadic argument is not required + +```py +def f(*args: int) -> int: + return 1 + +reveal_type(f()) # revealed: int +``` + +### Keywords argument is not required + +```py +def f(**kwargs: int) -> int: + return 1 + +reveal_type(f()) # revealed: int +``` + +### Multiple + +```py +def f(x: int, y: int) -> int: + return 1 + +# error: 13 [missing-argument] "No arguments provided for required parameters `x`, `y` of function `f`" +reveal_type(f()) # revealed: int +``` + +## Unknown argument + +```py +def f(x: int) -> int: + return 1 + +# error: 20 [unknown-argument] "Argument `y` does not match any known parameter of function `f`" +reveal_type(f(x=1, y=2)) # revealed: int +``` + +## Parameter already assigned + +```py +def f(x: int) -> int: + return 1 + +# error: 18 [parameter-already-assigned] "Multiple values provided for parameter `x` of function `f`" +reveal_type(f(1, x=2)) # revealed: int +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/invalid_syntax.md b/crates/red_knot_python_semantic/resources/mdtest/call/invalid_syntax.md new file mode 100644 index 0000000000..e919d26adf --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/call/invalid_syntax.md @@ -0,0 +1,44 @@ +# Invalid signatures + +## Multiple arguments with the same name + +We always map a keyword argument to the first parameter of that name. + +```py +# error: [invalid-syntax] "Duplicate parameter "x"" +def f(x: int, x: str) -> int: + return 1 + +# error: 13 [missing-argument] "No argument provided for required parameter `x` of function `f`" +# error: 18 [parameter-already-assigned] "Multiple values provided for parameter `x` of function `f`" +reveal_type(f(1, x=2)) # revealed: int +``` + +## Positional after non-positional + +When parameter kinds are given in an invalid order, we emit a diagnostic and implicitly reorder them +to the valid order: + +```py +# error: [invalid-syntax] "Parameter cannot follow var-keyword parameter" +def f(**kw: int, x: str) -> int: + return 1 + +# error: 15 [invalid-argument-type] "Object of type `Literal[1]` cannot be assigned to parameter 1 (`x`) of function `f`; expected type `str`" +reveal_type(f(1)) # revealed: int +``` + +## Non-defaulted after defaulted + +We emit a syntax diagnostic for this, but it doesn't cause any problems for binding. + +```py +# error: [invalid-syntax] "Parameter without a default cannot follow a parameter with a default" +def f(x: int = 1, y: str) -> int: + return 1 + +reveal_type(f(y="foo")) # revealed: int +# error: [invalid-argument-type] "Object of type `Literal["foo"]` cannot be assigned to parameter 1 (`x`) of function `f`; expected type `int`" +# error: [missing-argument] "No argument provided for required parameter `y` of function `f`" +reveal_type(f("foo")) # revealed: int +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/isinstance.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/isinstance.md index 58850844ff..3c1e6730f4 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/isinstance.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/isinstance.md @@ -169,8 +169,7 @@ def _(flag: bool): def _(flag: bool): x = 1 if flag else "a" - # TODO: this should cause us to emit a diagnostic - # (`isinstance` has no `foo` parameter) + # error: [unknown-argument] if isinstance(x, int, foo="bar"): reveal_type(x) # revealed: Literal[1] | Literal["a"] ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/issubclass.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/issubclass.md index 7d5697e4f7..119f767c38 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/issubclass.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/issubclass.md @@ -146,7 +146,7 @@ class A: ... t = object() -# TODO: we should emit a diagnostic here +# error: [invalid-argument-type] if issubclass(t, A): reveal_type(t) # revealed: type[A] ``` @@ -160,7 +160,7 @@ branch: ```py t = 1 -# TODO: we should emit a diagnostic here +# error: [invalid-argument-type] if issubclass(t, int): reveal_type(t) # revealed: Never ``` @@ -234,8 +234,7 @@ def flag() -> bool: ... t = int if flag() else str -# TODO: this should cause us to emit a diagnostic -# (`issubclass` has no `foo` parameter) +# error: [unknown-argument] if issubclass(t, int, foo="bar"): reveal_type(t) # revealed: Literal[int, str] ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6cbf49909c..d7d2043582 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -31,7 +31,7 @@ use crate::semantic_index::{ use crate::stdlib::{builtins_symbol, known_module_symbol, typing_extensions_symbol}; use crate::suppression::check_suppressions; use crate::symbol::{Boundness, Symbol}; -use crate::types::call::{CallDunderResult, CallOutcome}; +use crate::types::call::{bind_call, CallArguments, CallBinding, CallDunderResult, CallOutcome}; use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; use crate::types::mro::{Mro, MroError, MroIterator}; @@ -1681,11 +1681,13 @@ impl<'db> Type<'db> { return Truthiness::Ambiguous; } - if let Some(Type::BooleanLiteral(bool_val)) = - bool_method.call(db, &[*instance_ty]).return_ty(db) + if let Some(Type::BooleanLiteral(bool_val)) = bool_method + .call(db, &CallArguments::positional([*instance_ty])) + .return_ty(db) { bool_val.into() } else { + // TODO diagnostic if not assignable to bool Truthiness::Ambiguous } } @@ -1753,7 +1755,7 @@ impl<'db> Type<'db> { return usize_len.try_into().ok().map(Type::IntLiteral); } - let return_ty = match self.call_dunder(db, "__len__", &[*self]) { + let return_ty = match self.call_dunder(db, "__len__", &CallArguments::positional([*self])) { // TODO: emit a diagnostic CallDunderResult::MethodNotAvailable => return None, @@ -1767,49 +1769,47 @@ impl<'db> Type<'db> { /// Return the outcome of calling an object of this type. #[must_use] - fn call(self, db: &'db dyn Db, arg_types: &[Type<'db>]) -> CallOutcome<'db> { + fn call(self, db: &'db dyn Db, arguments: &CallArguments<'db>) -> CallOutcome<'db> { match self { - // TODO validate typed call arguments vs callable signature - Type::FunctionLiteral(function_type) => match function_type.known(db) { - Some(KnownFunction::RevealType) => CallOutcome::revealed( - function_type.signature(db).return_ty, - *arg_types.first().unwrap_or(&Type::Unknown), - ), + Type::FunctionLiteral(function_type) => { + let mut binding = bind_call(db, arguments, function_type.signature(db), Some(self)); + match function_type.known(db) { + Some(KnownFunction::RevealType) => { + let revealed_ty = binding.first_parameter().unwrap_or(Type::Unknown); + CallOutcome::revealed(binding, revealed_ty) + } - Some(KnownFunction::Len) => { - let normal_return_ty = function_type.signature(db).return_ty; + Some(KnownFunction::Len) => { + if let Some(first_arg) = binding.first_parameter() { + if let Some(len_ty) = first_arg.len(db) { + binding.set_return_ty(len_ty); + } + }; - let [only_arg] = arg_types else { - // TODO: Emit a diagnostic - return CallOutcome::callable(normal_return_ty); - }; - let len_ty = only_arg.len(db); + CallOutcome::callable(binding) + } - CallOutcome::callable(len_ty.unwrap_or(normal_return_ty)) + _ => CallOutcome::callable(binding), } - - _ => CallOutcome::callable(function_type.signature(db).return_ty), - }, + } // TODO annotated return type on `__new__` or metaclass `__call__` + // TODO check call vs signatures of `__new__` and/or `__init__` Type::ClassLiteral(ClassLiteralType { class }) => { - CallOutcome::callable(match class.known(db) { + CallOutcome::callable(CallBinding::from_return_ty(match class.known(db) { // If the class is the builtin-bool class (for example `bool(1)`), we try to // return the specific truthiness value of the input arg, `Literal[True]` for // the example above. - Some(KnownClass::Bool) => arg_types - .first() + Some(KnownClass::Bool) => arguments + .first_argument() .map(|arg| arg.bool(db).into_type(db)) .unwrap_or(Type::BooleanLiteral(false)), _ => Type::Instance(InstanceType { class }), - }) + })) } instance_ty @ Type::Instance(_) => { - let args = std::iter::once(self) - .chain(arg_types.iter().copied()) - .collect::>(); - match instance_ty.call_dunder(db, "__call__", &args) { + match instance_ty.call_dunder(db, "__call__", &arguments.with_self(instance_ty)) { CallDunderResult::CallOutcome(CallOutcome::NotCallable { .. }) => { // Turn "`` not callable" into // "`X` not callable" @@ -1836,17 +1836,21 @@ impl<'db> Type<'db> { } // Dynamic types are callable, and the return type is the same dynamic type - Type::Any | Type::Todo(_) | Type::Unknown => CallOutcome::callable(self), + Type::Any | Type::Todo(_) | Type::Unknown => { + CallOutcome::callable(CallBinding::from_return_ty(self)) + } Type::Union(union) => CallOutcome::union( self, union .elements(db) .iter() - .map(|elem| elem.call(db, arg_types)), + .map(|elem| elem.call(db, arguments)), ), - Type::Intersection(_) => CallOutcome::callable(todo_type!("Type::Intersection.call()")), + Type::Intersection(_) => CallOutcome::callable(CallBinding::from_return_ty( + todo_type!("Type::Intersection.call()"), + )), _ => CallOutcome::not_callable(self), } @@ -1857,14 +1861,14 @@ impl<'db> Type<'db> { self, db: &'db dyn Db, name: &str, - arg_types: &[Type<'db>], + arguments: &CallArguments<'db>, ) -> CallDunderResult<'db> { match self.to_meta_type(db).member(db, name) { Symbol::Type(callable_ty, Boundness::Bound) => { - CallDunderResult::CallOutcome(callable_ty.call(db, arg_types)) + CallDunderResult::CallOutcome(callable_ty.call(db, arguments)) } Symbol::Type(callable_ty, Boundness::PossiblyUnbound) => { - CallDunderResult::PossiblyUnbound(callable_ty.call(db, arg_types)) + CallDunderResult::PossiblyUnbound(callable_ty.call(db, arguments)) } Symbol::Unbound => CallDunderResult::MethodNotAvailable, } @@ -1885,7 +1889,8 @@ impl<'db> Type<'db> { }; } - let dunder_iter_result = self.call_dunder(db, "__iter__", &[self]); + let dunder_iter_result = + self.call_dunder(db, "__iter__", &CallArguments::positional([self])); match dunder_iter_result { CallDunderResult::CallOutcome(ref call_outcome) | CallDunderResult::PossiblyUnbound(ref call_outcome) => { @@ -1896,7 +1901,7 @@ impl<'db> Type<'db> { }; return if let Some(element_ty) = iterator_ty - .call_dunder(db, "__next__", &[iterator_ty]) + .call_dunder(db, "__next__", &CallArguments::positional([iterator_ty])) .return_ty(db) { if matches!(dunder_iter_result, CallDunderResult::PossiblyUnbound(..)) { @@ -1923,7 +1928,11 @@ impl<'db> Type<'db> { // TODO(Alex) this is only valid if the `__getitem__` method is annotated as // accepting `int` or `SupportsIndex` if let Some(element_ty) = self - .call_dunder(db, "__getitem__", &[self, KnownClass::Int.to_instance(db)]) + .call_dunder( + db, + "__getitem__", + &CallArguments::positional([self, KnownClass::Int.to_instance(db)]), + ) .return_ty(db) { IterationOutcome::Iterable { element_ty } diff --git a/crates/red_knot_python_semantic/src/types/call.rs b/crates/red_knot_python_semantic/src/types/call.rs index 87d16d9478..d803b6103d 100644 --- a/crates/red_knot_python_semantic/src/types/call.rs +++ b/crates/red_knot_python_semantic/src/types/call.rs @@ -1,17 +1,23 @@ use super::context::InferContext; use super::diagnostic::CALL_NON_CALLABLE; -use super::{Severity, Type, TypeArrayDisplay, UnionBuilder}; +use super::{Severity, Signature, Type, TypeArrayDisplay, UnionBuilder}; use crate::Db; use ruff_db::diagnostic::DiagnosticId; use ruff_python_ast as ast; +mod arguments; +mod bind; + +pub(super) use arguments::{Argument, CallArguments}; +pub(super) use bind::{bind_call, CallBinding}; + #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum CallOutcome<'db> { Callable { - return_ty: Type<'db>, + binding: CallBinding<'db>, }, RevealType { - return_ty: Type<'db>, + binding: CallBinding<'db>, revealed_ty: Type<'db>, }, NotCallable { @@ -29,8 +35,8 @@ pub(super) enum CallOutcome<'db> { impl<'db> CallOutcome<'db> { /// Create a new `CallOutcome::Callable` with given return type. - pub(super) fn callable(return_ty: Type<'db>) -> CallOutcome<'db> { - CallOutcome::Callable { return_ty } + pub(super) fn callable(binding: CallBinding<'db>) -> CallOutcome<'db> { + CallOutcome::Callable { binding } } /// Create a new `CallOutcome::NotCallable` with given not-callable type. @@ -39,9 +45,9 @@ impl<'db> CallOutcome<'db> { } /// Create a new `CallOutcome::RevealType` with given revealed and return types. - pub(super) fn revealed(return_ty: Type<'db>, revealed_ty: Type<'db>) -> CallOutcome<'db> { + pub(super) fn revealed(binding: CallBinding<'db>, revealed_ty: Type<'db>) -> CallOutcome<'db> { CallOutcome::RevealType { - return_ty, + binding, revealed_ty, } } @@ -60,11 +66,11 @@ impl<'db> CallOutcome<'db> { /// Get the return type of the call, or `None` if not callable. pub(super) fn return_ty(&self, db: &'db dyn Db) -> Option> { match self { - Self::Callable { return_ty } => Some(*return_ty), + Self::Callable { binding } => Some(binding.return_ty()), Self::RevealType { - return_ty, + binding, revealed_ty: _, - } => Some(*return_ty), + } => Some(binding.return_ty()), Self::NotCallable { not_callable_ty: _ } => None, Self::Union { outcomes, @@ -163,10 +169,16 @@ impl<'db> CallOutcome<'db> { context: &InferContext<'db>, node: ast::AnyNodeRef, ) -> Result, NotCallableError<'db>> { + // TODO should this method emit diagnostics directly, or just return results that allow the + // caller to decide about emitting diagnostics? Currently it emits binding diagnostics, but + // only non-callable diagnostics in the union case, which is inconsistent. match self { - Self::Callable { return_ty } => Ok(*return_ty), + Self::Callable { binding } => { + binding.report_diagnostics(context, node); + Ok(binding.return_ty()) + } Self::RevealType { - return_ty, + binding, revealed_ty, } => { context.report_diagnostic( @@ -175,7 +187,7 @@ impl<'db> CallOutcome<'db> { Severity::Info, format_args!("Revealed type is `{}`", revealed_ty.display(context.db())), ); - Ok(*return_ty) + Ok(binding.return_ty()) } Self::NotCallable { not_callable_ty } => Err(NotCallableError::Type { not_callable_ty: *not_callable_ty, @@ -204,11 +216,11 @@ impl<'db> CallOutcome<'db> { Type::Unknown } Self::RevealType { - return_ty, + binding, revealed_ty: _, } => { if revealed { - *return_ty + binding.return_ty() } else { revealed = true; outcome.unwrap_with_diagnostic(context, node) diff --git a/crates/red_knot_python_semantic/src/types/call/arguments.rs b/crates/red_knot_python_semantic/src/types/call/arguments.rs new file mode 100644 index 0000000000..6a247a0b65 --- /dev/null +++ b/crates/red_knot_python_semantic/src/types/call/arguments.rs @@ -0,0 +1,71 @@ +use super::Type; +use ruff_python_ast::name::Name; + +/// Typed arguments for a single call, in source order. +#[derive(Clone, Debug, Default)] +pub(crate) struct CallArguments<'db>(Vec>); + +impl<'db> CallArguments<'db> { + /// Create a [`CallArguments`] from an iterator over non-variadic positional argument types. + pub(crate) fn positional(positional_tys: impl IntoIterator>) -> Self { + positional_tys + .into_iter() + .map(Argument::Positional) + .collect() + } + + /// 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.extend_from_slice(&self.0); + Self(arguments) + } + + pub(crate) fn iter(&self) -> impl Iterator> { + self.0.iter() + } + + // TODO this should be eliminated in favor of [`bind_call`] + pub(crate) fn first_argument(&self) -> Option> { + self.0.first().map(Argument::ty) + } +} + +impl<'db, 'a> IntoIterator for &'a CallArguments<'db> { + type Item = &'a Argument<'db>; + type IntoIter = std::slice::Iter<'a, Argument<'db>>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl<'db> FromIterator> for CallArguments<'db> { + fn from_iter>>(iter: T) -> Self { + Self(iter.into_iter().collect()) + } +} + +#[derive(Clone, Debug)] +pub(crate) enum Argument<'db> { + /// A positional argument. + Positional(Type<'db>), + /// A starred positional argument (e.g. `*args`). + Variadic(Type<'db>), + /// A keyword argument (e.g. `a=1`). + Keyword { name: Name, ty: Type<'db> }, + /// The double-starred keywords argument (e.g. `**kwargs`). + Keywords(Type<'db>), +} + +impl<'db> Argument<'db> { + fn ty(&self) -> Type<'db> { + match self { + Self::Positional(ty) => *ty, + Self::Variadic(ty) => *ty, + Self::Keyword { name: _, ty } => *ty, + Self::Keywords(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 new file mode 100644 index 0000000000..0fcb40140e --- /dev/null +++ b/crates/red_knot_python_semantic/src/types/call/bind.rs @@ -0,0 +1,383 @@ +use super::{Argument, CallArguments, InferContext, Signature, Type}; +use crate::db::Db; +use crate::types::diagnostic::{ + INVALID_ARGUMENT_TYPE, MISSING_ARGUMENT, PARAMETER_ALREADY_ASSIGNED, + TOO_MANY_POSITIONAL_ARGUMENTS, UNKNOWN_ARGUMENT, +}; +use crate::types::signatures::Parameter; +use crate::types::UnionType; +use ruff_python_ast as ast; + +/// Bind a [`CallArguments`] against a callable [`Signature`]. +/// +/// The returned [`CallBinding`] provides the return type of the call, the bound types for all +/// parameters, and any errors resulting from binding the call. +pub(crate) fn bind_call<'db>( + db: &'db dyn Db, + arguments: &CallArguments<'db>, + signature: &Signature<'db>, + callable_ty: Option>, +) -> CallBinding<'db> { + let parameters = signature.parameters(); + // The type assigned to each parameter at this call site. + let mut parameter_tys = vec![None; parameters.len()]; + let mut errors = vec![]; + let mut next_positional = 0; + let mut first_excess_positional = None; + for (argument_index, argument) in arguments.iter().enumerate() { + let (index, parameter, argument_ty, positional) = match argument { + Argument::Positional(ty) => { + let Some((index, parameter)) = parameters + .get_positional(next_positional) + .map(|param| (next_positional, param)) + .or_else(|| parameters.variadic()) + else { + first_excess_positional.get_or_insert(argument_index); + next_positional += 1; + continue; + }; + next_positional += 1; + (index, parameter, ty, !parameter.is_variadic()) + } + Argument::Keyword { name, ty } => { + let Some((index, parameter)) = parameters + .keyword_by_name(name) + .or_else(|| parameters.keyword_variadic()) + else { + errors.push(CallBindingError::UnknownArgument { + argument_name: name.clone(), + argument_index, + }); + continue; + }; + (index, parameter, ty, false) + } + + Argument::Variadic(_) | Argument::Keywords(_) => { + // TODO + continue; + } + }; + if let Some(expected_ty) = parameter.annotated_ty() { + if !argument_ty.is_assignable_to(db, expected_ty) { + errors.push(CallBindingError::InvalidArgumentType { + parameter: ParameterContext::new(parameter, index, positional), + argument_index, + expected_ty, + provided_ty: *argument_ty, + }); + } + } + if let Some(existing) = parameter_tys[index].replace(*argument_ty) { + if parameter.is_variadic() { + let union = UnionType::from_elements(db, [existing, *argument_ty]); + parameter_tys[index].replace(union); + } else { + errors.push(CallBindingError::ParameterAlreadyAssigned { + argument_index, + parameter: ParameterContext::new(parameter, index, positional), + }); + } + } + } + if let Some(first_excess_argument_index) = first_excess_positional { + errors.push(CallBindingError::TooManyPositionalArguments { + first_excess_argument_index, + expected_positional_count: parameters.positional().count(), + provided_positional_count: next_positional, + }); + } + let mut missing = vec![]; + for (index, bound_ty) in parameter_tys.iter().enumerate() { + if bound_ty.is_none() { + let param = ¶meters[index]; + if param.is_variadic() || param.is_keyword_variadic() || param.default_ty().is_some() { + // variadic/keywords and defaulted arguments are not required + continue; + } + missing.push(ParameterContext::new(param, index, false)); + } + } + + if !missing.is_empty() { + errors.push(CallBindingError::MissingArguments { + parameters: ParameterContexts(missing), + }); + } + + CallBinding { + callable_ty, + return_ty: signature.return_ty.unwrap_or(Type::Unknown), + parameter_tys: parameter_tys + .into_iter() + .map(|opt_ty| opt_ty.unwrap_or(Type::Unknown)) + .collect(), + errors, + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct CallBinding<'db> { + /// Type of the callable object (function, class...) + callable_ty: Option>, + + /// Return type of the call. + return_ty: Type<'db>, + + /// Bound types for parameters, in parameter source order. + parameter_tys: Box<[Type<'db>]>, + + /// Call binding errors, if any. + errors: Vec>, +} + +impl<'db> CallBinding<'db> { + // TODO remove this constructor and construct always from `bind_call` + pub(crate) fn from_return_ty(return_ty: Type<'db>) -> Self { + Self { + callable_ty: None, + return_ty, + parameter_tys: Box::default(), + errors: vec![], + } + } + + pub(crate) fn set_return_ty(&mut self, return_ty: Type<'db>) { + self.return_ty = return_ty; + } + + pub(crate) fn return_ty(&self) -> Type<'db> { + self.return_ty + } + + pub(crate) fn parameter_tys(&self) -> &[Type<'db>] { + &self.parameter_tys + } + + pub(crate) fn first_parameter(&self) -> Option> { + self.parameter_tys().first().copied() + } + + fn callable_name(&self, db: &'db dyn Db) -> Option<&ast::name::Name> { + match self.callable_ty { + Some(Type::FunctionLiteral(function)) => Some(function.name(db)), + Some(Type::ClassLiteral(class_type)) => Some(class_type.class.name(db)), + _ => None, + } + } + + pub(super) fn report_diagnostics(&self, context: &InferContext<'db>, node: ast::AnyNodeRef) { + let callable_name = self.callable_name(context.db()); + for error in &self.errors { + error.report_diagnostic(context, node, callable_name); + } + } +} + +/// Information needed to emit a diagnostic regarding a parameter. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ParameterContext { + name: Option, + index: usize, + + /// Was the argument for this parameter passed positionally, and matched to a non-variadic + /// positional parameter? (If so, we will provide the index in the diagnostic, not just the + /// name.) + positional: bool, +} + +impl ParameterContext { + fn new(parameter: &Parameter, index: usize, positional: bool) -> Self { + Self { + name: parameter.display_name(), + index, + positional, + } + } +} + +impl std::fmt::Display for ParameterContext { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(name) = &self.name { + if self.positional { + write!(f, "{} (`{name}`)", self.index + 1) + } else { + write!(f, "`{name}`") + } + } else { + write!(f, "{}", self.index + 1) + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ParameterContexts(Vec); + +impl std::fmt::Display for ParameterContexts { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut iter = self.0.iter(); + if let Some(first) = iter.next() { + write!(f, "{first}")?; + for param in iter { + f.write_str(", ")?; + write!(f, "{param}")?; + } + } + Ok(()) + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum CallBindingError<'db> { + /// The type of an argument is not assignable to the annotated type of its corresponding + /// parameter. + InvalidArgumentType { + parameter: ParameterContext, + argument_index: usize, + expected_ty: Type<'db>, + provided_ty: Type<'db>, + }, + /// One or more required parameters (that is, with no default) is not supplied by any argument. + MissingArguments { parameters: ParameterContexts }, + /// A call argument can't be matched to any parameter. + UnknownArgument { + argument_name: ast::name::Name, + argument_index: usize, + }, + /// More positional arguments are provided in the call than can be handled by the signature. + TooManyPositionalArguments { + first_excess_argument_index: usize, + expected_positional_count: usize, + provided_positional_count: usize, + }, + /// Multiple arguments were provided for a single parameter. + ParameterAlreadyAssigned { + argument_index: usize, + parameter: ParameterContext, + }, +} + +impl<'db> CallBindingError<'db> { + pub(super) fn report_diagnostic( + &self, + context: &InferContext<'db>, + node: ast::AnyNodeRef, + callable_name: Option<&ast::name::Name>, + ) { + match self { + Self::InvalidArgumentType { + parameter, + argument_index, + expected_ty, + provided_ty, + } => { + let provided_ty_display = provided_ty.display(context.db()); + let expected_ty_display = expected_ty.display(context.db()); + context.report_lint( + &INVALID_ARGUMENT_TYPE, + Self::get_node(node, *argument_index), + 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}`") + } else { + String::new() + } + ), + ); + } + + Self::TooManyPositionalArguments { + first_excess_argument_index, + expected_positional_count, + provided_positional_count, + } => { + context.report_lint( + &TOO_MANY_POSITIONAL_ARGUMENTS, + Self::get_node(node, *first_excess_argument_index), + 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}`") + } else { + String::new() + } + ), + ); + } + + Self::MissingArguments { parameters } => { + let s = if parameters.0.len() == 1 { "" } else { "s" }; + context.report_lint( + &MISSING_ARGUMENT, + node, + format_args!( + "No argument{s} provided for required parameter{s} {parameters}{}", + if let Some(callable_name) = callable_name { + format!(" of function `{callable_name}`") + } else { + String::new() + } + ), + ); + } + + Self::UnknownArgument { + argument_name, + argument_index, + } => { + context.report_lint( + &UNKNOWN_ARGUMENT, + 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}`") + } else { + String::new() + } + ), + ); + } + + Self::ParameterAlreadyAssigned { + argument_index, + parameter, + } => { + context.report_lint( + &PARAMETER_ALREADY_ASSIGNED, + 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}`") + } else { + String::new() + } + ), + ); + } + } + } + + 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) => { + match call_node + .arguments + .arguments_source_order() + .nth(argument_index) + .expect("argument index should not be out of range") + { + ast::ArgOrKeyword::Arg(expr) => expr.into(), + ast::ArgOrKeyword::Keyword(keyword) => keyword.into(), + } + } + _ => node, + } + } +} diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 65f85c5f61..c2468e7847 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -30,6 +30,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&INCOMPATIBLE_SLOTS); registry.register_lint(&INCONSISTENT_MRO); registry.register_lint(&INDEX_OUT_OF_BOUNDS); + registry.register_lint(&INVALID_ARGUMENT_TYPE); registry.register_lint(&INVALID_ASSIGNMENT); registry.register_lint(&INVALID_BASE); registry.register_lint(&INVALID_CONTEXT_MANAGER); @@ -39,13 +40,17 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&INVALID_RAISE); registry.register_lint(&INVALID_TYPE_FORM); registry.register_lint(&INVALID_TYPE_VARIABLE_CONSTRAINTS); + registry.register_lint(&MISSING_ARGUMENT); registry.register_lint(&NON_SUBSCRIPTABLE); registry.register_lint(&NOT_ITERABLE); + registry.register_lint(&PARAMETER_ALREADY_ASSIGNED); registry.register_lint(&POSSIBLY_UNBOUND_ATTRIBUTE); registry.register_lint(&POSSIBLY_UNBOUND_IMPORT); registry.register_lint(&POSSIBLY_UNRESOLVED_REFERENCE); registry.register_lint(&SUBCLASS_OF_FINAL_CLASS); + registry.register_lint(&TOO_MANY_POSITIONAL_ARGUMENTS); registry.register_lint(&UNDEFINED_REVEAL); + registry.register_lint(&UNKNOWN_ARGUMENT); registry.register_lint(&UNRESOLVED_ATTRIBUTE); registry.register_lint(&UNRESOLVED_IMPORT); registry.register_lint(&UNRESOLVED_REFERENCE); @@ -226,6 +231,27 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Detects call arguments whose type is not assignable to the corresponding typed parameter. + /// + /// ## Why is this bad? + /// Passing an argument of a type the function (or callable object) does not accept violates + /// the expectations of the function author and may cause unexpected runtime errors within the + /// body of the function. + /// + /// ## Examples + /// ```python + /// def func(x: int): ... + /// func("foo") # error: [invalid-argument-type] + /// ``` + pub(crate) static INVALID_ARGUMENT_TYPE = { + summary: "detects call arguments whose type is not assignable to the corresponding typed parameter", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// TODO #14889 pub(crate) static INVALID_ASSIGNMENT = { @@ -375,6 +401,25 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for missing required arguments in a call. + /// + /// ## Why is this bad? + /// Failing to provide a required argument will raise a `TypeError` at runtime. + /// + /// ## Examples + /// ```python + /// def func(x: int): ... + /// func() # TypeError: func() missing 1 required positional argument: 'x' + /// ``` + pub(crate) static MISSING_ARGUMENT = { + summary: "detects missing required arguments in a call", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for subscripting objects that do not support subscripting. @@ -413,6 +458,27 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for calls which provide more than one argument for a single parameter. + /// + /// ## Why is this bad? + /// Providing multiple values for a single parameter will raise a `TypeError` at runtime. + /// + /// ## Examples + /// + /// ```python + /// def f(x: int) -> int: ... + /// + /// f(1, x=2) # Error raised here + /// ``` + pub(crate) static PARAMETER_ALREADY_ASSIGNED = { + summary: "detects multiple arguments for the same parameter", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for possibly unbound attributes. @@ -479,6 +545,27 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for calls that pass more positional arguments than the callable can accept. + /// + /// ## Why is this bad? + /// Passing too many positional arguments will raise `TypeError` at runtime. + /// + /// ## Example + /// + /// ```python + /// def f(): ... + /// + /// f("foo") # Error raised here + /// ``` + pub(crate) static TOO_MANY_POSITIONAL_ARGUMENTS = { + summary: "detects calls passing too many positional arguments", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for calls to `reveal_type` without importing it. @@ -495,6 +582,27 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for keyword arguments in calls that don't match any parameter of the callable. + /// + /// ## Why is this bad? + /// Providing an unknown keyword argument will raise `TypeError` at runtime. + /// + /// ## Example + /// + /// ```python + /// def f(x: int) -> int: ... + /// + /// f(x=1, y=2) # Error raised here + /// ``` + pub(crate) static UNKNOWN_ARGUMENT = { + summary: "detects unknown keyword arguments in calls", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for unresolved attributes. diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 04e19a6e63..48d5e8cd3f 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -49,6 +49,7 @@ use crate::semantic_index::semantic_index; use crate::semantic_index::symbol::{NodeWithScopeKind, NodeWithScopeRef, ScopeId}; use crate::semantic_index::SemanticIndex; use crate::stdlib::builtins_module_scope; +use crate::types::call::{Argument, CallArguments}; use crate::types::diagnostic::{ report_invalid_assignment, report_unresolved_module, TypeCheckDiagnostics, CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, @@ -1530,7 +1531,7 @@ impl<'db> TypeInferenceBuilder<'db> { } let target_ty = enter_ty - .call(self.db(), &[context_expression_ty]) + .call(self.db(), &CallArguments::positional([context_expression_ty])) .return_ty_result(&self.context, context_expression.into()) .unwrap_or_else(|err| { self.context.report_lint( @@ -1571,12 +1572,12 @@ impl<'db> TypeInferenceBuilder<'db> { if exit_ty .call( self.db(), - &[ + &CallArguments::positional([ context_manager_ty, Type::none(self.db()), Type::none(self.db()), Type::none(self.db()), - ], + ]), ) .return_ty_result(&self.context, context_expression.into()) .is_err() @@ -2024,7 +2025,10 @@ impl<'db> TypeInferenceBuilder<'db> { if let Symbol::Type(class_member, boundness) = class.class_member(self.db(), op.in_place_dunder()) { - let call = class_member.call(self.db(), &[target_type, value_type]); + let call = class_member.call( + self.db(), + &CallArguments::positional([target_type, value_type]), + ); let augmented_return_ty = match call .return_ty_result(&self.context, AnyNodeRef::StmtAugAssign(assignment)) { @@ -2483,25 +2487,43 @@ impl<'db> TypeInferenceBuilder<'db> { self.infer_expression(expression) } - fn infer_arguments(&mut self, arguments: &ast::Arguments) -> Vec> { - let mut types = Vec::with_capacity( - arguments - .args - .len() - .saturating_add(arguments.keywords.len()), - ); - - types.extend(arguments.args.iter().map(|arg| self.infer_expression(arg))); - - types.extend(arguments.keywords.iter().map( - |ast::Keyword { - range: _, - arg: _, - value, - }| self.infer_expression(value), - )); - - types + fn infer_arguments(&mut self, arguments: &ast::Arguments) -> CallArguments<'db> { + arguments + .arguments_source_order() + .map(|arg_or_keyword| { + match arg_or_keyword { + ast::ArgOrKeyword::Arg(arg) => match arg { + ast::Expr::Starred(ast::ExprStarred { + value, + range: _, + ctx: _, + }) => { + let ty = self.infer_expression(value); + self.store_expression_type(arg, ty); + Argument::Variadic(ty) + } + // TODO diagnostic if after a keyword argument + _ => Argument::Positional(self.infer_expression(arg)), + }, + ast::ArgOrKeyword::Keyword(ast::Keyword { + arg, + value, + range: _, + }) => { + let ty = self.infer_expression(value); + if let Some(arg) = arg { + Argument::Keyword { + name: arg.id.clone(), + ty, + } + } else { + // TODO diagnostic if not last + Argument::Keywords(ty) + } + } + } + }) + .collect() } fn infer_optional_expression(&mut self, expression: Option<&ast::Expr>) -> Option> { @@ -3012,12 +3034,11 @@ impl<'db> TypeInferenceBuilder<'db> { arguments, } = call_expression; - // TODO: proper typed call signature, representing keyword args etc - let arg_types = self.infer_arguments(arguments); + let call_arguments = self.infer_arguments(arguments); let function_type = self.infer_expression(func); function_type - .call(self.db(), arg_types.as_slice()) - .unwrap_with_diagnostic(&self.context, func.as_ref().into()) + .call(self.db(), &call_arguments) + .unwrap_with_diagnostic(&self.context, call_expression.into()) } fn infer_starred_expression(&mut self, starred: &ast::ExprStarred) -> Type<'db> { @@ -3316,9 +3337,11 @@ impl<'db> TypeInferenceBuilder<'db> { }; if let CallDunderResult::CallOutcome(call) - | CallDunderResult::PossiblyUnbound(call) = - operand_type.call_dunder(self.db(), unary_dunder_method, &[operand_type]) - { + | CallDunderResult::PossiblyUnbound(call) = operand_type.call_dunder( + self.db(), + unary_dunder_method, + &CallArguments::positional([operand_type]), + ) { match call.return_ty_result(&self.context, AnyNodeRef::ExprUnaryOp(unary)) { Ok(t) => t, Err(e) => { @@ -3571,11 +3594,19 @@ impl<'db> TypeInferenceBuilder<'db> { && rhs_reflected != left_class.member(self.db(), reflected_dunder) { return right_ty - .call_dunder(self.db(), reflected_dunder, &[right_ty, left_ty]) + .call_dunder( + self.db(), + reflected_dunder, + &CallArguments::positional([right_ty, left_ty]), + ) .return_ty(self.db()) .or_else(|| { left_ty - .call_dunder(self.db(), op.dunder(), &[left_ty, right_ty]) + .call_dunder( + self.db(), + op.dunder(), + &CallArguments::positional([left_ty, right_ty]), + ) .return_ty(self.db()) }); } @@ -3585,7 +3616,7 @@ impl<'db> TypeInferenceBuilder<'db> { left_class.member(self.db(), op.dunder()) { class_member - .call(self.db(), &[left_ty, right_ty]) + .call(self.db(), &CallArguments::positional([left_ty, right_ty])) .return_ty(self.db()) } else { None @@ -3599,7 +3630,7 @@ impl<'db> TypeInferenceBuilder<'db> { right_class.member(self.db(), op.reflected_dunder()) { class_member - .call(self.db(), &[right_ty, left_ty]) + .call(self.db(), &CallArguments::positional([right_ty, left_ty])) .return_ty(self.db()) } else { None @@ -4380,7 +4411,7 @@ impl<'db> TypeInferenceBuilder<'db> { } return dunder_getitem_method - .call(self.db(), &[slice_ty]) + .call(self.db(), &CallArguments::positional([value_ty, slice_ty])) .return_ty_result(&self.context, value_node.into()) .unwrap_or_else(|err| { self.context.report_lint( @@ -4425,7 +4456,7 @@ impl<'db> TypeInferenceBuilder<'db> { } return ty - .call(self.db(), &[slice_ty]) + .call(self.db(), &CallArguments::positional([value_ty, slice_ty])) .return_ty_result(&self.context, value_node.into()) .unwrap_or_else(|err| { self.context.report_lint( @@ -5506,7 +5537,10 @@ fn perform_rich_comparison<'db>( right: InstanceType<'db>| { match left.class.class_member(db, op.dunder()) { Symbol::Type(class_member_dunder, Boundness::Bound) => class_member_dunder - .call(db, &[Type::Instance(left), Type::Instance(right)]) + .call( + db, + &CallArguments::positional([Type::Instance(left), Type::Instance(right)]), + ) .return_ty(db), _ => None, } @@ -5550,7 +5584,10 @@ fn perform_membership_test_comparison<'db>( Symbol::Type(contains_dunder, Boundness::Bound) => { // If `__contains__` is available, it is used directly for the membership test. contains_dunder - .call(db, &[Type::Instance(right), Type::Instance(left)]) + .call( + db, + &CallArguments::positional([Type::Instance(right), Type::Instance(left)]), + ) .return_ty(db) } _ => { diff --git a/crates/red_knot_python_semantic/src/types/signatures.rs b/crates/red_knot_python_semantic/src/types/signatures.rs index 06b7e5c8f6..6c3caa4a5c 100644 --- a/crates/red_knot_python_semantic/src/types/signatures.rs +++ b/crates/red_knot_python_semantic/src/types/signatures.rs @@ -1,4 +1,3 @@ -#![allow(dead_code)] use super::{definition_expression_ty, Type}; use crate::Db; use crate::{semantic_index::definition::Definition, types::todo_type}; @@ -7,10 +6,18 @@ use ruff_python_ast::{self as ast, name::Name}; /// A typed callable signature. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct Signature<'db> { + /// Parameters, in source order. + /// + /// The ordering of parameters in a valid signature must be: first positional-only parameters, + /// then positional-or-keyword, then optionally the variadic parameter, then keyword-only + /// parameters, and last, optionally the variadic keywords parameter. Parameters with defaults + /// must come after parameters without defaults. + /// + /// We may get invalid signatures, though, and need to handle them without panicking. parameters: Parameters<'db>, - /// Annotated return type (Unknown if no annotation.) - pub(crate) return_ty: Type<'db>, + /// Annotated return type, if any. + pub(crate) return_ty: Option>, } impl<'db> Signature<'db> { @@ -18,7 +25,7 @@ impl<'db> Signature<'db> { pub(crate) fn todo() -> Self { Self { parameters: Parameters::todo(), - return_ty: todo_type!("return type"), + return_ty: Some(todo_type!("return type")), } } @@ -28,17 +35,13 @@ impl<'db> Signature<'db> { definition: Definition<'db>, function_node: &'db ast::StmtFunctionDef, ) -> Self { - let return_ty = function_node - .returns - .as_ref() - .map(|returns| { - if function_node.is_async { - todo_type!("generic types.CoroutineType") - } else { - definition_expression_ty(db, definition, returns.as_ref()) - } - }) - .unwrap_or(Type::Unknown); + let return_ty = function_node.returns.as_ref().map(|returns| { + if function_node.is_async { + todo_type!("generic types.CoroutineType") + } else { + definition_expression_ty(db, definition, returns.as_ref()) + } + }); Self { parameters: Parameters::from_parameters( @@ -49,45 +52,32 @@ impl<'db> Signature<'db> { return_ty, } } + + /// Return the parameters in this signature. + pub(crate) fn parameters(&self) -> &Parameters<'db> { + &self.parameters + } } -/// The parameters portion of a typed signature. -/// -/// The ordering of parameters is always as given in this struct: first positional-only parameters, -/// then positional-or-keyword, then optionally the variadic parameter, then keyword-only -/// parameters, and last, optionally the variadic keywords parameter. -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub(super) struct Parameters<'db> { - /// Parameters which may only be filled by positional arguments. - positional_only: Box<[ParameterWithDefault<'db>]>, - - /// Parameters which may be filled by positional or keyword arguments. - positional_or_keyword: Box<[ParameterWithDefault<'db>]>, - - /// The `*args` variadic parameter, if any. - variadic: Option>, - - /// Parameters which may only be filled by keyword arguments. - keyword_only: Box<[ParameterWithDefault<'db>]>, - - /// The `**kwargs` variadic keywords parameter, if any. - keywords: Option>, -} +// TODO: use SmallVec here once invariance bug is fixed +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Parameters<'db>(Vec>); impl<'db> Parameters<'db> { /// Return todo parameters: (*args: Todo, **kwargs: Todo) fn todo() -> Self { - Self { - variadic: Some(Parameter { + Self(vec![ + Parameter { name: Some(Name::new_static("args")), - annotated_ty: todo_type!(), - }), - keywords: Some(Parameter { + annotated_ty: Some(todo_type!("todo signature *args")), + kind: ParameterKind::Variadic, + }, + Parameter { name: Some(Name::new_static("kwargs")), - annotated_ty: todo_type!(), - }), - ..Default::default() - } + annotated_ty: Some(todo_type!("todo signature **kwargs")), + kind: ParameterKind::KeywordVariadic, + }, + ]) } fn from_parameters( @@ -103,94 +93,238 @@ impl<'db> Parameters<'db> { kwarg, range: _, } = parameters; - let positional_only = posonlyargs - .iter() - .map(|arg| ParameterWithDefault::from_node(db, definition, arg)) - .collect(); - let positional_or_keyword = args - .iter() - .map(|arg| ParameterWithDefault::from_node(db, definition, arg)) - .collect(); - let variadic = vararg - .as_ref() - .map(|arg| Parameter::from_node(db, definition, arg)); - let keyword_only = kwonlyargs - .iter() - .map(|arg| ParameterWithDefault::from_node(db, definition, arg)) - .collect(); - let keywords = kwarg - .as_ref() - .map(|arg| Parameter::from_node(db, definition, arg)); - Self { - positional_only, - positional_or_keyword, - variadic, - keyword_only, - keywords, - } - } -} - -/// A single parameter of a typed signature, with optional default value. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(super) struct ParameterWithDefault<'db> { - parameter: Parameter<'db>, - - /// Type of the default value, if any. - default_ty: Option>, -} - -impl<'db> ParameterWithDefault<'db> { - fn from_node( - db: &'db dyn Db, - definition: Definition<'db>, - parameter_with_default: &'db ast::ParameterWithDefault, - ) -> Self { - Self { - default_ty: parameter_with_default + let default_ty = |parameter_with_default: &ast::ParameterWithDefault| { + parameter_with_default .default .as_deref() - .map(|default| definition_expression_ty(db, definition, default)), - parameter: Parameter::from_node(db, definition, ¶meter_with_default.parameter), - } + .map(|default| definition_expression_ty(db, definition, default)) + }; + let positional_only = posonlyargs.iter().map(|arg| { + Parameter::from_node_and_kind( + db, + definition, + &arg.parameter, + ParameterKind::PositionalOnly { + default_ty: default_ty(arg), + }, + ) + }); + let positional_or_keyword = args.iter().map(|arg| { + Parameter::from_node_and_kind( + db, + definition, + &arg.parameter, + ParameterKind::PositionalOrKeyword { + default_ty: default_ty(arg), + }, + ) + }); + let variadic = vararg + .as_ref() + .map(|arg| Parameter::from_node_and_kind(db, definition, arg, ParameterKind::Variadic)); + let keyword_only = kwonlyargs.iter().map(|arg| { + Parameter::from_node_and_kind( + db, + definition, + &arg.parameter, + ParameterKind::KeywordOnly { + default_ty: default_ty(arg), + }, + ) + }); + let keywords = kwarg.as_ref().map(|arg| { + Parameter::from_node_and_kind(db, definition, arg, ParameterKind::KeywordVariadic) + }); + Self( + positional_only + .chain(positional_or_keyword) + .chain(variadic) + .chain(keyword_only) + .chain(keywords) + .collect(), + ) + } + + pub(crate) fn len(&self) -> usize { + self.0.len() + } + + pub(crate) fn iter(&self) -> std::slice::Iter> { + self.0.iter() + } + + /// Iterate initial positional parameters, not including variadic parameter, if any. + /// + /// For a valid signature, this will be all positional parameters. In an invalid signature, + /// there could be non-initial positional parameters; effectively, we just won't consider those + /// to be positional, which is fine. + pub(crate) fn positional(&self) -> impl Iterator> { + self.iter().take_while(|param| param.is_positional()) + } + + /// Return parameter at given index, or `None` if index is out-of-range. + pub(crate) fn get(&self, index: usize) -> Option<&Parameter<'db>> { + self.0.get(index) + } + + /// Return positional parameter at given index, or `None` if `index` is out of range. + /// + /// Does not return variadic parameter. + pub(crate) fn get_positional(&self, index: usize) -> Option<&Parameter<'db>> { + self.get(index) + .and_then(|parameter| parameter.is_positional().then_some(parameter)) + } + + /// Return the variadic parameter (`*args`), if any, and its index, or `None`. + pub(crate) fn variadic(&self) -> Option<(usize, &Parameter<'db>)> { + self.iter() + .enumerate() + .find(|(_, parameter)| parameter.is_variadic()) + } + + /// Return parameter (with index) for given name, or `None` if no such parameter. + /// + /// Does not return keywords (`**kwargs`) parameter. + /// + /// In an invalid signature, there could be multiple parameters with the same name; we will + /// just return the first that matches. + pub(crate) fn keyword_by_name(&self, name: &str) -> Option<(usize, &Parameter<'db>)> { + self.iter() + .enumerate() + .find(|(_, parameter)| parameter.callable_by_name(name)) + } + + /// Return the keywords parameter (`**kwargs`), if any, and its index, or `None`. + pub(crate) fn keyword_variadic(&self) -> Option<(usize, &Parameter<'db>)> { + self.iter() + .enumerate() + .rfind(|(_, parameter)| parameter.is_keyword_variadic()) + } +} + +impl<'db, 'a> IntoIterator for &'a Parameters<'db> { + type Item = &'a Parameter<'db>; + type IntoIter = std::slice::Iter<'a, Parameter<'db>>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl<'db> std::ops::Index for Parameters<'db> { + type Output = Parameter<'db>; + + fn index(&self, index: usize) -> &Self::Output { + &self.0[index] } } -/// A single parameter of a typed signature. #[derive(Clone, Debug, PartialEq, Eq)] -pub(super) struct Parameter<'db> { +pub(crate) struct Parameter<'db> { /// Parameter name. /// /// It is possible for signatures to be defined in ways that leave positional-only parameters /// nameless (e.g. via `Callable` annotations). name: Option, - /// Annotated type of the parameter (Unknown if no annotation.) - annotated_ty: Type<'db>, + /// Annotated type of the parameter. + annotated_ty: Option>, + + kind: ParameterKind<'db>, } impl<'db> Parameter<'db> { - fn from_node( + fn from_node_and_kind( db: &'db dyn Db, definition: Definition<'db>, parameter: &'db ast::Parameter, + kind: ParameterKind<'db>, ) -> Self { - Parameter { + Self { name: Some(parameter.name.id.clone()), annotated_ty: parameter .annotation .as_deref() - .map(|annotation| definition_expression_ty(db, definition, annotation)) - .unwrap_or(Type::Unknown), + .map(|annotation| definition_expression_ty(db, definition, annotation)), + kind, } } + + pub(crate) fn is_variadic(&self) -> bool { + matches!(self.kind, ParameterKind::Variadic) + } + + pub(crate) fn is_keyword_variadic(&self) -> bool { + matches!(self.kind, ParameterKind::KeywordVariadic) + } + + pub(crate) fn is_positional(&self) -> bool { + matches!( + self.kind, + ParameterKind::PositionalOnly { .. } | ParameterKind::PositionalOrKeyword { .. } + ) + } + + pub(crate) fn callable_by_name(&self, name: &str) -> bool { + match self.kind { + ParameterKind::PositionalOrKeyword { .. } | ParameterKind::KeywordOnly { .. } => self + .name + .as_ref() + .is_some_and(|param_name| param_name == name), + _ => false, + } + } + + /// Annotated type of the parameter, if annotated. + pub(crate) fn annotated_ty(&self) -> Option> { + self.annotated_ty + } + + /// Name of the parameter (if it has one). + pub(crate) fn name(&self) -> Option<&ast::name::Name> { + self.name.as_ref() + } + + /// Display name of the parameter, if it has one. + pub(crate) fn display_name(&self) -> Option { + self.name().map(|name| match self.kind { + ParameterKind::Variadic => ast::name::Name::new(format!("*{name}")), + ParameterKind::KeywordVariadic => ast::name::Name::new(format!("**{name}")), + _ => name.clone(), + }) + } + + /// Default-value type of the parameter, if any. + pub(crate) fn default_ty(&self) -> Option> { + match self.kind { + ParameterKind::PositionalOnly { default_ty } => default_ty, + ParameterKind::PositionalOrKeyword { default_ty } => default_ty, + ParameterKind::Variadic => None, + ParameterKind::KeywordOnly { default_ty } => default_ty, + ParameterKind::KeywordVariadic => None, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum ParameterKind<'db> { + /// Positional-only parameter, e.g. `def f(x, /): ...` + PositionalOnly { default_ty: Option> }, + /// Positional-or-keyword parameter, e.g. `def f(x): ...` + PositionalOrKeyword { default_ty: Option> }, + /// Variadic parameter, e.g. `def f(*args): ...` + Variadic, + /// Keyword-only parameter, e.g. `def f(*, x): ...` + KeywordOnly { default_ty: Option> }, + /// Variadic keywords parameter, e.g. `def f(**kwargs): ...` + KeywordVariadic, } #[cfg(test)] mod tests { use super::*; use crate::db::tests::{setup_db, TestDb}; - use crate::types::{global_symbol, FunctionType}; + use crate::types::{global_symbol, FunctionType, KnownClass}; use ruff_db::system::DbWithTestSystem; #[track_caller] @@ -202,39 +336,8 @@ mod tests { } #[track_caller] - fn assert_param_with_default<'db>( - db: &'db TestDb, - param_with_default: &ParameterWithDefault<'db>, - expected_name: &'static str, - expected_annotation_ty_display: &'static str, - expected_default_ty_display: Option<&'static str>, - ) { - assert_eq!( - param_with_default - .default_ty - .map(|ty| ty.display(db).to_string()), - expected_default_ty_display.map(ToString::to_string) - ); - assert_param( - db, - ¶m_with_default.parameter, - expected_name, - expected_annotation_ty_display, - ); - } - - #[track_caller] - fn assert_param<'db>( - db: &'db TestDb, - param: &Parameter<'db>, - expected_name: &'static str, - expected_annotation_ty_display: &'static str, - ) { - assert_eq!(param.name.as_ref().unwrap(), expected_name); - assert_eq!( - param.annotated_ty.display(db).to_string(), - expected_annotation_ty_display - ); + fn assert_params<'db>(signature: &Signature<'db>, expected: &[Parameter<'db>]) { + assert_eq!(signature.parameters.0.as_slice(), expected); } #[test] @@ -245,13 +348,8 @@ mod tests { let sig = func.internal_signature(&db); - assert_eq!(sig.return_ty.display(&db).to_string(), "Unknown"); - let params = sig.parameters; - assert!(params.positional_only.is_empty()); - assert!(params.positional_or_keyword.is_empty()); - assert!(params.variadic.is_none()); - assert!(params.keyword_only.is_empty()); - assert!(params.keywords.is_none()); + assert!(sig.return_ty.is_none()); + assert_params(&sig, &[]); } #[test] @@ -271,34 +369,74 @@ mod tests { let sig = func.internal_signature(&db); - assert_eq!(sig.return_ty.display(&db).to_string(), "bytes"); - let params = sig.parameters; - let [a, b, c, d] = ¶ms.positional_only[..] else { - panic!("expected four positional-only parameters"); - }; - let [e, f] = ¶ms.positional_or_keyword[..] else { - panic!("expected two positional-or-keyword parameters"); - }; - let Some(args) = params.variadic else { - panic!("expected a variadic parameter"); - }; - let [g, h] = ¶ms.keyword_only[..] else { - panic!("expected two keyword-only parameters"); - }; - let Some(kwargs) = params.keywords else { - panic!("expected a kwargs parameter"); - }; - - assert_param_with_default(&db, a, "a", "Unknown", None); - assert_param_with_default(&db, b, "b", "int", None); - assert_param_with_default(&db, c, "c", "Unknown", Some("Literal[1]")); - assert_param_with_default(&db, d, "d", "int", Some("Literal[2]")); - assert_param_with_default(&db, e, "e", "Unknown", Some("Literal[3]")); - assert_param_with_default(&db, f, "f", "Literal[4]", Some("Literal[4]")); - assert_param_with_default(&db, g, "g", "Unknown", Some("Literal[5]")); - assert_param_with_default(&db, h, "h", "Literal[6]", Some("Literal[6]")); - assert_param(&db, &args, "args", "object"); - assert_param(&db, &kwargs, "kwargs", "str"); + assert_eq!(sig.return_ty.unwrap().display(&db).to_string(), "bytes"); + assert_params( + &sig, + &[ + Parameter { + name: Some(Name::new_static("a")), + annotated_ty: None, + kind: ParameterKind::PositionalOnly { default_ty: None }, + }, + Parameter { + name: Some(Name::new_static("b")), + annotated_ty: Some(KnownClass::Int.to_instance(&db)), + kind: ParameterKind::PositionalOnly { default_ty: None }, + }, + Parameter { + name: Some(Name::new_static("c")), + annotated_ty: None, + kind: ParameterKind::PositionalOnly { + default_ty: Some(Type::IntLiteral(1)), + }, + }, + Parameter { + name: Some(Name::new_static("d")), + annotated_ty: Some(KnownClass::Int.to_instance(&db)), + kind: ParameterKind::PositionalOnly { + default_ty: Some(Type::IntLiteral(2)), + }, + }, + Parameter { + name: Some(Name::new_static("e")), + annotated_ty: None, + kind: ParameterKind::PositionalOrKeyword { + default_ty: Some(Type::IntLiteral(3)), + }, + }, + Parameter { + name: Some(Name::new_static("f")), + annotated_ty: Some(Type::IntLiteral(4)), + kind: ParameterKind::PositionalOrKeyword { + default_ty: Some(Type::IntLiteral(4)), + }, + }, + Parameter { + name: Some(Name::new_static("args")), + annotated_ty: Some(KnownClass::Object.to_instance(&db)), + kind: ParameterKind::Variadic, + }, + Parameter { + name: Some(Name::new_static("g")), + annotated_ty: None, + kind: ParameterKind::KeywordOnly { + default_ty: Some(Type::IntLiteral(5)), + }, + }, + Parameter { + name: Some(Name::new_static("h")), + annotated_ty: Some(Type::IntLiteral(6)), + kind: ParameterKind::KeywordOnly { + default_ty: Some(Type::IntLiteral(6)), + }, + }, + Parameter { + name: Some(Name::new_static("kwargs")), + annotated_ty: Some(KnownClass::Str.to_instance(&db)), + kind: ParameterKind::KeywordVariadic, + }, + ], + ); } #[test] @@ -322,11 +460,17 @@ mod tests { let sig = func.internal_signature(&db); - let [a] = &sig.parameters.positional_or_keyword[..] else { + let [Parameter { + name: Some(name), + annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }] = &sig.parameters.0[..] + else { panic!("expected one positional-or-keyword parameter"); }; + assert_eq!(name, "a"); // Parameter resolution not deferred; we should see A not B - assert_param_with_default(&db, a, "a", "A", None); + assert_eq!(annotated_ty.unwrap().display(&db).to_string(), "A"); } #[test] @@ -350,11 +494,17 @@ mod tests { let sig = func.internal_signature(&db); - let [a] = &sig.parameters.positional_or_keyword[..] else { + let [Parameter { + name: Some(name), + annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }] = &sig.parameters.0[..] + else { panic!("expected one positional-or-keyword parameter"); }; + assert_eq!(name, "a"); // Parameter resolution deferred; we should see B - assert_param_with_default(&db, a, "a", "B", None); + assert_eq!(annotated_ty.unwrap().display(&db).to_string(), "B"); } #[test] @@ -378,12 +528,23 @@ mod tests { let sig = func.internal_signature(&db); - let [a, b] = &sig.parameters.positional_or_keyword[..] else { + let [Parameter { + name: Some(a_name), + annotated_ty: a_annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }, Parameter { + name: Some(b_name), + annotated_ty: b_annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }] = &sig.parameters.0[..] + else { panic!("expected two positional-or-keyword parameters"); }; + assert_eq!(a_name, "a"); + assert_eq!(b_name, "b"); // TODO resolution should not be deferred; we should see A not B - assert_param_with_default(&db, a, "a", "B", None); - assert_param_with_default(&db, b, "b", "T", None); + assert_eq!(a_annotated_ty.unwrap().display(&db).to_string(), "B"); + assert_eq!(b_annotated_ty.unwrap().display(&db).to_string(), "T"); } #[test] @@ -407,12 +568,23 @@ mod tests { let sig = func.internal_signature(&db); - let [a, b] = &sig.parameters.positional_or_keyword[..] else { + let [Parameter { + name: Some(a_name), + annotated_ty: a_annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }, Parameter { + name: Some(b_name), + annotated_ty: b_annotated_ty, + kind: ParameterKind::PositionalOrKeyword { .. }, + }] = &sig.parameters.0[..] + else { panic!("expected two positional-or-keyword parameters"); }; + assert_eq!(a_name, "a"); + assert_eq!(b_name, "b"); // Parameter resolution deferred; we should see B - assert_param_with_default(&db, a, "a", "B", None); - assert_param_with_default(&db, b, "b", "T", None); + assert_eq!(a_annotated_ty.unwrap().display(&db).to_string(), "B"); + assert_eq!(b_annotated_ty.unwrap().display(&db).to_string(), "T"); } #[test] diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index fa6c966294..8abb040dae 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -33,6 +33,8 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:104:14 Name `char` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:115:14 Name `char` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:126:12 Name `char` used when possibly not defined", + // We don't handle intersections in `is_assignable_to` yet + "error[lint:invalid-argument-type] /src/tomllib/_parser.py:211:31 Object of type `Unknown & object | @Todo` cannot be assigned to parameter 1 (`obj`) of function `isinstance`; expected type `object`", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:348:20 Name `nest` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:353:5 Name `nest` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:453:24 Name `nest` used when possibly not defined", @@ -42,8 +44,11 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:573:12 Name `char` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:579:12 Name `char` used when possibly not defined", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:580:63 Name `char` used when possibly not defined", + "error[lint:invalid-argument-type] /src/tomllib/_parser.py:626:46 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_datetime`; expected type `Match`", "warning[lint:possibly-unresolved-reference] /src/tomllib/_parser.py:629:38 Name `datetime_obj` used when possibly not defined", - "warning[lint:unused-ignore-comment] /src/tomllib/_parser.py:682:31 Unused blanket `type: ignore` directive" + "error[lint:invalid-argument-type] /src/tomllib/_parser.py:632:58 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_localtime`; expected type `Match`", + "error[lint:invalid-argument-type] /src/tomllib/_parser.py:639:52 Object of type `@Todo & ~AlwaysFalsy` cannot be assigned to parameter 1 (`match`) of function `match_to_number`; expected type `Match`", + "warning[lint:unused-ignore-comment] /src/tomllib/_parser.py:682:31 Unused blanket `type: ignore` directive", ]; fn get_test_file(name: &str) -> TestFile {