From 19699cde614c399481e5ccfa4a2be4447f8c57c5 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Tue, 6 Mar 2018 17:58:09 +0300 Subject: [PATCH] Fix and unify calculating corresponding loop for `break` and `continue` (PY-23189) --- .../com/jetbrains/python/PyBundle.properties | 1 - .../PyBreakContinueGotoProvider.java | 46 ++++++++----------- .../codeFragment/PyCodeFragmentUtil.java | 5 +- .../src/com/jetbrains/python/psi/PyUtil.java | 18 ++++++++ .../python/psi/impl/PyBreakStatementImpl.java | 23 ++-------- .../psi/impl/PyContinueStatementImpl.java | 4 +- .../validation/BreakContinueAnnotator.java | 17 +++---- .../highlighting/breakOutsideOfLoop.py | 12 ++++- .../highlighting/continueInFinallyBlock.py | 5 -- .../highlighting/continueOutsideOfLoop.py | 15 ++++++ .../python/PythonHighlightingTest.java | 2 +- 11 files changed, 80 insertions(+), 68 deletions(-) delete mode 100644 python/testData/highlighting/continueInFinallyBlock.py create mode 100644 python/testData/highlighting/continueOutsideOfLoop.py diff --git a/python/src/com/jetbrains/python/PyBundle.properties b/python/src/com/jetbrains/python/PyBundle.properties index f890a1fd3075..6659f75d1a08 100644 --- a/python/src/com/jetbrains/python/PyBundle.properties +++ b/python/src/com/jetbrains/python/PyBundle.properties @@ -647,7 +647,6 @@ refactoring.extract.method.error.returns=Cannot extract method with return instr refactoring.extract.method.error.local.variable.modifications=Cannot perform refactoring from expression with local variable modifications inside code fragment refactoring.extract.method.error.local.variable.modifications.and.returns=Cannot perform refactoring from expression with local variables modifications and return instructions inside code fragment refactoring.extract.method.error.empty.fragment=Cannot perform refactoring from empty code fragment -refactoring.extract.method.error.undetermined.execution.flow=Cannot determine execution flow for the code fragment refactoring.extract.method.error.yield=Cannot perform refactoring with 'yield' statement inside code block refactoring.extract.method.error.class.level=Cannot perform refactoring at class level diff --git a/python/src/com/jetbrains/python/codeInsight/PyBreakContinueGotoProvider.java b/python/src/com/jetbrains/python/codeInsight/PyBreakContinueGotoProvider.java index 71655217ee46..a39f169e6756 100644 --- a/python/src/com/jetbrains/python/codeInsight/PyBreakContinueGotoProvider.java +++ b/python/src/com/jetbrains/python/codeInsight/PyBreakContinueGotoProvider.java @@ -2,14 +2,11 @@ package com.jetbrains.python.codeInsight; import com.intellij.codeInsight.navigation.actions.GotoDeclarationHandlerBase; -import com.intellij.lang.ASTNode; import com.intellij.openapi.editor.Editor; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.psi.PsiWhiteSpace; -import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; -import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.PythonLanguage; import com.jetbrains.python.psi.*; import org.jetbrains.annotations.Nullable; @@ -22,33 +19,30 @@ public class PyBreakContinueGotoProvider extends GotoDeclarationHandlerBase { @Override public PsiElement getGotoDeclarationTarget(@Nullable PsiElement source, Editor editor) { if (source != null && source.getLanguage() instanceof PythonLanguage) { - final PyLoopStatement loop = PsiTreeUtil.getParentOfType(source, PyLoopStatement.class, false, PyFunction.class, PyClass.class); + final PsiElement breakOrContinue = source.getParent(); + final PyLoopStatement loop = PyUtil.getCorrespondingLoop(breakOrContinue); if (loop != null) { - ASTNode node = source.getNode(); - if (node != null) { - IElementType node_type = node.getElementType(); - if (node_type == PyTokenTypes.CONTINUE_KEYWORD) { - return loop; - } - if (node_type == PyTokenTypes.BREAK_KEYWORD) { - PsiElement outer_element = loop; - PsiElement after_cycle; - while (true) { - after_cycle = outer_element.getNextSibling(); - if (after_cycle != null) { - if (after_cycle instanceof PsiWhiteSpace) { - after_cycle = after_cycle.getNextSibling(); - } - if (after_cycle instanceof PyStatement) return after_cycle; - } - outer_element = outer_element.getParent(); - if (PsiTreeUtil.instanceOf(outer_element, PsiFile.class, PyFunction.class, PyClass.class)) { - break; + if (breakOrContinue instanceof PyContinueStatement) { + return loop; + } + else { + PsiElement outer_element = loop; + PsiElement after_cycle; + while (true) { + after_cycle = outer_element.getNextSibling(); + if (after_cycle != null) { + if (after_cycle instanceof PsiWhiteSpace) { + after_cycle = after_cycle.getNextSibling(); } + if (after_cycle instanceof PyStatement) return after_cycle; + } + outer_element = outer_element.getParent(); + if (PsiTreeUtil.instanceOf(outer_element, PsiFile.class, PyFunction.class, PyClass.class)) { + break; } - // cycle is the last statement in the flow of execution. its last element is our best bet. - return PsiTreeUtil.getDeepestLast(loop); } + // cycle is the last statement in the flow of execution. its last element is our best bet. + return PsiTreeUtil.getDeepestLast(loop); } } } diff --git a/python/src/com/jetbrains/python/codeInsight/codeFragment/PyCodeFragmentUtil.java b/python/src/com/jetbrains/python/codeInsight/codeFragment/PyCodeFragmentUtil.java index 28d7005b44b1..cb9bcba807c1 100644 --- a/python/src/com/jetbrains/python/codeInsight/codeFragment/PyCodeFragmentUtil.java +++ b/python/src/com/jetbrains/python/codeInsight/codeFragment/PyCodeFragmentUtil.java @@ -55,9 +55,6 @@ public class PyCodeFragmentUtil { final int start = startInScope.getTextOffset(); final int end = endInScope.getTextOffset() + endInScope.getTextLength(); final ControlFlow flow = ControlFlowCache.getControlFlow(owner); - if (flow == null) { - throw new CannotCreateCodeFragmentException(PyBundle.message("refactoring.extract.method.error.undetermined.execution.flow")); - } final List graph = Arrays.asList(flow.getInstructions()); final List subGraph = getFragmentSubGraph(graph, start, end); final AnalysisResult subGraphAnalysis = analyseSubGraph(subGraph, start, end); @@ -246,7 +243,7 @@ public class PyCodeFragmentUtil { } } if (element instanceof PyContinueStatement || element instanceof PyBreakStatement) { - final PyLoopStatement loopStatement = PsiTreeUtil.getParentOfType(element, PyLoopStatement.class); + final PyLoopStatement loopStatement = PyUtil.getCorrespondingLoop(element); if (loopStatement != null && !subGraphElements.contains(loopStatement)) { outerLoopBreaks++; } diff --git a/python/src/com/jetbrains/python/psi/PyUtil.java b/python/src/com/jetbrains/python/psi/PyUtil.java index 15cda1093999..4c9883c91783 100644 --- a/python/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/src/com/jetbrains/python/psi/PyUtil.java @@ -1850,6 +1850,24 @@ public class PyUtil { return false; } + @Nullable + public static PyLoopStatement getCorrespondingLoop(@NotNull PsiElement breakOrContinue) { + return breakOrContinue instanceof PyContinueStatement || breakOrContinue instanceof PyBreakStatement + ? getCorrespondingLoopImpl(breakOrContinue) + : null; + } + + @Nullable + private static PyLoopStatement getCorrespondingLoopImpl(@NotNull PsiElement element) { + final PyLoopStatement loop = PsiTreeUtil.getParentOfType(element, PyLoopStatement.class, true, ScopeOwner.class); + + if (loop instanceof PyStatementWithElse && PsiTreeUtil.isAncestor(((PyStatementWithElse)loop).getElsePart(), element, true)) { + return getCorrespondingLoopImpl(loop); + } + + return loop; + } + /** * This helper class allows to collect various information about AST nodes composing {@link PyStringLiteralExpression}. */ diff --git a/python/src/com/jetbrains/python/psi/impl/PyBreakStatementImpl.java b/python/src/com/jetbrains/python/psi/impl/PyBreakStatementImpl.java index 908bfa234cde..430d3f348369 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyBreakStatementImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyBreakStatementImpl.java @@ -16,10 +16,10 @@ package com.jetbrains.python.psi.impl; import com.intellij.lang.ASTNode; -import com.intellij.psi.PsiElement; -import com.intellij.psi.util.PsiTreeUtil; -import com.jetbrains.python.psi.*; -import org.jetbrains.annotations.NotNull; +import com.jetbrains.python.psi.PyBreakStatement; +import com.jetbrains.python.psi.PyElementVisitor; +import com.jetbrains.python.psi.PyLoopStatement; +import com.jetbrains.python.psi.PyUtil; import org.jetbrains.annotations.Nullable; /** @@ -37,19 +37,6 @@ public class PyBreakStatementImpl extends PyElementImpl implements PyBreakStatem @Nullable public PyLoopStatement getLoopStatement() { - return getLoopStatement(this); - } - - @Nullable - private static PyLoopStatement getLoopStatement(@NotNull PsiElement element) { - final PyLoopStatement loop = PsiTreeUtil.getParentOfType(element, PyLoopStatement.class); - if (loop instanceof PyStatementWithElse) { - final PyStatementWithElse stmt = (PyStatementWithElse)loop; - final PyElsePart elsePart = stmt.getElsePart(); - if (PsiTreeUtil.isAncestor(elsePart, element, true)) { - return getLoopStatement(loop); - } - } - return loop; + return PyUtil.getCorrespondingLoop(this); } } diff --git a/python/src/com/jetbrains/python/psi/impl/PyContinueStatementImpl.java b/python/src/com/jetbrains/python/psi/impl/PyContinueStatementImpl.java index 3635e689431f..864603316b0e 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyContinueStatementImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyContinueStatementImpl.java @@ -16,10 +16,10 @@ package com.jetbrains.python.psi.impl; import com.intellij.lang.ASTNode; -import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.python.psi.PyContinueStatement; import com.jetbrains.python.psi.PyElementVisitor; import com.jetbrains.python.psi.PyLoopStatement; +import com.jetbrains.python.psi.PyUtil; import org.jetbrains.annotations.Nullable; /** @@ -37,6 +37,6 @@ public class PyContinueStatementImpl extends PyElementImpl implements PyContinue @Nullable public PyLoopStatement getLoopStatement() { - return PsiTreeUtil.getParentOfType(this, PyLoopStatement.class); + return PyUtil.getCorrespondingLoop(this); } } diff --git a/python/src/com/jetbrains/python/validation/BreakContinueAnnotator.java b/python/src/com/jetbrains/python/validation/BreakContinueAnnotator.java index 4705bfad254a..8bd227b8a8c9 100644 --- a/python/src/com/jetbrains/python/validation/BreakContinueAnnotator.java +++ b/python/src/com/jetbrains/python/validation/BreakContinueAnnotator.java @@ -16,8 +16,10 @@ package com.jetbrains.python.validation; import com.intellij.psi.util.PsiTreeUtil; -import com.jetbrains.python.psi.*; -import org.jetbrains.annotations.Nullable; +import com.jetbrains.python.psi.PyBreakStatement; +import com.jetbrains.python.psi.PyContinueStatement; +import com.jetbrains.python.psi.PyFinallyPart; +import com.jetbrains.python.psi.PyLoopStatement; import static com.jetbrains.python.PyBundle.message; @@ -27,22 +29,17 @@ import static com.jetbrains.python.PyBundle.message; public class BreakContinueAnnotator extends PyAnnotator { @Override public void visitPyBreakStatement(final PyBreakStatement node) { - if (getContainingLoop(node) == null) { + if (node.getLoopStatement() == null) { getHolder().createErrorAnnotation(node, message("ANN.break.outside.loop")); } } - @Nullable - private static PyLoopStatement getContainingLoop(PyStatement node) { - return PsiTreeUtil.getParentOfType(node, PyLoopStatement.class, false, PyFunction.class, PyClass.class); - } - @Override public void visitPyContinueStatement(final PyContinueStatement node) { - if (getContainingLoop(node) == null) { + if (node.getLoopStatement() == null) { getHolder().createErrorAnnotation(node, message("ANN.continue.outside.loop")); } - else if (PsiTreeUtil.getParentOfType(node, PyFinallyPart.class, false, PyLoopStatement.class) != null) { + else if (PsiTreeUtil.getParentOfType(node, PyFinallyPart.class, false, PyLoopStatement.class) != null) { getHolder().createErrorAnnotation(node, message("ANN.cant.continue.in.finally")); } } diff --git a/python/testData/highlighting/breakOutsideOfLoop.py b/python/testData/highlighting/breakOutsideOfLoop.py index c64227dbf550..847408249aee 100644 --- a/python/testData/highlighting/breakOutsideOfLoop.py +++ b/python/testData/highlighting/breakOutsideOfLoop.py @@ -1,3 +1,13 @@ for x in []: def foo(x): - break \ No newline at end of file + break + +for x in [1, 2, 3]: + pass +else: + break + +while True: + pass +else: + break \ No newline at end of file diff --git a/python/testData/highlighting/continueInFinallyBlock.py b/python/testData/highlighting/continueInFinallyBlock.py deleted file mode 100644 index a1615782d8c2..000000000000 --- a/python/testData/highlighting/continueInFinallyBlock.py +++ /dev/null @@ -1,5 +0,0 @@ -while True: - try: - print "a" - finally: - continue \ No newline at end of file diff --git a/python/testData/highlighting/continueOutsideOfLoop.py b/python/testData/highlighting/continueOutsideOfLoop.py new file mode 100644 index 000000000000..054c5c255b2f --- /dev/null +++ b/python/testData/highlighting/continueOutsideOfLoop.py @@ -0,0 +1,15 @@ +while True: + try: + print "a" + finally: + continue + +for x in [1, 2, 3]: + pass +else: + continue + +while True: + pass +else: + continue \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PythonHighlightingTest.java b/python/testSrc/com/jetbrains/python/PythonHighlightingTest.java index 8c12549271e8..f3b112cc5971 100644 --- a/python/testSrc/com/jetbrains/python/PythonHighlightingTest.java +++ b/python/testSrc/com/jetbrains/python/PythonHighlightingTest.java @@ -76,7 +76,7 @@ public class PythonHighlightingTest extends PyTestCase { doTest(); } - public void testContinueInFinallyBlock() { + public void testContinueOutsideOfLoop() { doTest(false, false); }