From 36df2222b6d7a15ba62805f4a53ed0d6a745f35d Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 23 Mar 2018 11:47:04 +0300 Subject: [PATCH] PY-24942 Don't chain AND expressions in Simplify chained comparison When the leftmost binary relationship expression of the two being transformed belongs itself to the left operand of a nested logical AND. Previously, we mistakenly tried to insert the containing logical AND expression into the generated chained comparison when useRightChildOfLeft parameter of the quickfix was false. --- .../quickfix/ChainedComparisonsQuickFix.java | 24 +++++++------------ .../inspections/ChainedComparison8.py | 1 + .../inspections/ChainedComparison8_after.py | 1 + .../com/jetbrains/python/PyQuickFixTest.java | 5 ++++ 4 files changed, 15 insertions(+), 16 deletions(-) create mode 100644 python/testData/inspections/ChainedComparison8.py create mode 100644 python/testData/inspections/ChainedComparison8_after.py diff --git a/python/src/com/jetbrains/python/inspections/quickfix/ChainedComparisonsQuickFix.java b/python/src/com/jetbrains/python/inspections/quickfix/ChainedComparisonsQuickFix.java index a5527860d303..d6e001a37eb7 100644 --- a/python/src/com/jetbrains/python/inspections/quickfix/ChainedComparisonsQuickFix.java +++ b/python/src/com/jetbrains/python/inspections/quickfix/ChainedComparisonsQuickFix.java @@ -63,25 +63,17 @@ public class ChainedComparisonsQuickFix implements LocalQuickFix { public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { final PyBinaryExpression expression = as(descriptor.getPsiElement(), PyBinaryExpression.class); - if (expression != null && expression.isWritable()) { + if (isLogicalAndExpression(expression) && expression.isWritable()) { final PyBinaryExpression rightExpression = as(expression.getRightExpression(), PyBinaryExpression.class); - final PyBinaryExpression leftExpression = as(expression.getLeftExpression(), PyBinaryExpression.class); - - if (rightExpression != null && leftExpression != null && isLogicalAndExpression(expression)) { - applyFix(goDownIfNeeded(leftExpression), rightExpression, project); + PyBinaryExpression leftExpression = as(expression.getLeftExpression(), PyBinaryExpression.class); + if (isLogicalAndExpression(leftExpression)) { + final PyExpression nested = myUseRightChildOfLeft ? leftExpression.getRightExpression() : leftExpression.getLeftExpression(); + leftExpression = as(nested, PyBinaryExpression.class); } - } - } - @NotNull - private PyBinaryExpression goDownIfNeeded(@NotNull PyBinaryExpression expression) { - final PyExpression rightExpression = expression.getRightExpression(); - - if (myUseRightChildOfLeft && rightExpression instanceof PyBinaryExpression && isLogicalAndExpression(expression)) { - return (PyBinaryExpression)rightExpression; - } - else { - return expression; + if (isComparisonExpression(leftExpression) && isComparisonExpression(rightExpression)) { + applyFix(leftExpression, rightExpression, project); + } } } diff --git a/python/testData/inspections/ChainedComparison8.py b/python/testData/inspections/ChainedComparison8.py new file mode 100644 index 000000000000..d4b87455ec15 --- /dev/null +++ b/python/testData/inspections/ChainedComparison8.py @@ -0,0 +1 @@ +row >= 0 and 0 <= col < self.width and row < self.height \ No newline at end of file diff --git a/python/testData/inspections/ChainedComparison8_after.py b/python/testData/inspections/ChainedComparison8_after.py new file mode 100644 index 000000000000..fa8e3738f513 --- /dev/null +++ b/python/testData/inspections/ChainedComparison8_after.py @@ -0,0 +1 @@ +0 <= row < self.height and 0 <= col < self.width \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java index 3c44383b52da..de6b66bcbd50 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java @@ -301,6 +301,11 @@ public class PyQuickFixTest extends PyTestCase { doInspectionTest(PyChainedComparisonsInspection.class, "Simplify chained comparison", true, true); } + // PY-24942 + public void testChainedComparison8() { + doInspectionTest(PyChainedComparisonsInspection.class, "Simplify chained comparison", true, true); + } + // PY-1362, PY-2585 public void testStatementEffect() { doInspectionTest(PyStatementEffectInspection.class, PyBundle.message("QFIX.statement.effect"), true, true);