[red-knot] Add redundant-cast error

[manzt]

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.

Example:

def f() -> int:
    return 10

# error: [redundant-cast] "Value is already of type `int`"
cast(int, f())

However, I don't think the comparison is quite right yet...there's probably some type comparison behavior I'm misunderstanding. Hopefully it's close enough that the fix is straightforward.

[github-actions]

mypy_primer results

No ecosystem changes detected ✅

[carljm]

This is great, really impressive first PR, thank you! I do have some suggestions, though ;)

[carljm]

Looks great!

[AlexWaygood]

The mypy_primer hits don't look ideal: we may want to special-case Todo types so that a "cast to a Todo type" is never reported as redundant

[manzt]

Sorry... I ran cargo and black on the docs to fix the errors.

[carljm]

The mypy_primer hits don't look ideal: we may want to special-case Todo types so that a "cast to a Todo type" is never reported as redundant

Oh yeah, good call. We probably should do that before landing this, to avoid adding new false positives. Looks like we need to special case "Todo type" and "union with todo type" to avoid adding new false positives.

[manzt]

Looks like we need to special case "Todo type" and "union with todo type" to avoid adding new false positives.

Would this be some check is_todo_type(source_ty) or some equivalent within infer.rs before deciding to report an error?

[carljm]

We usually just add methods on Type; looks like we already have Type::is_todo but might want to add Type::contains_todo to cover the union case. Then that can be checked in the new clause you added in infer_call_expression.

[manzt]

but might want to add Type::contains_todo to cover the union case.

Ok, I think I was able to implement this. I wasn't sure how to specify an explicity Todo type in cast.md to test the case.

[carljm]

I think it's OK not to add a test for this -- it's not important behavior we want to preserve, it's just a temporary thing to reduce false positives from the ecosystem check. So the ecosystem check itself will avoid regression (and whenever it doesn't see any false positives anymore, then we also don't want to keep this code around anymore.)

[carljm]

Looks great, thanks! And ecosystem check is now reporting no changes. Setting to auto-merge when CI finishes.