From fc30a55f54a99dc19cb967ad11660541b5ec3327 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Thu, 12 Oct 2017 18:45:32 +0300 Subject: [PATCH] PY-24160 Do not give indent to a binary expression in parentheses Use ContinuationIndent instead of ContinuationIndentWithoutFirst for its operands and operators instead. This way we don't over-indent a condition in parentheses when the opening bracket is on its own line. Also test that binary expressions in other positions are not over-indented thanks to the use of ContinuationWithoutFirst for them. --- .../jetbrains/python/formatter/PyBlock.java | 46 ++++++++++--------- ...ultilineBinaryExpressionInsideGenerator.py | 8 ++++ ...neBinaryExpressionInsideGenerator_after.py | 8 ++++ ...neIfConditionInParenthesesHangingIndent.py | 13 ++++++ ...nditionInParenthesesHangingIndent_after.py | 13 ++++++ .../notParenthesisedBinaryExpressions.py | 20 ++++++++ ...notParenthesisedBinaryExpressions_after.py | 20 ++++++++ .../com/jetbrains/python/PyFormatterTest.java | 13 ++++++ 8 files changed, 120 insertions(+), 21 deletions(-) create mode 100644 python/testData/formatter/multilineBinaryExpressionInsideGenerator.py create mode 100644 python/testData/formatter/multilineBinaryExpressionInsideGenerator_after.py create mode 100644 python/testData/formatter/multilineIfConditionInParenthesesHangingIndent.py create mode 100644 python/testData/formatter/multilineIfConditionInParenthesesHangingIndent_after.py create mode 100644 python/testData/formatter/notParenthesisedBinaryExpressions.py create mode 100644 python/testData/formatter/notParenthesisedBinaryExpressions_after.py diff --git a/python/src/com/jetbrains/python/formatter/PyBlock.java b/python/src/com/jetbrains/python/formatter/PyBlock.java index b974e2cbcfe4..b4041ff5f577 100644 --- a/python/src/com/jetbrains/python/formatter/PyBlock.java +++ b/python/src/com/jetbrains/python/formatter/PyBlock.java @@ -248,20 +248,24 @@ public class PyBlock implements ASTBlock { if (parentType == PyElementTypes.BINARY_EXPRESSION) { if (childType != PyElementTypes.BINARY_EXPRESSION) { final PyBlock topmostBinary = findTopmostBinaryExpressionBlock(child); - final PyBlock comprehensionBlock = topmostBinary != null ? topmostBinary.myParent : this; - if (comprehensionBlock != null - // TODO check for comprehensions explicitly here - && ourListElementTypes.contains(comprehensionBlock.myNode.getElementType()) - && needListAlignment(child) - && !myEmptySequence) { - childAlignment = comprehensionBlock.getChildAlignment(); + assert topmostBinary != null; + final PyBlock binaryParentBlock = topmostBinary.myParent; + final ASTNode binaryParentNode = binaryParentBlock.myNode; + final IElementType binaryParentType = binaryParentNode.getElementType(); + // TODO check for comprehensions explicitly here + if (ourListElementTypes.contains(binaryParentType) && needListAlignment(child) && !myEmptySequence) { + childAlignment = binaryParentBlock.getChildAlignment(); } + final boolean parenthesised = binaryParentType == PyElementTypes.PARENTHESIZED_EXPRESSION; if (childAlignment == null && topmostBinary != null && - !isParenthesisedIfCondition(topmostBinary.myNode) && + !(parenthesised && isIfCondition(binaryParentNode)) && !(isCondition(topmostBinary.myNode) && !ALIGN_CONDITIONS_WITHOUT_PARENTHESES)) { childAlignment = topmostBinary.getAlignmentForChildren(); } - childIndent = Indent.getContinuationWithoutFirstIndent(); + // We omit indentation for the binary expression itself in this case (similarly to PyTupleExpression inside + // PyParenthesisedExpression) because we indent individual operands and operators inside rather than + // the whole contained expression. + childIndent = parenthesised ? Indent.getContinuationIndent() : Indent.getContinuationWithoutFirstIndent(); } } else if (parentType == PyElementTypes.LIST_LITERAL_EXPRESSION || parentType == PyElementTypes.LIST_COMP_EXPRESSION) { @@ -342,6 +346,10 @@ public class PyBlock implements ASTBlock { !hasLineBreaksBeforeInSameParent(child, 1)) { childIndent = Indent.getNoneIndent(); } + // Operands and operators of a binary expression have their own indent, no need to increase it by indenting the expression itself + else if (childType == PyElementTypes.BINARY_EXPRESSION && parentType == PyElementTypes.PARENTHESIZED_EXPRESSION) { + childIndent = Indent.getNoneIndent(); + } else { childIndent = isIndentNext(child) ? Indent.getContinuationIndent() : Indent.getNormalIndent(); } @@ -392,7 +400,9 @@ public class PyBlock implements ASTBlock { childIndent = Indent.getNormalIndent(); } - if (isAfterStatementList(child) && !hasLineBreaksBeforeInSameParent(child, 2) && child.getElementType() != PyTokenTypes.END_OF_LINE_COMMENT) { + if (isAfterStatementList(child) && + !hasLineBreaksBeforeInSameParent(child, 2) && + child.getElementType() != PyTokenTypes.END_OF_LINE_COMMENT) { // maybe enter was pressed and cut us from a previous (nested) statement list childIndent = Indent.getNormalIndent(); } @@ -426,14 +436,10 @@ public class PyBlock implements ASTBlock { return createBlock(this, child, childAlignment, childIndent, childWrap, myContext); } - private static boolean isParenthesisedIfCondition(@NotNull ASTNode node) { - final PyParenthesizedExpression parens = as(node.getPsi().getParent(), PyParenthesizedExpression.class); - return parens != null && isIfCondition(parens); - } - - private static boolean isIfCondition(@NotNull PyExpression expr) { - final PyIfPart ifPart = as(expr.getParent(), PyIfPart.class); - return ifPart != null && ifPart.getCondition() == expr && !ifPart.isElif(); + private static boolean isIfCondition(@NotNull ASTNode node) { + @NotNull PsiElement element = node.getPsi(); + final PyIfPart ifPart = as(element.getParent(), PyIfPart.class); + return ifPart != null && ifPart.getCondition() == element && !ifPart.isElif(); } private static boolean isCondition(@NotNull ASTNode node) { @@ -854,7 +860,6 @@ public class PyBlock implements ASTBlock { return getBlankLinesForOption(pySettings.BLANK_LINES_AROUND_TOP_LEVEL_CLASSES_FUNCTIONS); } } - } return myContext.getSpacingBuilder().getSpacing(this, child1, child2); } @@ -935,8 +940,7 @@ public class PyBlock implements ASTBlock { // delegation sometimes causes NPEs in formatter core, so we calculate the // correct indent manually. if (statementListsBelow > 0) { // was 1... strange - @SuppressWarnings("ConstantConditions") - final int indent = myContext.getSettings().getIndentOptions().INDENT_SIZE; + @SuppressWarnings("ConstantConditions") final int indent = myContext.getSettings().getIndentOptions().INDENT_SIZE; return new ChildAttributes(Indent.getSpaceIndent(indent * statementListsBelow), null); } diff --git a/python/testData/formatter/multilineBinaryExpressionInsideGenerator.py b/python/testData/formatter/multilineBinaryExpressionInsideGenerator.py new file mode 100644 index 000000000000..aa2d72d2cd3c --- /dev/null +++ b/python/testData/formatter/multilineBinaryExpressionInsideGenerator.py @@ -0,0 +1,8 @@ +xs = ( +x and +x + for x in range(10) + if +x and +x +) \ No newline at end of file diff --git a/python/testData/formatter/multilineBinaryExpressionInsideGenerator_after.py b/python/testData/formatter/multilineBinaryExpressionInsideGenerator_after.py new file mode 100644 index 000000000000..981c9b651877 --- /dev/null +++ b/python/testData/formatter/multilineBinaryExpressionInsideGenerator_after.py @@ -0,0 +1,8 @@ +xs = ( + x and + x + for x in range(10) + if + x and + x +) diff --git a/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent.py b/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent.py new file mode 100644 index 000000000000..a09103cdfa5b --- /dev/null +++ b/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent.py @@ -0,0 +1,13 @@ +if ( + 1 < 2 + and 3 == 3 + and 4 != 6 +): + print('True') + +if ( + 1 < 2 and + 3 == 3 and + 4 != 6 +): + print('True') \ No newline at end of file diff --git a/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent_after.py b/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent_after.py new file mode 100644 index 000000000000..635aca043aae --- /dev/null +++ b/python/testData/formatter/multilineIfConditionInParenthesesHangingIndent_after.py @@ -0,0 +1,13 @@ +if ( + 1 < 2 + and 3 == 3 + and 4 != 6 +): + print('True') + +if ( + 1 < 2 and + 3 == 3 and + 4 != 6 +): + print('True') diff --git a/python/testData/formatter/notParenthesisedBinaryExpressions.py b/python/testData/formatter/notParenthesisedBinaryExpressions.py new file mode 100644 index 000000000000..cef220e91145 --- /dev/null +++ b/python/testData/formatter/notParenthesisedBinaryExpressions.py @@ -0,0 +1,20 @@ +func( + x or y +) + +_list = [ + x or y +] + +_tuple = ( + x or y, +) + +_set = { + x or y +} + +_dict = { + 'foo': + x or y +} diff --git a/python/testData/formatter/notParenthesisedBinaryExpressions_after.py b/python/testData/formatter/notParenthesisedBinaryExpressions_after.py new file mode 100644 index 000000000000..cef220e91145 --- /dev/null +++ b/python/testData/formatter/notParenthesisedBinaryExpressions_after.py @@ -0,0 +1,20 @@ +func( + x or y +) + +_list = [ + x or y +] + +_tuple = ( + x or y, +) + +_set = { + x or y +} + +_dict = { + 'foo': + x or y +} diff --git a/python/testSrc/com/jetbrains/python/PyFormatterTest.java b/python/testSrc/com/jetbrains/python/PyFormatterTest.java index cd68175d0b41..afcba111fb22 100644 --- a/python/testSrc/com/jetbrains/python/PyFormatterTest.java +++ b/python/testSrc/com/jetbrains/python/PyFormatterTest.java @@ -907,6 +907,19 @@ public class PyFormatterTest extends PyTestCase { doTest(); } + // PY-24160 + public void testMultilineIfConditionInParenthesesHangingIndent() { + doTest(); + } + + public void testMultilineBinaryExpressionInsideGenerator() { + doTest(); + } + + public void testNotParenthesisedBinaryExpressions() { + doTest(); + } + public void testVariableAnnotations() { runWithLanguageLevel(LanguageLevel.PYTHON36, this::doTest); }