From 53cfaaebc466c09aa6db31bae9568666cff4fc1e Mon Sep 17 00:00:00 2001 From: Trevor Manz Date: Mon, 31 Mar 2025 17:37:25 -0700 Subject: [PATCH] [red-knot] Add redundant-cast error (#17100) ## Summary Following up from earlier discussion on Discord, this PR adds logic to flag casts as redundant when the inferred type of the expression is the same as the target type. It should follow the semantics from [mypy](https://github.com/python/mypy/pull/1705). Example: ```python def f() -> int: return 10 # error: [redundant-cast] "Value is already of type `int`" cast(int, f()) ``` --- .../resources/mdtest/directives/cast.md | 14 +++++++++++- crates/red_knot_python_semantic/src/types.rs | 5 +++++ .../src/types/diagnostic.rs | 22 +++++++++++++++++++ .../src/types/infer.rs | 20 +++++++++++++++-- knot.schema.json | 10 +++++++++ 5 files changed, 68 insertions(+), 3 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 4203d83fe9..e991849f3e 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md +++ b/crates/red_knot_python_semantic/resources/mdtest/directives/cast.md @@ -5,7 +5,7 @@ The (inferred) type of the value and the given type do not need to have any correlation. ```py -from typing import Literal, cast +from typing import Literal, cast, Any reveal_type(True) # revealed: Literal[True] reveal_type(cast(str, True)) # revealed: str @@ -25,4 +25,16 @@ reveal_type(cast(1, True)) # revealed: Unknown cast(str) # error: [too-many-positional-arguments] "Too many positional arguments to function `cast`: expected 2, got 3" cast(str, b"ar", "foo") + +def function_returning_int() -> int: + return 10 + +# error: [redundant-cast] "Value is already of type `int`" +cast(int, function_returning_int()) + +def function_returning_any() -> Any: + return "blah" + +# error: [redundant-cast] "Value is already of type `Any`" +cast(Any, function_returning_any()) ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 48da634620..db8d74b6a4 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -320,6 +320,11 @@ impl<'db> Type<'db> { matches!(self, Type::Dynamic(DynamicType::Todo(_))) } + 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)) + } + pub const fn class_literal(class: Class<'db>) -> Self { Self::ClassLiteral(ClassLiteralType { class }) } diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 2026dbd8f6..1877bf7de4 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -67,6 +67,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&ZERO_STEPSIZE_IN_SLICE); registry.register_lint(&STATIC_ASSERT_ERROR); registry.register_lint(&INVALID_ATTRIBUTE_ACCESS); + registry.register_lint(&REDUNDANT_CAST); // String annotations registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION); @@ -878,6 +879,27 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Detects redundant `cast` calls where the value already has the target type. + /// + /// ## Why is this bad? + /// These casts have no effect and can be removed. + /// + /// ## Example + /// ```python + /// def f() -> int: + /// return 10 + /// + /// cast(int, f()) # Redundant + /// ``` + pub(crate) static REDUNDANT_CAST = { + summary: "detects redundant `cast` calls", + status: LintStatus::preview("1.0.0"), + default_level: Level::Warn, + } +} + #[derive(Debug, Eq, PartialEq, Clone)] pub struct TypeCheckDiagnostic { pub(crate) id: DiagnosticId, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index abc2accc0b..4c1d078ea0 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -93,8 +93,8 @@ use super::diagnostic::{ report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause, report_invalid_exception_raised, report_invalid_type_checking_constant, report_non_subscriptable, report_possibly_unresolved_reference, report_slice_step_size_zero, - report_unresolved_reference, INVALID_METACLASS, STATIC_ASSERT_ERROR, SUBCLASS_OF_FINAL_CLASS, - TYPE_ASSERTION_FAILURE, + report_unresolved_reference, INVALID_METACLASS, REDUNDANT_CAST, STATIC_ASSERT_ERROR, + SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE, }; use super::slots::check_class_slots; use super::string_annotation::{ @@ -4017,6 +4017,22 @@ impl<'db> TypeInferenceBuilder<'db> { } } } + KnownFunction::Cast => { + if let [Some(casted_ty), Some(source_ty)] = overload.parameter_types() { + if source_ty.is_gradual_equivalent_to(self.context.db(), *casted_ty) + && !source_ty.contains_todo(self.context.db()) + { + self.context.report_lint( + &REDUNDANT_CAST, + call_expression, + format_args!( + "Value is already of type `{}`", + casted_ty.display(self.context.db()), + ), + ); + } + } + } _ => {} } } diff --git a/knot.schema.json b/knot.schema.json index 66e11b3b26..7f96fb4fe1 100644 --- a/knot.schema.json +++ b/knot.schema.json @@ -610,6 +610,16 @@ } ] }, + "redundant-cast": { + "title": "detects redundant `cast` calls", + "description": "## What it does\nDetects redundant `cast` calls where the value already has the target type.\n\n## Why is this bad?\nThese casts have no effect and can be removed.\n\n## Example\n```python\ndef f() -> int:\n return 10\n\ncast(int, f()) # Redundant\n```", + "default": "warn", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "static-assert-error": { "title": "Failed static assertion", "description": "## What it does\nMakes sure that the argument of `static_assert` is statically known to be true.\n\n## Examples\n```python\nfrom knot_extensions import static_assert\n\nstatic_assert(1 + 1 == 3) # error: evaluates to `False`\n\nstatic_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known truthiness\n```",