From ef45185dbc9666aee8877a54e3fe3539bb9a7051 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 1 Oct 2024 17:22:13 -0400 Subject: [PATCH] Allow users to provide custom diagnostic messages when unwrapping calls (#13597) ## Summary You can now call `return_ty_result` to operate on a `Result` directly thereby using your own diagnostics, as in: ```rust return dunder_getitem_method .call(self.db, &[slice_ty]) .return_ty_result(self.db, value.as_ref().into(), self) .unwrap_or_else(|err| { self.add_diagnostic( (&**value).into(), "call-non-callable", format_args!( "Method `__getitem__` is not callable on object of type '{}'.", value_ty.display(self.db), ), ); err.return_ty() }); ``` --- crates/red_knot_python_semantic/src/types.rs | 171 +++++++++++++----- .../src/types/infer.rs | 32 +++- 2 files changed, 156 insertions(+), 47 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index f360184189451..a95856d3d336f 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -221,9 +221,9 @@ fn declarations_ty<'db>( first }; if conflicting.is_empty() { - DeclaredTypeResult::Ok(declared_ty) + Ok(declared_ty) } else { - DeclaredTypeResult::Err(( + Err(( declared_ty, [first].into_iter().chain(conflicting).collect(), )) @@ -900,37 +900,88 @@ impl<'db> CallOutcome<'db> { } } - /// Get the return type of the call, emitting diagnostics if needed. + /// Get the return type of the call, emitting default diagnostics if needed. fn unwrap_with_diagnostic<'a>( &self, db: &'db dyn Db, node: ast::AnyNodeRef, builder: &'a mut TypeInferenceBuilder<'db>, ) -> Type<'db> { - match self { - Self::Callable { return_ty } => *return_ty, - Self::RevealType { + match self.return_ty_result(db, node, builder) { + Ok(return_ty) => return_ty, + Err(NotCallableError::Type { + not_callable_ty, return_ty, - revealed_ty, - } => { + }) => { builder.add_diagnostic( node, - "revealed-type", - format_args!("Revealed type is '{}'.", revealed_ty.display(db)), + "call-non-callable", + format_args!( + "Object of type '{}' is not callable.", + not_callable_ty.display(db) + ), ); - *return_ty + return_ty } - Self::NotCallable { not_callable_ty } => { + Err(NotCallableError::UnionElement { + not_callable_ty, + called_ty, + return_ty, + }) => { builder.add_diagnostic( node, "call-non-callable", format_args!( - "Object of type '{}' is not callable.", - not_callable_ty.display(db) + "Object of type '{}' is not callable (due to union element '{}').", + called_ty.display(db), + not_callable_ty.display(db), ), ); - Type::Unknown + return_ty } + Err(NotCallableError::UnionElements { + not_callable_tys, + called_ty, + return_ty, + }) => { + builder.add_diagnostic( + node, + "call-non-callable", + format_args!( + "Object of type '{}' is not callable (due to union elements {}).", + called_ty.display(db), + not_callable_tys.display(db), + ), + ); + return_ty + } + } + } + + /// Get the return type of the call as a result. + fn return_ty_result<'a>( + &self, + db: &'db dyn Db, + node: ast::AnyNodeRef, + builder: &'a mut TypeInferenceBuilder<'db>, + ) -> Result, NotCallableError<'db>> { + match self { + Self::Callable { return_ty } => Ok(*return_ty), + Self::RevealType { + return_ty, + revealed_ty, + } => { + builder.add_diagnostic( + node, + "revealed-type", + format_args!("Revealed type is '{}'.", revealed_ty.display(db)), + ); + Ok(*return_ty) + } + Self::NotCallable { not_callable_ty } => Err(NotCallableError::Type { + not_callable_ty: *not_callable_ty, + return_ty: Type::Unknown, + }), Self::Union { outcomes, called_ty, @@ -959,41 +1010,75 @@ impl<'db> CallOutcome<'db> { }; union_builder = union_builder.add(return_ty); } + let return_ty = union_builder.build(); match not_callable[..] { - [] => {} - [elem] => builder.add_diagnostic( - node, - "call-non-callable", - format_args!( - "Object of type '{}' is not callable (due to union element '{}').", - called_ty.display(db), - elem.display(db), - ), - ), - _ if not_callable.len() == outcomes.len() => builder.add_diagnostic( - node, - "call-non-callable", - format_args!( - "Object of type '{}' is not callable.", - called_ty.display(db) - ), - ), - _ => builder.add_diagnostic( - node, - "call-non-callable", - format_args!( - "Object of type '{}' is not callable (due to union elements {}).", - called_ty.display(db), - not_callable.display(db), - ), - ), + [] => Ok(return_ty), + [elem] => Err(NotCallableError::UnionElement { + not_callable_ty: elem, + called_ty: *called_ty, + return_ty, + }), + _ if not_callable.len() == outcomes.len() => Err(NotCallableError::Type { + not_callable_ty: *called_ty, + return_ty, + }), + _ => Err(NotCallableError::UnionElements { + not_callable_tys: not_callable.into_boxed_slice(), + called_ty: *called_ty, + return_ty, + }), } - union_builder.build() } } } } +#[derive(Debug, Clone, PartialEq, Eq)] +enum NotCallableError<'db> { + /// The type is not callable. + Type { + not_callable_ty: Type<'db>, + return_ty: Type<'db>, + }, + /// A single union element is not callable. + UnionElement { + not_callable_ty: Type<'db>, + called_ty: Type<'db>, + return_ty: Type<'db>, + }, + /// Multiple (but not all) union elements are not callable. + UnionElements { + not_callable_tys: Box<[Type<'db>]>, + called_ty: Type<'db>, + return_ty: Type<'db>, + }, +} + +impl<'db> NotCallableError<'db> { + /// The return type that should be used when a call is not callable. + fn return_ty(&self) -> Type<'db> { + match self { + Self::Type { return_ty, .. } => *return_ty, + Self::UnionElement { return_ty, .. } => *return_ty, + Self::UnionElements { return_ty, .. } => *return_ty, + } + } + + /// The resolved type that was not callable. + /// + /// For unions, returns the union type itself, which may contain a mix of callable and + /// non-callable types. + fn called_ty(&self) -> Type<'db> { + match self { + Self::Type { + not_callable_ty, .. + } => *not_callable_ty, + Self::UnionElement { called_ty, .. } => *called_ty, + Self::UnionElements { called_ty, .. } => *called_ty, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum IterationOutcome<'db> { Iterable { element_ty: Type<'db> }, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index ed25acfafb8d6..065fd1c133e19 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2616,7 +2616,19 @@ impl<'db> TypeInferenceBuilder<'db> { if !dunder_getitem_method.is_unbound() { return dunder_getitem_method .call(self.db, &[slice_ty]) - .unwrap_with_diagnostic(self.db, value.as_ref().into(), self); + .return_ty_result(self.db, value.as_ref().into(), self) + .unwrap_or_else(|err| { + self.add_diagnostic( + (&**value).into(), + "call-non-callable", + format_args!( + "Method `__getitem__` of type '{}' is not callable on object of type '{}'.", + err.called_ty().display(self.db), + value_ty.display(self.db), + ), + ); + err.return_ty() + }); } // Otherwise, if the value is itself a class and defines `__class_getitem__`, @@ -2626,7 +2638,19 @@ impl<'db> TypeInferenceBuilder<'db> { if !dunder_class_getitem_method.is_unbound() { return dunder_class_getitem_method .call(self.db, &[slice_ty]) - .unwrap_with_diagnostic(self.db, value.as_ref().into(), self); + .return_ty_result(self.db, value.as_ref().into(), self) + .unwrap_or_else(|err| { + self.add_diagnostic( + (&**value).into(), + "call-non-callable", + format_args!( + "Method `__class_getitem__` of type '{}' is not callable on object of type '{}'.", + err.called_ty().display(self.db), + value_ty.display(self.db), + ), + ); + err.return_ty() + }); } self.non_subscriptable_diagnostic( @@ -6840,7 +6864,7 @@ mod tests { assert_file_diagnostics( &db, "/src/a.py", - &["Object of type 'None' is not callable."], + &["Method `__getitem__` of type 'None' is not callable on object of type 'NotSubscriptable'."], ); Ok(()) @@ -7015,7 +7039,7 @@ mod tests { assert_file_diagnostics( &db, "/src/a.py", - &["Object of type 'Literal[__class_getitem__] | Unbound' is not callable (due to union element 'Unbound')."], + &["Method `__class_getitem__` of type 'Literal[__class_getitem__] | Unbound' is not callable on object of type 'Literal[Identity, Identity]'."], ); Ok(())