[python] PyInvalidCastInspectionTest cleanup

- moved functions to `PyTypeUtil`
- improved naming and documentation
- improved hitbox for unnecessary
- added tests to suites


Merge-request: IJ-MR-174646
Merged-by: Morgan Bartholomew <morgan.bartholomew@jetbrains.com>

GitOrigin-RevId: 4cecc6eb9a22cd8cc4d4019e0ff6f22faebe8cae
This commit is contained in:
Morgan Bartholomew
2025-09-17 08:15:57 +00:00
committed by intellij-monorepo-bot
parent 95e54245ef
commit c2f7a647e3
7 changed files with 68 additions and 33 deletions

View File

@@ -1,7 +1,14 @@
<html>
<body>
<p>Reports calls to `typing.cast` where no possible value of the source type can be assignable to the target type. We can refer to this as "non overlapping" types</p>
<p>This usually indicates a mistake. If the conversion is intentional, first convert the expression to the common parent type to make the intent explicit.</p>
<p>Reports <code>typing.cast</code> calls where the source and target types are unrelated.</p>
<p>An error is reported when neither the source type is a subtype of the target, nor the target type is a subtype of the source.
Such casts often indicate a logical error, as an instance of one type cannot be assumed to be an instance of the other,
and <code>typing.cast</code> does not dynamically validate the type.</p>
<p>This check applies even to types that could theoretically have a common descendant.
For example, it will flag a cast between two sibling classes <code>Left</code> and <code>Right</code> that both inherit from <code>Top</code>,
because there is no direct inheritance relationship between them.</p>
<p><b>Example:</b></p>
<pre><code>
from typing import cast

View File

@@ -1338,8 +1338,8 @@ INSP.NAME.new.type.new.type.cannot.be.generic=NewType cannot be generic
packaging.could.not.parse.relation=Could not parse relation from: {0}
# PyInvalidCastInspection
INSP.NAME.invalid.cast=Type cast with impossible types
INSP.invalid.cast.message=Cast of type ''{0}'' to type ''{1}'' may be a mistake because no possible value of one is assignable with the other. If this was intentional, cast the expression to ''{2}'' first.
INSP.NAME.invalid.cast=Type cast between unrelated types
INSP.invalid.cast.message=Cast of type ''{0}'' to type ''{1}'' may be a mistake because they are not in the same inheritance hierarchy. If this was intentional, cast the expression to ''{2}'' first.
# Quick fixes for PyInvalidCastInspection
QFIX.add.intermediate.cast=Add cast({0}, ...)

View File

