Fix and unify calculating corresponding loop for break and continue (PY-23189)

This commit is contained in:
Semyon Proshev
2018-03-06 17:58:09 +03:00
parent 12527d82c1
commit 19699cde61
11 changed files with 80 additions and 68 deletions

View File

@@ -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

View File

@@ -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);
}
}
}

View File

@@ -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<Instruction> graph = Arrays.asList(flow.getInstructions());
final List<Instruction> 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++;
}

View File

@@ -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}.
*/

View File

@@ -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);
}
}

View File

@@ -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);
}
}

View File

@@ -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"));
}
}

View File

@@ -1,3 +1,13 @@
for x in []:
def foo(x):
<error descr="'break' outside loop">break</error>
<error descr="'break' outside loop">break</error>
for x in [1, 2, 3]:
pass
else:
<error descr="'break' outside loop">break</error>
while True:
pass
else:
<error descr="'break' outside loop">break</error>

View File

@@ -1,5 +0,0 @@
while True:
try:
print "a"
finally:
<error descr="'continue' not supported inside 'finally' clause">continue</error>

View File

@@ -0,0 +1,15 @@
while True:
try:
print "a"
finally:
<error descr="'continue' not supported inside 'finally' clause">continue</error>
for x in [1, 2, 3]:
pass
else:
<error descr="'continue' outside loop">continue</error>
while True:
pass
else:
<error descr="'continue' outside loop">continue</error>

View File

@@ -76,7 +76,7 @@ public class PythonHighlightingTest extends PyTestCase {
doTest();
}
public void testContinueInFinallyBlock() {
public void testContinueOutsideOfLoop() {
doTest(false, false);
}