From bf22c95cc3a860106acfbd5fe6f21ac85803d94b Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Sat, 15 Jul 2017 16:37:27 +0300 Subject: [PATCH] PY-21175 Add implicit negative type assertion after "if" in CFG unconditionally The check that we should add such synthetic node only if there were no pending edges inside the body of if statement (e.g. if it contained only break/continue/return for the enclosing loop) seems doubtful and doesn't cover the simplest/most common cases. --- .../controlflow/PyControlFlowBuilder.java | 21 ++++--------- .../implicitnegativetypeassertionafterif.py | 3 ++ .../implicitnegativetypeassertionafterif.txt | 12 ++++++++ ...itnegativetypeassertionaftertwonestedif.py | 5 ++++ ...tnegativetypeassertionaftertwonestedif.txt | 21 +++++++++++++ .../codeInsight/controlflow/tryfinally.txt | 30 ++++++++++--------- .../python/PyControlFlowBuilderTest.java | 10 +++++++ 7 files changed, 72 insertions(+), 30 deletions(-) create mode 100644 python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.py create mode 100644 python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.txt create mode 100644 python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.py create mode 100644 python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.txt diff --git a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java index da1716e952a1..a7e271d0dd17 100644 --- a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java +++ b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java @@ -365,13 +365,6 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { }); myBuilder.addPendingEdge(node, myBuilder.prevInstruction); } - final Ref pendingInScopeEdges = Ref.create(false); - myBuilder.processPending((pendingScope, instruction) -> { - if (pendingScope != null && PsiTreeUtil.isAncestor(node, pendingScope, false)) { - pendingInScopeEdges.set(true); - } - myBuilder.addPendingEdge(pendingScope, instruction); - }); final PyTypeAssertionEvaluator negativeAssertionEvaluator = new PyTypeAssertionEvaluator(false); final PyExpression ifCondition = ifPart.getCondition(); // TODO: Add support for 'elif' @@ -386,15 +379,11 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { InstructionBuilder.addAssertInstructions(myBuilder, negativeAssertionEvaluator); elseBranch.accept(this); myBuilder.addPendingEdge(node, myBuilder.prevInstruction); - } else { - if (!pendingInScopeEdges.get()) { - myBuilder.prevInstruction = lastBranchingPoint; - InstructionBuilder.addAssertInstructions(myBuilder, negativeAssertionEvaluator); - myBuilder.addPendingEdge(node, myBuilder.prevInstruction); - } - else { - myBuilder.addPendingEdge(node, lastBranchingPoint); - } + } + else { + myBuilder.prevInstruction = lastBranchingPoint; + InstructionBuilder.addAssertInstructions(myBuilder, negativeAssertionEvaluator); + myBuilder.addPendingEdge(node, myBuilder.prevInstruction); } } diff --git a/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.py b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.py new file mode 100644 index 000000000000..560fa098578b --- /dev/null +++ b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.py @@ -0,0 +1,3 @@ +if xs is None: + xs = [1, 2, 3] +print(xs) \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.txt b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.txt new file mode 100644 index 000000000000..e4f1a6beb5f0 --- /dev/null +++ b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionafterif.txt @@ -0,0 +1,12 @@ +0(1) element: null +1(2) element: PyIfStatement +2(3) READ ACCESS: xs +3(4,8) READ ACCESS: None +4(5) element: PyStatementList. Condition: xs is None:true +5(6) ASSERTTYPE ACCESS: xs +6(7) element: PyAssignmentStatement +7(9) WRITE ACCESS: xs +8(9) ASSERTTYPE ACCESS: xs +9(10) element: PyPrintStatement +10(11) READ ACCESS: xs +11() element: null \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.py b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.py new file mode 100644 index 000000000000..8eeb463d6cef --- /dev/null +++ b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.py @@ -0,0 +1,5 @@ +if not isinstance(x, str): + if x is None: + print(x) + print(x) +print(x) \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.txt b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.txt new file mode 100644 index 000000000000..2243367e6343 --- /dev/null +++ b/python/testData/codeInsight/controlflow/implicitnegativetypeassertionaftertwonestedif.txt @@ -0,0 +1,21 @@ +0(1) element: null +1(2) element: PyIfStatement +2(3) READ ACCESS: isinstance +3(4) READ ACCESS: x +4(5,17) READ ACCESS: str +5(6) element: PyStatementList. Condition: not isinstance(x, str):true +6(7) ASSERTTYPE ACCESS: x +7(8) element: PyIfStatement +8(9) READ ACCESS: x +9(10,14) READ ACCESS: None +10(11) element: PyStatementList. Condition: x is None:true +11(12) ASSERTTYPE ACCESS: x +12(13) element: PyPrintStatement +13(15) READ ACCESS: x +14(15) ASSERTTYPE ACCESS: x +15(16) element: PyPrintStatement +16(18) READ ACCESS: x +17(18) ASSERTTYPE ACCESS: x +18(19) element: PyPrintStatement +19(20) READ ACCESS: x +20() element: null \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/tryfinally.txt b/python/testData/codeInsight/controlflow/tryfinally.txt index 3409584f677d..3a694cd2e856 100644 --- a/python/testData/codeInsight/controlflow/tryfinally.txt +++ b/python/testData/codeInsight/controlflow/tryfinally.txt @@ -6,23 +6,25 @@ 5(6,9) element: PyTryPart 6(7,9) element: PyAssignmentStatement 7(8,9) READ ACCESS: open -8(9,17) WRITE ACCESS: status +8(9,18) WRITE ACCESS: status 9(10) element: PyFinallyPart 10(11) element: PyIfStatement 11(12) READ ACCESS: status -12(13,27) READ ACCESS: None +12(13,17) READ ACCESS: None 13(14) element: PyStatementList. Condition: status is not None:true 14(15) ASSERTTYPE ACCESS: status 15(16) element: PyPrintStatement -16(27) READ ACCESS: status -17(18) element: PyFinallyPart -18(19) element: PyIfStatement -19(20) READ ACCESS: status -20(21,25) READ ACCESS: None -21(22) element: PyStatementList. Condition: status is not None:true -22(23) ASSERTTYPE ACCESS: status -23(24) element: PyPrintStatement -24(25) READ ACCESS: status -25(26) element: PyExpressionStatement -26(27) READ ACCESS: status -27() element: null \ No newline at end of file +16(29) READ ACCESS: status +17(29) ASSERTTYPE ACCESS: status +18(19) element: PyFinallyPart +19(20) element: PyIfStatement +20(21) READ ACCESS: status +21(22,26) READ ACCESS: None +22(23) element: PyStatementList. Condition: status is not None:true +23(24) ASSERTTYPE ACCESS: status +24(25) element: PyPrintStatement +25(27) READ ACCESS: status +26(27) ASSERTTYPE ACCESS: status +27(28) element: PyExpressionStatement +28(29) READ ACCESS: status +29() element: null \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java b/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java index 5e108b72f203..90a46a31a4a5 100644 --- a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java +++ b/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java @@ -238,6 +238,16 @@ public class PyControlFlowBuilderTest extends LightMarkedTestCase { runWithLanguageLevel(LanguageLevel.PYTHON36, this::doTest); } + // PY-21175 + public void testImplicitNegativeTypeAssertionAfterIf() { + doTest(); + } + + // PY-21175 + public void testImplicitNegativeTypeAssertionAfterTwoNestedIf() { + doTest(); + } + private void doTestFirstStatement() { final String testName = getTestName(false).toLowerCase(); configureByFile(testName + ".py");