@@ -29,7 +29,7 @@ class PyInvalidCastInspection : PyInspection() {
val targetType = Ref.deref(targetTypeRef)
val actualType = myTypeEvalContext.getType(args[1])
if (PyTypeChecker.overlappingTypes(targetType, actualType, myTypeEvalContext)) return
if (PyTypeUtil.isOverlappingWith(targetType, actualType, myTypeEvalContext)) return
val fromName = PythonDocumentationProvider.getTypeName(actualType, myTypeEvalContext)
val toName = PythonDocumentationProvider.getVerboseTypeName(targetType, myTypeEvalContext)

View File

@@ -18,7 +18,7 @@ import com.jetbrains.python.documentation.PythonDocumentationProvider
import com.jetbrains.python.psi.PyCallExpression
import com.jetbrains.python.psi.PyFunction
import com.jetbrains.python.psi.types.PyType
import com.jetbrains.python.psi.types.PyTypeChecker
import com.jetbrains.python.psi.types.PyTypeUtil
class PyUnnecessaryCastInspection : PyInspection() {
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
@@ -37,7 +37,7 @@ class PyUnnecessaryCastInspection : PyInspection() {
val targetType = Ref.deref(targetTypeRef)
val actualType: PyType? = myTypeEvalContext.getType(args[1])
if (!PyTypeChecker.sameType(targetType, actualType, myTypeEvalContext)) return
if (!PyTypeUtil.isSameType(targetType, actualType, myTypeEvalContext)) return
val toName = PythonDocumentationProvider.getTypeName(targetType, myTypeEvalContext)
registerProblem(
callExpression,
@@ -50,6 +50,16 @@ class PyUnnecessaryCastInspection : PyInspection() {
TextRange(0, callExpression.arguments[0].nextSibling.endOffset - callExpression.startOffset),
RemoveUnnecessaryCastQuickFix(),
)
registerProblem(
callExpression,
PyPsiBundle.message(
"INSP.unnecessary.cast.message",
toName
),
ProblemHighlightType.INFORMATION,
null,
RemoveUnnecessaryCastQuickFix(),
)
}
}
}

View File

@@ -571,27 +571,6 @@ public final class PyTypeChecker {
return Optional.empty();
}
public static boolean sameType(@Nullable PyType type1, @Nullable PyType type2, @NotNull TypeEvalContext context) {
if ((type1 == null || type2 == null) && type1 != type2) return false;
return match(type1, type2, context)
&& match(type2, type1, context);
}
/**
* if some possible value of one type is assignable to the other type
*/
public static boolean overlappingTypes(@Nullable PyType type1, @Nullable PyType type2, @NotNull TypeEvalContext context) {
if (type1 instanceof PyUnionType unionType1) {
return ContainerUtil.exists(unionType1.getMembers(), t -> overlappingTypes(t, type2, context));
}
if (type2 instanceof PyUnionType unionType2) {
return ContainerUtil.exists(unionType2.getMembers(), t -> overlappingTypes(type1, t, context));
}
return match(type1, type2, context)
|| match(type2, type1, context);
}
private static boolean matchProtocols(@NotNull PyClassType expected, @NotNull PyClassType actual, @NotNull MatchContext matchContext) {
GenericSubstitutions substitutions = collectTypeSubstitutions(actual, matchContext.context);

View File

@@ -20,6 +20,7 @@ import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.UserDataHolder;
import com.intellij.psi.PsiElement;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.jetbrains.python.psi.PyClass;
import com.jetbrains.python.psi.PyPsiFacade;
import com.jetbrains.python.psi.impl.PyBuiltinCache;
@@ -32,6 +33,8 @@ import java.util.function.Function;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import static com.jetbrains.python.psi.types.PyTypeChecker.match;
/**
* Tools and wrappers around {@link PyType} inheritors
*
@@ -42,6 +45,42 @@ public final class PyTypeUtil {
private PyTypeUtil() {
}
/**
* Checks if two types are both assignable to each other, does not check if two types are equal
*/
public static boolean isSameType(@Nullable PyType type1, @Nullable PyType type2, @NotNull TypeEvalContext context) {
if ((type1 == null || type2 == null) && type1 != type2) return false;
return match(type1, type2, context)
&& match(type2, type1, context);
}
/**
* Checks if two types have a direct inheritance relationship, meaning one is a
* subtype or supertype of the other.
* <p>
* This method handles {@link PyUnionType} by distributing the check across its
* members. The types are considered overlapping if the condition holds for any
* pair of members.
*/
public static boolean isOverlappingWith(@Nullable PyType type1, @Nullable PyType type2, @NotNull TypeEvalContext context) {
// TODO: collapse this when PyUnionType and PyUnsafeUnionType have a common base
if (type1 instanceof PyUnionType unionType1) {
return ContainerUtil.exists(unionType1.getMembers(), t -> isOverlappingWith(t, type2, context));
}
if (type2 instanceof PyUnionType unionType2) {
return ContainerUtil.exists(unionType2.getMembers(), t -> isOverlappingWith(type1, t, context));
}
if (type1 instanceof PyUnsafeUnionType unionType1) {
return ContainerUtil.exists(unionType1.getMembers(), t -> isOverlappingWith(t, type2, context));
}
if (type2 instanceof PyUnsafeUnionType unionType2) {
return ContainerUtil.exists(unionType2.getMembers(), t -> isOverlappingWith(type1, t, context));
}
return match(type1, type2, context)
|| match(type2, type1, context);
}
/**
* Returns members of certain type from {@link PyClassLikeType}.
*/