From 7730b97ee44d4ba6ce18058ae9d48a49b48d7dc7 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 12 Jul 2018 11:26:49 +0700 Subject: [PATCH] DeclarationJoinLinesHandler: respect precedence for both initializer and right-hand operand Fixes IDEA-195416 Join lines over arithmetic operations should preserve order Also StreamApiMigrationInspection#getOperationSign moved to TypeConversionUtil#getBinaryOperationText and reused --- .../DeclarationJoinLinesHandler.java | 79 +++++-------------- .../StreamApiMigrationInspection.java | 41 +--------- .../intellij/psi/util/TypeConversionUtil.java | 39 +++++++++ .../AssignmentExpressionPrecedence.java | 6 ++ .../AssignmentExpressionPrecedence_after.java | 5 ++ .../java/codeInsight/JoinLinesTest.java | 1 + 6 files changed, 71 insertions(+), 100 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence.java create mode 100644 java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence_after.java diff --git a/java/java-impl/src/com/intellij/codeInsight/editorActions/DeclarationJoinLinesHandler.java b/java/java-impl/src/com/intellij/codeInsight/editorActions/DeclarationJoinLinesHandler.java index d07969e4ce25..b377c036f43b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/editorActions/DeclarationJoinLinesHandler.java +++ b/java/java-impl/src/com/intellij/codeInsight/editorActions/DeclarationJoinLinesHandler.java @@ -24,6 +24,7 @@ import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiUtil; +import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.ParenthesesUtils; @@ -81,7 +82,7 @@ public class DeclarationJoinLinesHandler implements JoinLinesHandlerDelegate { try { PsiDeclarationStatement newDecl = factory.createVariableDeclarationStatement(var.getName(), var.getType(), initializerExpression); PsiVariable newVar = (PsiVariable)newDecl.getDeclaredElements()[0]; - if (var.getModifierList().getText().length() > 0) { + if (!var.getModifierList().getText().isEmpty()) { PsiUtil.setModifierProperty(newVar, PsiModifier.FINAL, true); } newVar.getModifierList().replace(var.getModifierList()); @@ -121,73 +122,31 @@ public class DeclarationJoinLinesHandler implements JoinLinesHandlerDelegate { } @Nullable - public static PsiExpression getInitializerExpression(PsiExpression initializer, - PsiAssignmentExpression assignment) { + public static PsiExpression getInitializerExpression(PsiExpression initializer, PsiAssignmentExpression assignment) { PsiExpression initializerExpression; - final IElementType originalOpSign = assignment.getOperationTokenType(); + final IElementType compoundOp = assignment.getOperationTokenType(); final PsiExpression rExpression = assignment.getRExpression(); if (rExpression == null) return null; - if (originalOpSign == JavaTokenType.EQ) { + if (compoundOp == JavaTokenType.EQ) { initializerExpression = rExpression; } else { if (initializer == null) return null; - String opSign = null; - if (originalOpSign == JavaTokenType.ANDEQ) { - opSign = "&"; - } - else if (originalOpSign == JavaTokenType.ASTERISKEQ) { - opSign = "*"; - } - else if (originalOpSign == JavaTokenType.DIVEQ) { - opSign = "/"; - } - else if (originalOpSign == JavaTokenType.GTGTEQ) { - opSign = ">>"; - } - else if (originalOpSign == JavaTokenType.GTGTGTEQ) { - opSign = ">>>"; - } - else if (originalOpSign == JavaTokenType.LTLTEQ) { - opSign = "<<"; - } - else if (originalOpSign == JavaTokenType.MINUSEQ) { - opSign = "-"; - } - else if (originalOpSign == JavaTokenType.OREQ) { - opSign = "|"; - } - else if (originalOpSign == JavaTokenType.PERCEQ) { - opSign = "%"; - } - else if (originalOpSign == JavaTokenType.PLUSEQ) { - opSign = "+"; - } - else if (originalOpSign == JavaTokenType.XOREQ) { - opSign = "^"; - } - - try { - final Project project = assignment.getProject(); - String initializerText = initializer.getText() + opSign; - final String rightText = rExpression.getText(); - if (ParenthesesUtils.areParenthesesNeeded(assignment.getOperationSign(), rExpression)) { - initializerText += "(" + rightText + ")"; - } - else { - initializerText += rightText; - } - if ("+".equals(opSign) && ExpressionUtils.isZero(initializer) || - "*".equals(opSign) && ExpressionUtils.isOne(initializer)) { - initializerText = rightText; - } - initializerExpression = JavaPsiFacade.getElementFactory(project).createExpressionFromText(initializerText, assignment); - initializerExpression = (PsiExpression)CodeStyleManager.getInstance(project).reformat(initializerExpression); - } - catch (IncorrectOperationException e) { - LOG.error(e); - return null; + IElementType simpleOp = TypeConversionUtil.convertEQtoOperation(compoundOp); + if (simpleOp == null) return null; + String opSign = TypeConversionUtil.getBinaryOperationText(simpleOp); + if (opSign == null) return null; + final Project project = assignment.getProject(); + final String rightText = rExpression.getText(); + int precedence = ParenthesesUtils.getPrecedenceForOperator(simpleOp) + 1; + String initializerText = + ParenthesesUtils.getText(initializer, precedence) + opSign + ParenthesesUtils.getText(rExpression, precedence); + if ("+".equals(opSign) && ExpressionUtils.isZero(initializer) || + "*".equals(opSign) && ExpressionUtils.isOne(initializer)) { + initializerText = rightText; } + initializerExpression = JavaPsiFacade.getElementFactory(project).createExpressionFromText(initializerText, assignment); + initializerExpression = (PsiExpression)CodeStyleManager.getInstance(project).reformat(initializerExpression); } return initializerExpression; } diff --git a/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java b/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java index fa5eb42d659c..125ac44a421d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java @@ -1234,45 +1234,6 @@ public class StreamApiMigrationInspection extends AbstractBaseJavaLocalInspectio myUnaryExpression = unaryExpression; } - @Nullable - @Contract(pure = true) - private static String getOperationSign(IElementType op) { - if (op == JavaTokenType.AND) { - return "&"; - } - else if (op == JavaTokenType.ASTERISK) { - return "*"; - } - else if (op == JavaTokenType.DIV) { - return "/"; - } - else if (op == JavaTokenType.GTGT) { - return ">>"; - } - else if (op == JavaTokenType.GTGTGT) { - return ">>>"; - } - else if (op == JavaTokenType.LTLT) { - return "<<"; - } - else if (op == JavaTokenType.MINUS) { - return "-"; - } - else if (op == JavaTokenType.OR) { - return "|"; - } - else if (op == JavaTokenType.PERC) { - return "%"; - } - else if (op == JavaTokenType.PLUS) { - return "+"; - } - else if (op == JavaTokenType.XOR) { - return "^"; - } - return null; - } - @Override String createReplacement(CommentTracker ct) { String lambda; @@ -1280,7 +1241,7 @@ public class StreamApiMigrationInspection extends AbstractBaseJavaLocalInspectio PsiElementFactory factory = JavaPsiFacade.getElementFactory(myVariable.getProject()); PsiExpression expression = myUnaryExpression == null ? myExpression : factory.createExpressionFromText("1", null); String expressionText = ParenthesesUtils.getText(ct.markUnchanged(expression), ParenthesesUtils.getPrecedenceForOperator(myOpType)); - String lambdaBody = myVariable.getName() + getOperationSign(myOpType) + expressionText; + String lambdaBody = myVariable.getName() + TypeConversionUtil.getBinaryOperationText(myOpType) + expressionText; if (!myVariable.getType().equals(expression.getType())) { lambdaBody = ("(" + myVariable.getType().getCanonicalText() + ")") + "(" + lambdaBody + ")"; } diff --git a/java/java-psi-api/src/com/intellij/psi/util/TypeConversionUtil.java b/java/java-psi-api/src/com/intellij/psi/util/TypeConversionUtil.java index bc355ec54bd4..ae7c0a179027 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/TypeConversionUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/TypeConversionUtil.java @@ -1333,6 +1333,45 @@ public class TypeConversionUtil { return PsiType.INT; } + @Nullable + @Contract(pure = true) + public static String getBinaryOperationText(IElementType op) { + if (op == JavaTokenType.AND) { + return "&"; + } + else if (op == JavaTokenType.ASTERISK) { + return "*"; + } + else if (op == JavaTokenType.DIV) { + return "/"; + } + else if (op == JavaTokenType.GTGT) { + return ">>"; + } + else if (op == JavaTokenType.GTGTGT) { + return ">>>"; + } + else if (op == JavaTokenType.LTLT) { + return "<<"; + } + else if (op == JavaTokenType.MINUS) { + return "-"; + } + else if (op == JavaTokenType.OR) { + return "|"; + } + else if (op == JavaTokenType.PERC) { + return "%"; + } + else if (op == JavaTokenType.PLUS) { + return "+"; + } + else if (op == JavaTokenType.XOR) { + return "^"; + } + return null; + } + public static IElementType convertEQtoOperation(IElementType eqOpSign) { IElementType opSign = null; if (eqOpSign == JavaTokenType.ANDEQ) { diff --git a/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence.java b/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence.java new file mode 100644 index 000000000000..091780cd8b33 --- /dev/null +++ b/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence.java @@ -0,0 +1,6 @@ +class Foo { + { + int a = 1 + 2; + a *= 2 + 3; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence_after.java b/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence_after.java new file mode 100644 index 000000000000..a5a135229ae9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/joinLines/AssignmentExpressionPrecedence_after.java @@ -0,0 +1,5 @@ +class Foo { + { + int a = (1 + 2) * (2 + 3); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/JoinLinesTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/JoinLinesTest.java index 3eb85d266ee5..685b8d3c861a 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/JoinLinesTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/JoinLinesTest.java @@ -187,6 +187,7 @@ public class JoinLinesTest extends LightCodeInsightTestCase { public void testAssignmentExpression() { doTest(); } public void testAssignmentExpression2() { doTest(); } + public void testAssignmentExpressionPrecedence() { doTest(); } public void testReformatInsertsNewlines() { CommonCodeStyleSettings settings = getJavaSettings();