Skip to content

Commit 53cfaae

Browse files
authored
[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](python/mypy#1705). Example: ```python def f() -> int: return 10 # error: [redundant-cast] "Value is already of type `int`" cast(int, f()) ```
1 parent 3ad123b commit 53cfaae

File tree

5 files changed

+68
-3
lines changed

5 files changed

+68
-3
lines changed

crates/red_knot_python_semantic/resources/mdtest/directives/cast.md

+13-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
The (inferred) type of the value and the given type do not need to have any correlation.
66

77
```py
8-
from typing import Literal, cast
8+
from typing import Literal, cast, Any
99

1010
reveal_type(True) # revealed: Literal[True]
1111
reveal_type(cast(str, True)) # revealed: str
@@ -25,4 +25,16 @@ reveal_type(cast(1, True)) # revealed: Unknown
2525
cast(str)
2626
# error: [too-many-positional-arguments] "Too many positional arguments to function `cast`: expected 2, got 3"
2727
cast(str, b"ar", "foo")
28+
29+
def function_returning_int() -> int:
30+
return 10
31+
32+
# error: [redundant-cast] "Value is already of type `int`"
33+
cast(int, function_returning_int())
34+
35+
def function_returning_any() -> Any:
36+
return "blah"
37+
38+
# error: [redundant-cast] "Value is already of type `Any`"
39+
cast(Any, function_returning_any())
2840
```

crates/red_knot_python_semantic/src/types.rs

+5
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,11 @@ impl<'db> Type<'db> {
320320
matches!(self, Type::Dynamic(DynamicType::Todo(_)))
321321
}
322322

323+
pub fn contains_todo(&self, db: &'db dyn Db) -> bool {
324+
self.is_todo()
325+
|| matches!(self, Type::Union(union) if union.elements(db).iter().any(Type::is_todo))
326+
}
327+
323328
pub const fn class_literal(class: Class<'db>) -> Self {
324329
Self::ClassLiteral(ClassLiteralType { class })
325330
}

crates/red_knot_python_semantic/src/types/diagnostic.rs

+22
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
6767
registry.register_lint(&ZERO_STEPSIZE_IN_SLICE);
6868
registry.register_lint(&STATIC_ASSERT_ERROR);
6969
registry.register_lint(&INVALID_ATTRIBUTE_ACCESS);
70+
registry.register_lint(&REDUNDANT_CAST);
7071

7172
// String annotations
7273
registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION);
@@ -878,6 +879,27 @@ declare_lint! {
878879
}
879880
}
880881

882+
declare_lint! {
883+
/// ## What it does
884+
/// Detects redundant `cast` calls where the value already has the target type.
885+
///
886+
/// ## Why is this bad?
887+
/// These casts have no effect and can be removed.
888+
///
889+
/// ## Example
890+
/// ```python
891+
/// def f() -> int:
892+
/// return 10
893+
///
894+
/// cast(int, f()) # Redundant
895+
/// ```
896+
pub(crate) static REDUNDANT_CAST = {
897+
summary: "detects redundant `cast` calls",
898+
status: LintStatus::preview("1.0.0"),
899+
default_level: Level::Warn,
900+
}
901+
}
902+
881903
#[derive(Debug, Eq, PartialEq, Clone)]
882904
pub struct TypeCheckDiagnostic {
883905
pub(crate) id: DiagnosticId,

crates/red_knot_python_semantic/src/types/infer.rs

+18-2
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ use super::diagnostic::{
9393
report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause,
9494
report_invalid_exception_raised, report_invalid_type_checking_constant,
9595
report_non_subscriptable, report_possibly_unresolved_reference, report_slice_step_size_zero,
96-
report_unresolved_reference, INVALID_METACLASS, STATIC_ASSERT_ERROR, SUBCLASS_OF_FINAL_CLASS,
97-
TYPE_ASSERTION_FAILURE,
96+
report_unresolved_reference, INVALID_METACLASS, REDUNDANT_CAST, STATIC_ASSERT_ERROR,
97+
SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE,
9898
};
9999
use super::slots::check_class_slots;
100100
use super::string_annotation::{
@@ -4017,6 +4017,22 @@ impl<'db> TypeInferenceBuilder<'db> {
40174017
}
40184018
}
40194019
}
4020+
KnownFunction::Cast => {
4021+
if let [Some(casted_ty), Some(source_ty)] = overload.parameter_types() {
4022+
if source_ty.is_gradual_equivalent_to(self.context.db(), *casted_ty)
4023+
&& !source_ty.contains_todo(self.context.db())
4024+
{
4025+
self.context.report_lint(
4026+
&REDUNDANT_CAST,
4027+
call_expression,
4028+
format_args!(
4029+
"Value is already of type `{}`",
4030+
casted_ty.display(self.context.db()),
4031+
),
4032+
);
4033+
}
4034+
}
4035+
}
40204036
_ => {}
40214037
}
40224038
}

knot.schema.json

+10
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,16 @@
610610
}
611611
]
612612
},
613+
"redundant-cast": {
614+
"title": "detects redundant `cast` calls",
615+
"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```",
616+
"default": "warn",
617+
"oneOf": [
618+
{
619+
"$ref": "#/definitions/Level"
620+
}
621+
]
622+
},
613623
"static-assert-error": {
614624
"title": "Failed static assertion",
615625
"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```",

0 commit comments

Comments
 (0)