PY-78964 Incorrect return type warning when using try...finally

Added a new CFG instruction to indicate implicit raise after finally block. Also added PyDataFlow as an future entry point for DFA.


Merge-request: IJ-MR-155798
Merged-by: Aleksandr Govenko <aleksandr.govenko@jetbrains.com>

GitOrigin-RevId: db63ba632c48235cc51b8e64869d21f5bcfc2d1c
This commit is contained in:
Aleksandr.Govenko
2025-03-07 01:31:33 +00:00
committed by intellij-monorepo-bot
parent 6c9e33b91e
commit 68ae14a71c
17 changed files with 294 additions and 107 deletions
@@ -19,6 +19,8 @@ import com.intellij.codeInsight.controlflow.ControlFlow;
import com.intellij.openapi.util.Key;
import com.jetbrains.python.codeInsight.dataflow.scope.Scope;
import com.jetbrains.python.codeInsight.dataflow.scope.impl.ScopeImpl;
import com.jetbrains.python.psi.PyUtil;
import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.NotNull;
import java.lang.ref.SoftReference;
@@ -62,4 +64,11 @@ public final class ControlFlowCache {
}
return scope;
}
public static @NotNull PyDataFlow getDataFlow(@NotNull ScopeOwner element, @NotNull TypeEvalContext context) {
// Cache will reset on psi modification, same as TypeEvalContext
return PyUtil.getParameterizedCachedValue(element, context, (ctx) -> {
return new PyDataFlow(getControlFlow(element), context);
});
}
}
@@ -641,6 +641,7 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor {
myBuilder.flowAbrupted();
finallyFailInstruction = myBuilder.startNode(finallyPart);
finallyPart.accept(this);
myBuilder.addNode(new PyFinallyFailExitInstruction(myBuilder, finallyFailInstruction));
myBuilder.addPendingEdge(null, myBuilder.prevInstruction);
myBuilder.flowAbrupted();
}
@@ -0,0 +1,61 @@
package com.jetbrains.python.codeInsight.controlflow;
import com.intellij.codeInsight.controlflow.ControlFlow;
import com.intellij.codeInsight.controlflow.ControlFlowUtil;
import com.intellij.codeInsight.controlflow.Instruction;
import com.intellij.psi.PsiElement;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import java.util.*;
@ApiStatus.Internal
public class PyDataFlow {
private final TypeEvalContext myTypeEvalContext;
private final Instruction[] myInstructions;
private final boolean[] myReachability;
public PyDataFlow(@NotNull ControlFlow controlFlow, @NotNull TypeEvalContext context) {
myTypeEvalContext = context;
myInstructions = controlFlow.getInstructions();
myReachability = new boolean[myInstructions.length];
buildReachability();
}
private void buildReachability() {
Queue<Instruction> toBeProcessed = new ArrayDeque<>();
toBeProcessed.add(myInstructions[0]);
while (!toBeProcessed.isEmpty()) {
Instruction instruction = toBeProcessed.poll();
myReachability[instruction.num()] = true;
for (var successor : getReachableSuccessors(instruction)) {
if (!myReachability[successor.num()]) {
toBeProcessed.add(successor);
}
}
}
}
private @NotNull Collection<Instruction> getReachableSuccessors(@NotNull Instruction instruction) {
if (instruction instanceof CallInstruction ci && ci.isNoReturnCall(myTypeEvalContext)) return List.of();
if (instruction instanceof PyWithContextExitInstruction wi && !wi.isSuppressingExceptions(myTypeEvalContext)) return List.of();
return instruction.allSucc();
}
public boolean isUnreachable(@NotNull Instruction instruction) {
return !myReachability[instruction.num()];
}
public static boolean isUnreachable(@NotNull PsiElement element, @NotNull TypeEvalContext context) {
final var scope = ScopeUtil.getScopeOwner(element);
if (scope != null) {
final var flow = ControlFlowCache.getControlFlow(scope).getInstructions();
int idx = ControlFlowUtil.findInstructionNumberByElement(flow, element);
if (idx < 0) return false;
return ControlFlowCache.getDataFlow(scope, context).isUnreachable(flow[idx]);
}
return false;
}
}
@@ -0,0 +1,13 @@
package com.jetbrains.python.codeInsight.controlflow
import com.intellij.codeInsight.controlflow.ControlFlowBuilder
import com.intellij.codeInsight.controlflow.Instruction
import com.intellij.codeInsight.controlflow.impl.InstructionImpl
/**
* It is an implicit raise at the end of a finally block when we got here due to exception propagation
* or return statement in try-except-else parts.
*/
class PyFinallyFailExitInstruction(builder: ControlFlowBuilder, val begin: Instruction) : InstructionImpl(builder, begin.element) {
override fun getElementPresentation(): String = "finally fail exit"
}
@@ -7,6 +7,7 @@ import com.intellij.codeInsight.dataflow.map.DfaMapInstance;
import com.intellij.psi.PsiElement;
import com.intellij.psi.util.PsiTreeUtil;
import com.jetbrains.python.codeInsight.controlflow.CallInstruction;
import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
import com.jetbrains.python.codeInsight.controlflow.ReadWriteInstruction;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeVariable;
@@ -36,6 +37,12 @@ public class PyReachingDefsDfaInstance implements DfaMapInstance<ScopeVariable>
if (element == null || ((PyFile) element.getContainingFile()).getLanguageLevel().isPython2()){
return processReducedMap(map, instruction, element);
}
var scope = ScopeUtil.getScopeOwner(element);
if (scope != null) {
if (ControlFlowCache.getDataFlow(scope, myContext).isUnreachable(instruction)) {
return UNREACHABLE_MARKER;
}
}
if (instruction instanceof CallInstruction callInstruction) {
if (callInstruction.isNoReturnCall(myContext)) {
return UNREACHABLE_MARKER;
@@ -10,7 +10,6 @@ import com.jetbrains.python.codeInsight.controlflow.CallInstruction;
import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
import com.jetbrains.python.codeInsight.controlflow.PyWithContextExitInstruction;
import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
import com.jetbrains.python.psi.PyStatementListContainer;
import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.ApiStatus;
@@ -21,14 +20,6 @@ import java.util.ArrayList;
import java.util.List;
public final class PyInspectionsUtil {
@ApiStatus.Internal
public static boolean hasAnyInterruptedControlFlowPaths(@NotNull PsiElement element, @NotNull TypeEvalContext context) {
final ScopeOwner owner = ScopeUtil.getScopeOwner(element);
if (owner != null) {
return !collectUnreachable(owner, element, context).isEmpty();
}
return false;
}
/**
* Collects a list of unreachable elements, iterating through CFG backwards
@@ -15,6 +15,7 @@ import com.intellij.psi.PsiPolyVariantReference;
import com.intellij.psi.util.PsiTreeUtil;
import com.jetbrains.python.PyPsiBundle;
import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
import com.jetbrains.python.codeInsight.controlflow.PyDataFlow;
import com.jetbrains.python.codeInsight.controlflow.ReadWriteInstruction;
import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
import com.jetbrains.python.codeInsight.dataflow.scope.Scope;
@@ -66,6 +67,9 @@ public final class PyUnboundLocalVariableInspection extends PyInspection {
if (PsiTreeUtil.getParentOfType(node, PyImportStatementBase.class) != null) {
return;
}
if (PyDataFlow.isUnreachable(node, myTypeEvalContext)) {
return;
}
final String name = node.getReferencedName();
if (name == null) {
return;
@@ -122,9 +126,6 @@ public final class PyUnboundLocalVariableInspection extends PyInspection {
if (resolvedUnderWithStatement(node, resolved) || resolvedUnderAssignmentExpressionAndCondition(node, resolved)) {
return;
}
if (PyInspectionsUtil.hasAnyInterruptedControlFlowPaths(node, myTypeEvalContext)) {
return;
}
if (owner instanceof PyFile) {
if (isBuiltin) {
return;
@@ -28,6 +28,7 @@ import com.jetbrains.python.PythonRuntimeService;
import com.jetbrains.python.codeInsight.PyCustomMember;
import com.jetbrains.python.codeInsight.PySubstitutionChunkReference;
import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache;
import com.jetbrains.python.codeInsight.controlflow.PyDataFlow;
import com.jetbrains.python.codeInsight.controlflow.ReadWriteInstruction;
import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil;
@@ -38,7 +39,6 @@ import com.jetbrains.python.documentation.docstrings.DocStringTypeReference;
import com.jetbrains.python.inspections.PyInspection;
import com.jetbrains.python.inspections.PyInspectionExtension;
import com.jetbrains.python.inspections.PyInspectionVisitor;
import com.jetbrains.python.inspections.PyInspectionsUtil;
import com.jetbrains.python.inspections.quickfix.*;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.impl.*;
@@ -298,7 +298,7 @@ public abstract class PyUnresolvedReferencesVisitor extends PyInspectionVisitor
return;
}
if (!expr.isQualified()) {
if (PyInspectionsUtil.hasAnyInterruptedControlFlowPaths(expr, myTypeEvalContext)) {
if (PyDataFlow.isUnreachable(expr, myTypeEvalContext)) {
return;
}
ContainerUtil.addIfNotNull(fixes, getTrueFalseQuickFix(refText));
@@ -17,6 +17,7 @@ import com.intellij.psi.stubs.IStubElementType;
import com.intellij.psi.stubs.StubElement;
import com.intellij.psi.util.*;
import com.intellij.ui.IconManager;
import com.intellij.util.ArrayUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.PlatformIcons;
import com.intellij.util.containers.JBIterable;
@@ -379,26 +380,52 @@ public class PyFunctionImpl extends PyBaseElementImpl<PyFunctionStub> implements
@Override
public @NotNull List<PyStatement> getReturnPoints(@NotNull TypeEvalContext context) {
final Instruction[] flow = ControlFlowCache.getControlFlow(this).getInstructions();
final List<PyStatement> returnPoints = new ArrayList<>();
final PyDataFlow dataFlow = ControlFlowCache.getDataFlow(this, context);
ControlFlowUtil.iteratePrev(flow.length-1, flow, instruction -> {
if (instruction instanceof CallInstruction ci && ci.isNoReturnCall(context)) {
class ReturnPointCollector {
final List<PyStatement> returnPoints = new ArrayList<>();
boolean collectImplicitReturn = true;
ControlFlowUtil.Operation checkInstruction(@NotNull Instruction instruction) {
if (dataFlow.isUnreachable(instruction)) {
return ControlFlowUtil.Operation.CONTINUE;
}
if (instruction instanceof PyFinallyFailExitInstruction exitInstruction) {
// Most nodes in try-part are connected to finally-fail-part,
// but we will only be interested in explicit return statements.
boolean oldCollectImplicitReturn = collectImplicitReturn;
collectImplicitReturn = false;
walkCFG(ArrayUtil.indexOf(flow, exitInstruction.getBegin()));
collectImplicitReturn = oldCollectImplicitReturn;
return ControlFlowUtil.Operation.CONTINUE;
}
if (instruction instanceof CallInstruction ci && ci.isNoReturnCall(context)) {
return ControlFlowUtil.Operation.CONTINUE;
}
if (instruction instanceof PyRaiseInstruction) {
return ControlFlowUtil.Operation.CONTINUE;
}
if (instruction instanceof PyWithContextExitInstruction withExit && !withExit.isSuppressingExceptions(context)) {
return ControlFlowUtil.Operation.CONTINUE;
}
final PsiElement element = instruction.getElement();
if (!(element instanceof PyStatement statement)) {
return ControlFlowUtil.Operation.NEXT;
}
if (collectImplicitReturn || statement instanceof PyReturnStatement) {
returnPoints.add(statement);
}
return ControlFlowUtil.Operation.CONTINUE;
}
if (instruction instanceof PyRaiseInstruction) {
return ControlFlowUtil.Operation.CONTINUE;
void walkCFG(int startInstruction) {
ControlFlowUtil.iteratePrev(startInstruction, flow, this::checkInstruction);
}
if (instruction instanceof PyWithContextExitInstruction withExit && !withExit.isSuppressingExceptions(context)) {
return ControlFlowUtil.Operation.CONTINUE;
}
final PsiElement element = instruction.getElement();
if (!(element instanceof PyStatement statement)) {
return ControlFlowUtil.Operation.NEXT;
}
returnPoints.add(statement);
return ControlFlowUtil.Operation.CONTINUE;
});
return returnPoints;
}
ReturnPointCollector collector = new ReturnPointCollector();
collector.walkCFG(flow.length - 1);
return collector.returnPoints;
}
@Override
@@ -6,7 +6,7 @@
5(6) element: PyExceptPart
6(7) READ ACCESS: ImportError
7(8) raise: PyRaiseStatement
8(25) READ ACCESS: Error
8(26) READ ACCESS: Error
9(10) element: PyAssignmentStatement
10(11) WRITE ACCESS: p
11(12) element: PyTryExceptStatement
@@ -14,13 +14,14 @@
13(14,17) element: PyReturnStatement
14(17) READ ACCESS: foo
15(16,17) element: PyAssignmentStatement
16(17,20) WRITE ACCESS: x
16(17,21) WRITE ACCESS: x
17(18) element: PyFinallyPart
18(19) element: PyPrintStatement
19(25) READ ACCESS: p
20(21) element: PyFinallyPart
21(22) element: PyPrintStatement
22(23) READ ACCESS: p
23(24) element: PyAssignmentStatement
24(25) WRITE ACCESS: y
25() element: null
19(20) READ ACCESS: p
20(26) finally fail exit
21(22) element: PyFinallyPart
22(23) element: PyPrintStatement
23(24) READ ACCESS: p
24(25) element: PyAssignmentStatement
25(26) WRITE ACCESS: y
26() element: null
@@ -12,12 +12,13 @@
11(12,18) element: PyElsePart
12(13,18) element: PyPrintStatement
13(14,18) READ ACCESS: v
14(18,20) element: PyCallExpression: 'element value {0}'.format
14(18,21) element: PyCallExpression: 'element value {0}'.format
15(16,18) element: PyExceptPart
16(17,18) READ ACCESS: KeyError
17(18,20) element: PyPrintStatement
17(18,21) element: PyPrintStatement
18(19) element: PyFinallyPart
19(22) element: PyPrintStatement
20(21) element: PyFinallyPart
21(22) element: PyPrintStatement
22() element: null
19(20) element: PyPrintStatement
20(23) finally fail exit
21(22) element: PyFinallyPart
22(23) element: PyPrintStatement
23() element: null
@@ -7,30 +7,31 @@
6(7,10) element: PyAssignmentStatement
7(8,10) READ ACCESS: open
8(9,10) element: PyCallExpression: open
9(10,21) WRITE ACCESS: status
9(10,22) WRITE ACCESS: status
10(11) element: PyFinallyPart
11(12) element: PyIfStatement
12(13) READ ACCESS: status
13(14,16) READ ACCESS: None
14(15) element: null. Condition: status is not None:false
15() ASSERTTYPE ACCESS: status
15(21) ASSERTTYPE ACCESS: status
16(17) element: null. Condition: status is not None:true
17(18) ASSERTTYPE ACCESS: status
18(19) element: PyStatementList
19(20) element: PyPrintStatement
20() READ ACCESS: status
21(22) element: PyFinallyPart
22(23) element: PyIfStatement
23(24) READ ACCESS: status
24(25,27) READ ACCESS: None
25(26) element: null. Condition: status is not None:false
26(32) ASSERTTYPE ACCESS: status
27(28) element: null. Condition: status is not None:true
28(29) ASSERTTYPE ACCESS: status
29(30) element: PyStatementList
30(31) element: PyPrintStatement
31(32) READ ACCESS: status
32(33) element: PyExpressionStatement
33(34) READ ACCESS: status
34(35) element: PyCallExpression: status.close
35() element: null
20(21) READ ACCESS: status
21(36) finally fail exit
22(23) element: PyFinallyPart
23(24) element: PyIfStatement
24(25) READ ACCESS: status
25(26,28) READ ACCESS: None
26(27) element: null. Condition: status is not None:false
27(33) ASSERTTYPE ACCESS: status
28(29) element: null. Condition: status is not None:true
29(30) ASSERTTYPE ACCESS: status
30(31) element: PyStatementList
31(32) element: PyPrintStatement
32(33) READ ACCESS: status
33(34) element: PyExpressionStatement
34(35) READ ACCESS: status
35(36) element: PyCallExpression: status.close
36() element: null
@@ -5,4 +5,5 @@
4(5) READ ACCESS: KeyboardInterrupt
5(6) element: PyFinallyPart
6(7) element: PyPrintStatement
7() element: null
7(8) finally fail exit
8() element: null
@@ -2,17 +2,17 @@
1(2) element: PyAssignmentStatement
2(3) WRITE ACCESS: a
3(4) element: PyTryExceptStatement
4(5,69) element: PyTryPart
5(6,69) element: PyAssignmentStatement
6(7,69) WRITE ACCESS: b
7(8,69) element: PyForStatement
8(9,69) element: PyTargetExpression: x
9(10,69) WRITE ACCESS: x
10(11,69) element: PyTryExceptStatement
11(12,59) element: PyTryPart
12(13,59) element: PyAssignmentStatement
13(14,59) WRITE ACCESS: c
14(15,59) element: PyTryExceptStatement
4(5,71) element: PyTryPart
5(6,71) element: PyAssignmentStatement
6(7,71) WRITE ACCESS: b
7(8,71) element: PyForStatement
8(9,71) element: PyTargetExpression: x
9(10,71) WRITE ACCESS: x
10(11,71) element: PyTryExceptStatement
11(12,60) element: PyTryPart
12(13,60) element: PyAssignmentStatement
13(14,60) WRITE ACCESS: c
14(15,60) element: PyTryExceptStatement
15(16,51) element: PyTryPart
16(17,51) element: PyAssignmentStatement
17(18,51) WRITE ACCESS: d
@@ -23,14 +23,14 @@
22(23) element: null. Condition: x == 0:true
23(51,24) ASSERTTYPE ACCESS: x
24(25) element: PyStatementList
25(54) element: PyBreakStatement
25(55) element: PyBreakStatement
26(27,29,51) READ ACCESS: x
27(28) element: null. Condition: x == 1:false
28(51,33) ASSERTTYPE ACCESS: x
29(30) element: null. Condition: x == 1:true
30(51,31) ASSERTTYPE ACCESS: x
31(32) element: PyStatementList
32(7,54) element: PyContinueStatement
32(7,55) element: PyContinueStatement
33(34,36,51) READ ACCESS: x
34(35) element: null. Condition: x == 2:false
35(51,42) ASSERTTYPE ACCESS: x
@@ -48,31 +48,34 @@
47(48) element: PyStatementList
48(51) element: PyReturnStatement
49(50,51) element: PyAssignmentStatement
50(51,54) WRITE ACCESS: e
51(52,59) element: PyFinallyPart
52(53,59) element: PyAssignmentStatement
53(59) WRITE ACCESS: f
54(55,59) element: PyFinallyPart
55(56,59) element: PyAssignmentStatement
56(57,59,62) WRITE ACCESS: f
57(58,59) element: PyAssignmentStatement
58(59,62) WRITE ACCESS: g
59(60,69) element: PyFinallyPart
60(61,69) element: PyAssignmentStatement
61(69) WRITE ACCESS: h
62(63,69) element: PyFinallyPart
63(64,69) element: PyAssignmentStatement
64(65,67,69) WRITE ACCESS: h
65(66,69) element: PyAssignmentStatement
66(8,67,69) WRITE ACCESS: i
67(68,69) element: PyAssignmentStatement
68(69,72) WRITE ACCESS: j
69(70) element: PyFinallyPart
70(71) element: PyAssignmentStatement
71(77) WRITE ACCESS: k
72(73) element: PyFinallyPart
73(74) element: PyAssignmentStatement
74(75) WRITE ACCESS: k
75(76) element: PyAssignmentStatement
76(77) WRITE ACCESS: l
77() element: null
50(51,55) WRITE ACCESS: e
51(52,60) element: PyFinallyPart
52(53,60) element: PyAssignmentStatement
53(54,60) WRITE ACCESS: f
54(60) finally fail exit
55(56,60) element: PyFinallyPart
56(57,60) element: PyAssignmentStatement
57(58,60,64) WRITE ACCESS: f
58(59,60) element: PyAssignmentStatement
59(60,64) WRITE ACCESS: g
60(61,71) element: PyFinallyPart
61(62,71) element: PyAssignmentStatement
62(63,71) WRITE ACCESS: h
63(71) finally fail exit
64(65,71) element: PyFinallyPart
65(66,71) element: PyAssignmentStatement
66(67,69,71) WRITE ACCESS: h
67(68,71) element: PyAssignmentStatement
68(8,69,71) WRITE ACCESS: i
69(70,71) element: PyAssignmentStatement
70(71,75) WRITE ACCESS: j
71(72) element: PyFinallyPart
72(73) element: PyAssignmentStatement
73(74) WRITE ACCESS: k
74(80) finally fail exit
75(76) element: PyFinallyPart
76(77) element: PyAssignmentStatement
77(78) WRITE ACCESS: k
78(79) element: PyAssignmentStatement
79(80) WRITE ACCESS: l
80() element: null
@@ -28,6 +28,22 @@ public class Py3TypeTest extends PyTestCase {
""");
}
// PY-78964
public void testFunctionReturnTypeTryFinally() {
doTest("str",
"""
def test():
try:
return 42
finally:
return "str"
return True
expr = test()
""");
}
// PY-20710
public void testLambdaGenerator() {
doTest("Generator[int, Any, Any]", """
@@ -3438,6 +3454,44 @@ public class Py3TypeTest extends PyTestCase {
""");
}
public void testShadowingReturnInsideFinally() {
doTest("str", """
def f():
try:
return 42
finally:
return "foo"
expr = f()
""");
}
public void testNonShadowingReturnInsideFinally() {
doTest("int | str", """
def f(p):
try:
return 42
finally:
if p:
return "foo"
expr = f()
""");
}
public void testReturnInsideExceptElse() {
doTest("str | bool", """
def f(p):
try:
e1()
except Exception:
return "foo"
else:
return True
finally:
pass
expr = f()
""");
}
private void doTest(final String expectedType, final String text) {
myFixture.configureByText(PythonFileType.INSTANCE, text);
final PyExpression expr = myFixture.findElementByText("expr", PyExpression.class);
@@ -99,8 +99,8 @@ public class PyTypeTest extends PyTestCase {
public void testUnionOfTuples() {
doTest("Union[Tuple[int, str], Tuple[str, int]]",
"""
def x():
if True:
def x(b):
if b:
return (1, 'a')
else:
return ('a', 1)
@@ -976,6 +976,22 @@ public class PyTypeCheckerInspectionTest extends PyInspectionTestCase {
);
}
// PY-78964
public void testFunctionWithTryFinally() {
runWithLanguageLevel(
LanguageLevel.getLatest(),
() -> doTestByText("""
def test() -> bool:
try:
pass
finally:
pass
return True
""")
);
}
// PY-33500
public void testImplicitGenericDunderCallCallOnTypedElement() {
runWithLanguageLevel(