All-around check that operator precedence is not broken during inlining

* Add missing priorities for operators: XOR, boolean NOT and floordiv.
* Always surround ternary conditional operator in braces if it's going to be inlined
in another conditional expression for readability sake (though it does not break
semantics if we inline in the else branch).
This commit is contained in:
Mikhail Golubev
2014-10-03 13:36:23 +04:00
parent 17fde0f0ff
commit 25e070c338
17 changed files with 912 additions and 21 deletions
@@ -83,6 +83,9 @@ public class PyReplaceExpressionUtil implements PyElementTypes {
return true;
}
}
else if (newExpr instanceof PyConditionalExpression && parentExpr instanceof PyConditionalExpression) {
return true;
}
return false;
}
@@ -433,30 +436,32 @@ public class PyReplaceExpressionUtil implements PyElementTypes {
private static boolean isNotAssociative(@NotNull final PyBinaryExpression binaryExpression) {
final IElementType opType = getOperationType(binaryExpression);
return COMPARISON_OPERATIONS.contains(opType) || binaryExpression instanceof PySliceExpression ||
opType == DIV || opType == PERC || opType == EXP || opType == MINUS;
opType == DIV || opType == FLOORDIV || opType == PERC || opType == EXP || opType == MINUS;
}
private static int getExpressionPriority(PyElement expr) {
int priority = 0;
if (expr instanceof PySubscriptionExpression || expr instanceof PySliceExpression ||
expr instanceof PyCallExpression) priority = 1;
if (expr instanceof PyPrefixExpression) {
if (expr instanceof PySubscriptionExpression || expr instanceof PySliceExpression || expr instanceof PyCallExpression) priority = 1;
else if (expr instanceof PyPrefixExpression) {
final IElementType opType = getOperationType(expr);
if (opType == PLUS || opType == MINUS || opType == TILDE) priority = 2;
if (opType == NOT_KEYWORD) priority = 10;
if (opType == NOT_KEYWORD) priority = 11;
}
if (expr instanceof PyBinaryExpression) {
else if (expr instanceof PyBinaryExpression) {
final IElementType opType = getOperationType(expr);
if (opType == EXP) priority = 3;
if (opType == MULT || opType == DIV || opType == PERC) priority = 4;
if (opType == MULT || opType == DIV || opType == PERC || opType == FLOORDIV) priority = 4;
if (opType == PLUS || opType == MINUS) priority = 5;
if (opType == LTLT || opType == GTGT) priority = 6;
if (opType == AND) priority = 7;
if (opType == OR) priority = 8;
if (COMPARISON_OPERATIONS.contains(opType)) priority = 9;
if (opType == AND_KEYWORD) priority = 11;
if (opType == XOR) priority = 8;
if (opType == OR) priority = 9;
if (COMPARISON_OPERATIONS.contains(opType)) priority = 10;
if (opType == AND_KEYWORD) priority = 12;
if (opType == OR_KEYWORD) priority = 13;
}
if (expr instanceof PyLambdaExpression) priority = 12;
else if (expr instanceof PyConditionalExpression) priority = 14;
else if (expr instanceof PyLambdaExpression) priority = 15;
return -priority;
}
@@ -0,0 +1,57 @@
(10 + 2)[::-5]
(10 + 2)[5]
(10 + 2)(5)
(10 + 2).foo
-(10 + 2)
+(10 + 2)
~(10 + 2)
5 ** (10 + 2)
(10 + 2) ** 5
5 * (10 + 2)
(10 + 2) * 5
5 / (10 + 2)
(10 + 2) / 5
5 // (10 + 2)
(10 + 2) // 5
5 + 10 + 2
10 + 2 + 5
10 + 2 - 5
5 - (10 + 2)
5 >> 10 + 2
10 + 2 << 5
5 & 10 + 2
10 + 2 & 5
5 ^ 10 + 2
10 + 2 ^ 5
5 | 10 + 2
10 + 2 | 5
() in 10 + 2
10 + 2 in ()
5 is 10 + 2
10 + 2 is 5
5 < 10 + 2
10 + 2 < 5
not 10 + 2
5 and 10 + 2
10 + 2 and 5
5 or 10 + 2
10 + 2 or 5
10 + 2 if 10 + 2 else 10 + 2
@@ -0,0 +1,57 @@
(10 & 2)[::-5]
(10 & 2)[5]
(10 & 2)(5)
(10 & 2).foo
-(10 & 2)
+(10 & 2)
~(10 & 2)
5 ** (10 & 2)
(10 & 2) ** 5
5 * (10 & 2)
(10 & 2) * 5
5 / (10 & 2)
(10 & 2) / 5
5 // (10 & 2)
(10 & 2) // 5
5 + (10 & 2)
(10 & 2) + 5
(10 & 2) - 5
5 - (10 & 2)
5 >> (10 & 2)
(10 & 2) << 5
5 & 10 & 2
10 & 2 & 5
5 ^ 10 & 2
10 & 2 ^ 5
5 | 10 & 2
10 & 2 | 5
() in 10 & 2
10 & 2 in ()
5 is 10 & 2
10 & 2 is 5
5 < 10 & 2
10 & 2 < 5
not 10 & 2
5 and 10 & 2
10 & 2 and 5
5 or 10 & 2
10 & 2 or 5
10 & 2 if 10 & 2 else 10 & 2
@@ -0,0 +1,57 @@
(10 | 2)[::-5]
(10 | 2)[5]
(10 | 2)(5)
(10 | 2).foo
-(10 | 2)
+(10 | 2)
~(10 | 2)
5 ** (10 | 2)
(10 | 2) ** 5
5 * (10 | 2)
(10 | 2) * 5
5 / (10 | 2)
(10 | 2) / 5
5 // (10 | 2)
(10 | 2) // 5
5 + (10 | 2)
(10 | 2) + 5
(10 | 2) - 5
5 - (10 | 2)
5 >> (10 | 2)
(10 | 2) << 5
5 & (10 | 2)
(10 | 2) & 5
5 ^ (10 | 2)
(10 | 2) ^ 5
5 | 10 | 2
10 | 2 | 5
() in 10 | 2
10 | 2 in ()
5 is 10 | 2
10 | 2 is 5
5 < 10 | 2
10 | 2 < 5
not 10 | 2
5 and 10 | 2
10 | 2 and 5
5 or 10 | 2
10 | 2 or 5
10 | 2 if 10 | 2 else 10 | 2
@@ -0,0 +1,57 @@
(10 << 2)[::-5]
(10 << 2)[5]
(10 << 2)(5)
(10 << 2).foo
-(10 << 2)
+(10 << 2)
~(10 << 2)
5 ** (10 << 2)
(10 << 2) ** 5
5 * (10 << 2)
(10 << 2) * 5
5 / (10 << 2)
(10 << 2) / 5
5 // (10 << 2)
(10 << 2) // 5
5 + (10 << 2)
(10 << 2) + 5
(10 << 2) - 5
5 - (10 << 2)
5 >> 10 << 2
10 << 2 << 5
5 & 10 << 2
10 << 2 & 5
5 ^ 10 << 2
10 << 2 ^ 5
5 | 10 << 2
10 << 2 | 5
() in 10 << 2
10 << 2 in ()
5 is 10 << 2
10 << 2 is 5
5 < 10 << 2
10 << 2 < 5
not 10 << 2
5 and 10 << 2
10 << 2 and 5
5 or 10 << 2
10 << 2 or 5
10 << 2 if 10 << 2 else 10 << 2
@@ -0,0 +1,57 @@
(10 ^ 2)[::-5]
(10 ^ 2)[5]
(10 ^ 2)(5)
(10 ^ 2).foo
-(10 ^ 2)
+(10 ^ 2)
~(10 ^ 2)
5 ** (10 ^ 2)
(10 ^ 2) ** 5
5 * (10 ^ 2)
(10 ^ 2) * 5
5 / (10 ^ 2)
(10 ^ 2) / 5
5 // (10 ^ 2)
(10 ^ 2) // 5
5 + (10 ^ 2)
(10 ^ 2) + 5
(10 ^ 2) - 5
5 - (10 ^ 2)
5 >> (10 ^ 2)
(10 ^ 2) << 5
5 & (10 ^ 2)
(10 ^ 2) & 5
5 ^ 10 ^ 2
10 ^ 2 ^ 5
5 | 10 ^ 2
10 ^ 2 | 5
() in 10 ^ 2
10 ^ 2 in ()
5 is 10 ^ 2
10 ^ 2 is 5
5 < 10 ^ 2
10 ^ 2 < 5
not 10 ^ 2
5 and 10 ^ 2
10 ^ 2 and 5
5 or 10 ^ 2
10 ^ 2 or 5
10 ^ 2 if 10 ^ 2 else 10 ^ 2
@@ -0,0 +1,57 @@
(10 and 2)[::-5]
(10 and 2)[5]
(10 and 2)(5)
(10 and 2).foo
-(10 and 2)
+(10 and 2)
~(10 and 2)
5 ** (10 and 2)
(10 and 2) ** 5
5 * (10 and 2)
(10 and 2) * 5
5 / (10 and 2)
(10 and 2) / 5
5 // (10 and 2)
(10 and 2) // 5
5 + (10 and 2)
(10 and 2) + 5
(10 and 2) - 5
5 - (10 and 2)
5 >> (10 and 2)
(10 and 2) << 5
5 & (10 and 2)
(10 and 2) & 5
5 ^ (10 and 2)
(10 and 2) ^ 5
5 | (10 and 2)
(10 and 2) | 5
() in (10 and 2)
(10 and 2) in ()
5 is (10 and 2)
(10 and 2) is 5
5 < (10 and 2)
(10 and 2) < 5
not (10 and 2)
5 and 10 and 2
10 and 2 and 5
5 or 10 and 2
10 and 2 or 5
10 and 2 if 10 and 2 else 10 and 2
@@ -0,0 +1,57 @@
(not 10)[::-5]
(not 10)[5]
(not 10)(5)
(not 10).foo
-(not 10)
+(not 10)
~(not 10)
5 ** (not 10)
(not 10) ** 5
5 * (not 10)
(not 10) * 5
5 / (not 10)
(not 10) / 5
5 // (not 10)
(not 10) // 5
5 + (not 10)
(not 10) + 5
(not 10) - 5
5 - (not 10)
5 >> (not 10)
(not 10) << 5
5 & (not 10)
(not 10) & 5
5 ^ (not 10)
(not 10) ^ 5
5 | (not 10)
(not 10) | 5
() in (not 10)
(not 10) in ()
5 is (not 10)
(not 10) is 5
5 < (not 10)
(not 10) < 5
not not 10
5 and not 10
not 10 and 5
5 or not 10
not 10 or 5
not 10 if not 10 else not 10
@@ -0,0 +1,57 @@
(10 or 2)[::-5]
(10 or 2)[5]
(10 or 2)(5)
(10 or 2).foo
-(10 or 2)
+(10 or 2)
~(10 or 2)
5 ** (10 or 2)
(10 or 2) ** 5
5 * (10 or 2)
(10 or 2) * 5
5 / (10 or 2)
(10 or 2) / 5
5 // (10 or 2)
(10 or 2) // 5
5 + (10 or 2)
(10 or 2) + 5
(10 or 2) - 5
5 - (10 or 2)
5 >> (10 or 2)
(10 or 2) << 5
5 & (10 or 2)
(10 or 2) & 5
5 ^ (10 or 2)
(10 or 2) ^ 5
5 | (10 or 2)
(10 or 2) | 5
() in (10 or 2)
(10 or 2) in ()
5 is (10 or 2)
(10 or 2) is 5
5 < (10 or 2)
(10 or 2) < 5
not (10 or 2)
5 and (10 or 2)
(10 or 2) and 5
5 or 10 or 2
10 or 2 or 5
10 or 2 if 10 or 2 else 10 or 2
@@ -0,0 +1,57 @@
(10 < 2)[::-5]
(10 < 2)[5]
(10 < 2)(5)
(10 < 2).foo
-(10 < 2)
+(10 < 2)
~(10 < 2)
5 ** (10 < 2)
(10 < 2) ** 5
5 * (10 < 2)
(10 < 2) * 5
5 / (10 < 2)
(10 < 2) / 5
5 // (10 < 2)
(10 < 2) // 5
5 + (10 < 2)
(10 < 2) + 5
(10 < 2) - 5
5 - (10 < 2)
5 >> (10 < 2)
(10 < 2) << 5
5 & (10 < 2)
(10 < 2) & 5
5 ^ (10 < 2)
(10 < 2) ^ 5
5 | (10 < 2)
(10 < 2) | 5
() in (10 < 2)
10 < 2 in ()
5 is (10 < 2)
10 < 2 is 5
5 < (10 < 2)
10 < 2 < 5
not 10 < 2
5 and 10 < 2
10 < 2 and 5
5 or 10 < 2
10 < 2 or 5
10 < 2 if 10 < 2 else 10 < 2
@@ -0,0 +1,57 @@
(10 if True else 2)[::-5]
(10 if True else 2)[5]
(10 if True else 2)(5)
(10 if True else 2).foo
-(10 if True else 2)
+(10 if True else 2)
~(10 if True else 2)
5 ** (10 if True else 2)
(10 if True else 2) ** 5
5 * (10 if True else 2)
(10 if True else 2) * 5
5 / (10 if True else 2)
(10 if True else 2) / 5
5 // (10 if True else 2)
(10 if True else 2) // 5
5 + (10 if True else 2)
(10 if True else 2) + 5
(10 if True else 2) - 5
5 - (10 if True else 2)
5 >> (10 if True else 2)
(10 if True else 2) << 5
5 & (10 if True else 2)
(10 if True else 2) & 5
5 ^ (10 if True else 2)
(10 if True else 2) ^ 5
5 | (10 if True else 2)
(10 if True else 2) | 5
() in (10 if True else 2)
(10 if True else 2) in ()
5 is (10 if True else 2)
(10 if True else 2) is 5
5 < (10 if True else 2)
(10 if True else 2) < 5
not (10 if True else 2)
5 and (10 if True else 2)
(10 if True else 2) and 5
5 or (10 if True else 2)
(10 if True else 2) or 5
(10 if True else 2) if (10 if True else 2) else (10 if True else 2)
@@ -0,0 +1,57 @@
(10 / 2)[::-5]
(10 / 2)[5]
(10 / 2)(5)
(10 / 2).foo
-(10 / 2)
+(10 / 2)
~(10 / 2)
5 ** (10 / 2)
(10 / 2) ** 5
5 * 10 / 2
10 / 2 * 5
5 / (10 / 2)
10 / 2 / 5
5 // (10 / 2)
10 / 2 // 5
5 + 10 / 2
10 / 2 + 5
10 / 2 - 5
5 - 10 / 2
5 >> 10 / 2
10 / 2 << 5
5 & 10 / 2
10 / 2 & 5
5 ^ 10 / 2
10 / 2 ^ 5
5 | 10 / 2
10 / 2 | 5
() in 10 / 2
10 / 2 in ()
5 is 10 / 2
10 / 2 is 5
5 < 10 / 2
10 / 2 < 5
not 10 / 2
5 and 10 / 2
10 / 2 and 5
5 or 10 / 2
10 / 2 or 5
10 / 2 if 10 / 2 else 10 / 2
@@ -0,0 +1,57 @@
(10 * 2)[::-5]
(10 * 2)[5]
(10 * 2)(5)
(10 * 2).foo
-(10 * 2)
+(10 * 2)
~(10 * 2)
5 ** (10 * 2)
(10 * 2) ** 5
5 * 10 * 2
10 * 2 * 5
5 / (10 * 2)
10 * 2 / 5
5 // (10 * 2)
10 * 2 // 5
5 + 10 * 2
10 * 2 + 5
10 * 2 - 5
5 - 10 * 2
5 >> 10 * 2
10 * 2 << 5
5 & 10 * 2
10 * 2 & 5
5 ^ 10 * 2
10 * 2 ^ 5
5 | 10 * 2
10 * 2 | 5
() in 10 * 2
10 * 2 in ()
5 is 10 * 2
10 * 2 is 5
5 < 10 * 2
10 * 2 < 5
not 10 * 2
5 and 10 * 2
10 * 2 and 5
5 or 10 * 2
10 * 2 or 5
10 * 2 if 10 * 2 else 10 * 2
@@ -0,0 +1,57 @@
(10 ** 2)[::-5]
(10 ** 2)[5]
(10 ** 2)(5)
(10 ** 2).foo
-(10 ** 2)
+(10 ** 2)
~(10 ** 2)
5 ** 10 ** 2
(10 ** 2) ** 5
5 * 10 ** 2
10 ** 2 * 5
5 / 10 ** 2
10 ** 2 / 5
5 // 10 ** 2
10 ** 2 // 5
5 + 10 ** 2
10 ** 2 + 5
10 ** 2 - 5
5 - 10 ** 2
5 >> 10 ** 2
10 ** 2 << 5
5 & 10 ** 2
10 ** 2 & 5
5 ^ 10 ** 2
10 ** 2 ^ 5
5 | 10 ** 2
10 ** 2 | 5
() in 10 ** 2
10 ** 2 in ()
5 is 10 ** 2
10 ** 2 is 5
5 < 10 ** 2
10 ** 2 < 5
not 10 ** 2
5 and 10 ** 2
10 ** 2 and 5
5 or 10 ** 2
10 ** 2 or 5
10 ** 2 if 10 ** 2 else 10 ** 2
@@ -0,0 +1,57 @@
(10 - 2)[::-5]
(10 - 2)[5]
(10 - 2)(5)
(10 - 2).foo
-(10 - 2)
+(10 - 2)
~(10 - 2)
5 ** (10 - 2)
(10 - 2) ** 5
5 * (10 - 2)
(10 - 2) * 5
5 / (10 - 2)
(10 - 2) / 5
5 // (10 - 2)
(10 - 2) // 5
5 + 10 - 2
10 - 2 + 5
10 - 2 - 5
5 - (10 - 2)
5 >> 10 - 2
10 - 2 << 5
5 & 10 - 2
10 - 2 & 5
5 ^ 10 - 2
10 - 2 ^ 5
5 | 10 - 2
10 - 2 | 5
() in 10 - 2
10 - 2 in ()
5 is 10 - 2
10 - 2 is 5
5 < 10 - 2
10 - 2 < 5
not 10 - 2
5 and 10 - 2
10 - 2 and 5
5 or 10 - 2
10 - 2 or 5
10 - 2 if 10 - 2 else 10 - 2
@@ -0,0 +1,57 @@
x[::-5]
x[5]
x(5)
x.foo
-x
+x
~x
5 ** x
x ** 5
5 * x
x * 5
5 / x
x / 5
5 // x
x // 5
5 + x
x + 5
x - 5
5 - x
5 >> x
x << 5
5 & x
x & 5
5 ^ x
x ^ 5
5 | x
x | 5
() in x
x in ()
5 is x
x is 5
5 < x
x < 5
not x
5 and x
x and 5
5 or x
x or 5
x if x else x
@@ -16,6 +16,8 @@
package com.jetbrains.python.refactoring;
import com.intellij.codeInsight.TargetElementUtilBase;
import com.intellij.openapi.command.WriteCommandAction;
import com.intellij.openapi.fileEditor.FileDocumentManager;
import com.intellij.openapi.util.Comparing;
import com.intellij.psi.PsiElement;
import com.intellij.psi.codeStyle.CodeStyleSettings;
@@ -24,6 +26,8 @@ import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
import com.jetbrains.python.PythonLanguage;
import com.jetbrains.python.fixtures.PyTestCase;
import com.jetbrains.python.refactoring.inline.PyInlineLocalHandler;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
* @author Dennis.Ushakov
@@ -33,13 +37,18 @@ public class PyInlineLocalTest extends PyTestCase {
doTest(null);
}
private void doTest(String expectedError) {
private void doTest(@Nullable String expectedError) {
final String name = getTestName(true);
myFixture.configureByFile("/refactoring/inlinelocal/" + name + ".before.py");
if (!performRefactoring(expectedError)) return;
myFixture.checkResultByFile("/refactoring/inlinelocal/" + name + ".after.py");
}
private boolean performRefactoring(@Nullable String expectedError) {
try {
PsiElement element = TargetElementUtilBase.findTargetElement(myFixture.getEditor(),
TargetElementUtilBase.getInstance().getReferenceSearchFlags());
PyInlineLocalHandler handler = PyInlineLocalHandler.getInstance();
final PsiElement element = TargetElementUtilBase.findTargetElement(myFixture.getEditor(),
TargetElementUtilBase.getInstance().getReferenceSearchFlags());
final PyInlineLocalHandler handler = PyInlineLocalHandler.getInstance();
handler.inlineElement(myFixture.getProject(), myFixture.getEditor(), element);
if (expectedError != null) fail("expected error: '" + expectedError + "', got none");
}
@@ -48,9 +57,9 @@ public class PyInlineLocalTest extends PyTestCase {
e.printStackTrace();
}
assertEquals(expectedError, e.getMessage());
return;
return false;
}
myFixture.checkResultByFile("/refactoring/inlinelocal/" + name + ".after.py");
return true;
}
public void testSimple() {
@@ -114,11 +123,33 @@ public class PyInlineLocalTest extends PyTestCase {
}
}
public void testParenthesisInsertedForSubtraction() {
doTest();
public void testOperatorPrecedence() throws Exception {
checkOperatorPrecedence("x = 10 ** 2", "power");
checkOperatorPrecedence("x = 10 * 2", "multiplication");
checkOperatorPrecedence("x = 10 / 2", "division");
checkOperatorPrecedence("x = 10 + 2", "addition");
checkOperatorPrecedence("x = 10 - 2", "subtraction");
checkOperatorPrecedence("x = 10 << 2", "bitwiseShift");
checkOperatorPrecedence("x = 10 & 2", "bitwiseAnd");
checkOperatorPrecedence("x = 10 ^ 2", "bitwiseXor");
checkOperatorPrecedence("x = 10 | 2", "bitwiseOr");
checkOperatorPrecedence("x = 10 < 2", "comparison");
checkOperatorPrecedence("x = not 10", "booleanNot");
checkOperatorPrecedence("x = 10 and 2", "booleanAnd");
checkOperatorPrecedence("x = 10 or 2", "booleanOr");
checkOperatorPrecedence("x = 10 if True else 2", "conditional");
}
public void testParenthesisInsertedForPower() {
doTest();
private void checkOperatorPrecedence(@NotNull final String firstLine, @NotNull String resultPrefix) throws Exception {
myFixture.configureByFile("/refactoring/inlinelocal/operatorPrecedence/template.py");
WriteCommandAction.runWriteCommandAction(myFixture.getProject(), new Runnable() {
@Override
public void run() {
myFixture.getEditor().getDocument().insertString(0, firstLine + "\n");
}
});
performRefactoring(null);
myFixture.checkResultByFile("/refactoring/inlinelocal/operatorPrecedence/" + resultPrefix + ".after.py");
FileDocumentManager.getInstance().reloadFromDisk(myFixture.getDocument(myFixture.getFile()));
}
}