fixed PY-6467 Simplify chained comparison suggests strange solutions

This commit is contained in:
Ekaterina Tuzova
2012-05-02 13:30:55 +04:00
parent eb33aeb4e4
commit 936c7f4d0a
10 changed files with 95 additions and 26 deletions

View File

@@ -19,9 +19,12 @@ import org.jetbrains.annotations.Nullable;
public class ChainedComparisonsQuickFix implements LocalQuickFix {
boolean myIsLeftLeft;
boolean myIsRightLeft;
public ChainedComparisonsQuickFix(boolean isLeft, boolean isRight) {
Boolean getInnerRight;
public ChainedComparisonsQuickFix(boolean isLeft, boolean isRight, @Nullable Boolean getInner) {
myIsLeftLeft = isLeft;
myIsRightLeft = isRight;
getInnerRight = getInner;
}
@NotNull
@@ -42,6 +45,10 @@ public class ChainedComparisonsQuickFix implements LocalQuickFix {
PyExpression rightExpression = ((PyBinaryExpression)expression).getRightExpression();
if (rightExpression instanceof PyBinaryExpression && leftExpression instanceof PyBinaryExpression) {
if (((PyBinaryExpression)expression).getOperator() == PyTokenTypes.AND_KEYWORD) {
if (getInnerRight && ((PyBinaryExpression)leftExpression).getRightExpression() instanceof PyBinaryExpression
&& PyTokenTypes.AND_KEYWORD == ((PyBinaryExpression)leftExpression).getOperator()) {
leftExpression = ((PyBinaryExpression)leftExpression).getRightExpression();
}
checkOperator((PyBinaryExpression)leftExpression, (PyBinaryExpression)rightExpression, project);
}
}

View File

@@ -6,7 +6,9 @@ import com.intellij.psi.PsiElementVisitor;
import com.jetbrains.python.PyBundle;
import com.jetbrains.python.PyTokenTypes;
import com.jetbrains.python.actions.ChainedComparisonsQuickFix;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.PyBinaryExpression;
import com.jetbrains.python.psi.PyElementType;
import com.jetbrains.python.psi.PyExpression;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -36,6 +38,8 @@ public class PyChainedComparisonsInspection extends PyInspection {
private static class Visitor extends PyInspectionVisitor {
boolean myIsLeft;
boolean myIsRight;
PyElementType myOperator;
boolean getInnerRight;
public Visitor(@Nullable ProblemsHolder holder, @NotNull LocalInspectionToolSession session) {
super(holder, session);
@@ -43,6 +47,11 @@ public class PyChainedComparisonsInspection extends PyInspection {
@Override
public void visitPyBinaryExpression(final PyBinaryExpression node){
myIsLeft = false;
myIsRight = false;
myOperator = null;
getInnerRight = false;
PyExpression leftExpression = node.getLeftExpression();
PyExpression rightExpression = node.getRightExpression();
@@ -51,28 +60,38 @@ public class PyChainedComparisonsInspection extends PyInspection {
if (node.getOperator() == PyTokenTypes.AND_KEYWORD) {
if (isRightSimplified((PyBinaryExpression)leftExpression, (PyBinaryExpression)rightExpression) ||
isLeftSimplified((PyBinaryExpression)leftExpression, (PyBinaryExpression)rightExpression))
registerProblem(node, "Simplify chained comparison", new ChainedComparisonsQuickFix(myIsLeft, myIsRight));
registerProblem(node, "Simplify chained comparison", new ChainedComparisonsQuickFix(myIsLeft, myIsRight,
getInnerRight));
}
}
}
private boolean isRightSimplified(PyBinaryExpression leftExpression, PyBinaryExpression rightExpression) {
if (leftExpression.getRightExpression() instanceof PyBinaryExpression &&
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)leftExpression.getRightExpression()).getOperator())){
if (isRightSimplified((PyBinaryExpression)leftExpression.getRightExpression(), rightExpression))
private boolean isRightSimplified(@NotNull final PyBinaryExpression leftExpression,
@NotNull final PyBinaryExpression rightExpression) {
final PyExpression leftRight = leftExpression.getRightExpression();
if (leftRight != null && PyTokenTypes.AND_KEYWORD == leftExpression.getOperator()) {
if (isRightSimplified((PyBinaryExpression)leftRight, rightExpression)) {
getInnerRight = true;
return true;
}
}
if (leftRight instanceof PyBinaryExpression &&
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)leftRight).getOperator())){
if (isRightSimplified((PyBinaryExpression)leftRight, rightExpression))
return true;
}
if (PyTokenTypes.RELATIONAL_OPERATIONS.contains(leftExpression.getOperator())) {
PyExpression leftRight = leftExpression.getRightExpression();
myOperator = leftExpression.getOperator();
if (PyTokenTypes.RELATIONAL_OPERATIONS.contains(myOperator)) {
if (leftRight != null) {
if (leftRight.getText().equals(getLeftExpression(rightExpression).getText())) {
if (leftRight.getText().equals(getLeftExpression(rightExpression, true).getText())) {
myIsLeft = false;
myIsRight = true;
return true;
}
PyExpression right = getSmallestRight(rightExpression);
final PyExpression right = getSmallestRight(rightExpression, true);
if (right != null && leftRight.getText().equals(right.getText())) {
myIsLeft = false;
myIsRight = false;
@@ -83,22 +102,41 @@ public class PyChainedComparisonsInspection extends PyInspection {
return false;
}
private static boolean isOpposite(final PyElementType op1, final PyElementType op2) {
if ((op1 == PyTokenTypes.GT || op1 == PyTokenTypes.GE) && (op2 == PyTokenTypes.LT || op2 == PyTokenTypes.LE))
return true;
if ((op2 == PyTokenTypes.GT || op2 == PyTokenTypes.GE) && (op1 == PyTokenTypes.LT || op1 == PyTokenTypes.LE))
return true;
return false;
}
private boolean isLeftSimplified(PyBinaryExpression leftExpression, PyBinaryExpression rightExpression) {
if (leftExpression.getLeftExpression() instanceof PyBinaryExpression &&
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)leftExpression.getLeftExpression()).getOperator())){
if (isLeftSimplified((PyBinaryExpression)leftExpression.getLeftExpression(), rightExpression))
final PyExpression leftRight = leftExpression.getLeftExpression();
if (leftExpression.getRightExpression() instanceof PyBinaryExpression
&& PyTokenTypes.AND_KEYWORD == leftExpression.getOperator()) {
if (isLeftSimplified((PyBinaryExpression)leftExpression.getRightExpression(), rightExpression)) {
getInnerRight = true;
return true;
}
}
if (leftRight instanceof PyBinaryExpression &&
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)leftRight).getOperator())){
if (isLeftSimplified((PyBinaryExpression)leftRight, rightExpression))
return true;
}
if (PyTokenTypes.RELATIONAL_OPERATIONS.contains(leftExpression.getOperator())) {
PyExpression leftRight = leftExpression.getLeftExpression();
myOperator = leftExpression.getOperator();
if (PyTokenTypes.RELATIONAL_OPERATIONS.contains(myOperator)) {
if (leftRight != null) {
if (leftRight.getText().equals(getLeftExpression(rightExpression).getText())) {
if (leftRight.getText().equals(getLeftExpression(rightExpression, false).getText())) {
myIsLeft = true;
myIsRight = true;
return true;
}
PyExpression right = getSmallestRight(rightExpression);
final PyExpression right = getSmallestRight(rightExpression, false);
if (right != null && leftRight.getText().equals(right.getText())) {
myIsLeft = true;
myIsRight = false;
@@ -109,21 +147,30 @@ public class PyChainedComparisonsInspection extends PyInspection {
return false;
}
static private PyExpression getLeftExpression(PyBinaryExpression expression) {
private PyExpression getLeftExpression(PyBinaryExpression expression, boolean isRight) {
PyExpression result = expression;
while (result instanceof PyBinaryExpression && (
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)result).getOperator())
|| PyTokenTypes.EQUALITY_OPERATIONS.contains(((PyBinaryExpression)result).getOperator()))) {
final boolean opposite = isOpposite(((PyBinaryExpression)result).getOperator(), myOperator);
if ((isRight && opposite) || (!isRight && !opposite))
break;
result = ((PyBinaryExpression)result).getLeftExpression();
}
return result;
}
static private PyExpression getSmallestRight(PyBinaryExpression expression) {
@Nullable
private PyExpression getSmallestRight(PyBinaryExpression expression, boolean isRight) {
PyExpression result = expression;
while (result instanceof PyBinaryExpression && (
PyTokenTypes.RELATIONAL_OPERATIONS.contains(((PyBinaryExpression)result).getOperator())
|| PyTokenTypes.EQUALITY_OPERATIONS.contains(((PyBinaryExpression)result).getOperator()))) {
final boolean opposite = isOpposite(((PyBinaryExpression)result).getOperator(), myOperator);
if ((isRight && !opposite) || (!isRight && opposite))
break;
result = ((PyBinaryExpression)result).getRightExpression();
}
return result;

View File

@@ -1,2 +1,2 @@
if <weak_warning descr="Simplify chained comparison">b >= <caret>a > e and c < b</weak_warning>:
if <weak_warning descr="Simplify chained comparison">b >= <caret>a > e and c > b</weak_warning>:
print "q"

View File

@@ -1,2 +1,2 @@
if e < a <= b > c:
if e < a <= b < c:
print "q"

View File

@@ -1 +1 @@
a = <weak_warning descr="Simplify chained comparison">0 <= <caret>x < top and 0 <= y < top</weak_warning>
a = <weak_warning descr="Simplify chained comparison">0 <= <caret>x < top and 0 >= y > top</weak_warning>

View File

@@ -1 +1 @@
a = 0 <= x < top > y >= 0
a = 0 <= x < top < y <= 0

View File

@@ -0,0 +1,5 @@
mapsize = 35
def test(x, y):
if <weak_warning descr="Simplify chained comparison">0 <= x < <caret>mapsize and y >= 0 and y < mapsize</weak_warning>:
return 1

View File

@@ -0,0 +1,5 @@
mapsize = 35
def test(x, y):
if 0 <= x < mapsize and 0 <= y < mapsize:
return 1

View File

@@ -14,7 +14,7 @@ q = <weak_warning descr="Simplify chained comparison">a >= b >= c and c > d</wea
if <weak_warning descr="Simplify chained comparison">b > a and b < c</weak_warning>:
print ("a")
if <weak_warning descr="Simplify chained comparison">c > a and b < c</weak_warning>:
if c > a and b < c:
print("b")
if <weak_warning descr="Simplify chained comparison">a > c and b < c</weak_warning>:

View File

@@ -256,6 +256,11 @@ public class PyQuickFixTest extends PyTestCase {
PyBundle.message("QFIX.chained.comparison"), true, true);
}
public void testChainedComparison5() { // PY-6467
doInspectionTest("ChainedComparison5.py", PyChainedComparisonsInspection.class,
PyBundle.message("QFIX.chained.comparison"), true, true);
}
public void testStatementEffect() { // PY-1362, PY-2585
doInspectionTest("StatementEffect.py", PyStatementEffectInspection.class,
PyBundle.message("QFIX.statement.effect"), true, true);