From c74ba00219fb6d06bc3da08d98102da7ab7bce96 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 1 Apr 2025 19:03:42 +0100 Subject: [PATCH] [red-knot] Fix more `redundant-cast` false positives (#17119) ## Summary There are quite a few places we infer `Todo` types currently, and some of them are nested somewhat deeply in type expressions. These can cause spurious issues for the new `redundant-cast` diagnostics. We fixed all the false positives we saw in the mypy_primer report before merging https://github.com/astral-sh/ruff/pull/17100, but I think there are still lots of places where we'd emit false positives due to this check -- we currently don't run on that many projects at all in our mypy_primer check: https://github.com/astral-sh/ruff/blob/d0c8eaa0923352ccc4ec30c9fac1f573f20998b3/.github/workflows/mypy_primer.yaml#L71 This PR fixes some more false positives from this diagnostic by making the `Type::contains_todo()` method more expansive. ## Test Plan I added a regression test which causes us to emit a spurious diagnostic on `main`, but does not with this PR. --- .../resources/mdtest/directives/cast.md | 11 ++++ crates/red_knot_python_semantic/src/types.rs | 56 ++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md b/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md index e991849f3e..81712cff8b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md +++ b/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md @@ -38,3 +38,14 @@ def function_returning_any() -> Any: # error: [redundant-cast] "Value is already of type `Any`" cast(Any, function_returning_any()) ``` + +Complex type expressions (which may be unsupported) do not lead to spurious `[redundant-cast]` +diagnostics. + +```py +from typing import Callable + +def f(x: Callable[[dict[str, int]], None], y: tuple[dict[str, int]]): + a = cast(Callable[[list[bytes]], None], x) + b = cast(tuple[list[bytes]], y) +``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 923884d268..774f6fd0ce 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -321,8 +321,60 @@ impl<'db> Type<'db> { } pub fn contains_todo(&self, db: &'db dyn Db) -> bool { - self.is_todo() - || matches!(self, Type::Union(union) if union.elements(db).iter().any(Type::is_todo)) + match self { + Self::Dynamic(DynamicType::Todo(_) | DynamicType::TodoProtocol) => true, + + Self::AlwaysFalsy + | Self::AlwaysTruthy + | Self::Never + | Self::BooleanLiteral(_) + | Self::BytesLiteral(_) + | Self::FunctionLiteral(_) + | Self::Instance(_) + | Self::ModuleLiteral(_) + | Self::ClassLiteral(_) + | Self::KnownInstance(_) + | Self::StringLiteral(_) + | Self::IntLiteral(_) + | Self::LiteralString + | Self::SliceLiteral(_) + | Self::Dynamic(DynamicType::Unknown | DynamicType::Any) + | Self::Callable( + CallableType::BoundMethod(_) + | CallableType::WrapperDescriptorDunderGet + | CallableType::MethodWrapperDunderGet(_), + ) => false, + + Self::Callable(CallableType::General(callable)) => { + let signature = callable.signature(db); + signature.parameters().iter().any(|param| { + param + .annotated_type() + .is_some_and(|ty| ty.contains_todo(db)) + }) || signature.return_ty.is_some_and(|ty| ty.contains_todo(db)) + } + + Self::SubclassOf(subclass_of) => match subclass_of.subclass_of() { + ClassBase::Dynamic(DynamicType::Todo(_) | DynamicType::TodoProtocol) => true, + ClassBase::Dynamic(DynamicType::Unknown | DynamicType::Any) => false, + ClassBase::Class(_) => false, + }, + + Self::Tuple(tuple) => tuple.elements(db).iter().any(|ty| ty.contains_todo(db)), + + Self::Union(union) => union.elements(db).iter().any(|ty| ty.contains_todo(db)), + + Self::Intersection(intersection) => { + intersection + .positive(db) + .iter() + .any(|ty| ty.contains_todo(db)) + || intersection + .negative(db) + .iter() + .any(|ty| ty.contains_todo(db)) + } + } } pub const fn class_literal(class: Class<'db>) -> Self {