From 1203c4cd05cef512089d2c883192c55324ada876 Mon Sep 17 00:00:00 2001 From: "Anna.Kozlova" Date: Fri, 24 Nov 2017 16:07:33 +0100 Subject: [PATCH] preserve comments: conditional -> if --- .../ConditionalExpressionInspection.java | 53 +++++++++---------- .../conditional_expression/Comment.after.java | 7 ++- .../conditional_expression/Comment.java | 2 +- .../CommentWithDeclaration.after.java | 12 +++++ .../CommentWithDeclaration.java | 8 +++ .../ConditionalExpressionFixTest.java | 1 + 6 files changed, 53 insertions(+), 30 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.java diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/ConditionalExpressionInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/ConditionalExpressionInspection.java index 892da2f3a4a4..4fb99c47a680 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/ConditionalExpressionInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/ConditionalExpressionInspection.java @@ -22,7 +22,6 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; -import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.ArrayUtilRt; @@ -31,10 +30,7 @@ import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; -import com.siyeh.ig.psiutils.ExpectedTypeUtils; -import com.siyeh.ig.psiutils.ExpressionUtils; -import com.siyeh.ig.psiutils.MethodCallUtils; -import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.*; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -123,10 +119,11 @@ public class ConditionalExpressionInspection extends BaseInspection { PsiExpression thenExpression = ParenthesesUtils.stripParentheses(expression.getThenExpression()); PsiExpression elseExpression = ParenthesesUtils.stripParentheses(expression.getElseExpression()); final PsiExpression condition = ParenthesesUtils.stripParentheses(expression.getCondition()); + CommentTracker tracker = new CommentTracker(); final StringBuilder newStatement = new StringBuilder(); newStatement.append("if("); if (condition != null) { - newStatement.append(condition.getText()); + newStatement.append(tracker.markUnchanged(condition).getText()); } newStatement.append(')'); if (variable != null) { @@ -147,15 +144,16 @@ public class ConditionalExpressionInspection extends BaseInspection { elseExpression = expression.getElseExpression(); } } - appendElementTextWithoutParentheses(initializer, expression, thenExpression, newStatement); + appendElementTextWithoutParentheses(initializer, expression, thenExpression, newStatement, tracker); newStatement.append("; else ").append(name).append('='); - appendElementTextWithoutParentheses(initializer, expression, elseExpression, newStatement); + appendElementTextWithoutParentheses(initializer, expression, elseExpression, newStatement, tracker); newStatement.append(';'); - initializer.delete(); + tracker.delete(initializer); final PsiManager manager = statement.getManager(); final PsiStatement ifStatement = JavaPsiFacade.getElementFactory(project).createStatementFromText(newStatement.toString(), statement); final PsiElement parent = statement.getParent(); final PsiElement addedElement = parent.addAfter(ifStatement, statement); + tracker.insertCommentsBefore(addedElement); final CodeStyleManager styleManager = CodeStyleManager.getInstance(manager.getProject()); styleManager.reformat(addedElement); } @@ -164,7 +162,7 @@ public class ConditionalExpressionInspection extends BaseInspection { if (addBraces || thenExpression == null) { newStatement.append('{'); } - appendElementTextWithoutParentheses(statement, expression, thenExpression, newStatement); + appendElementTextWithoutParentheses(statement, expression, thenExpression, newStatement, tracker); if (addBraces) { newStatement.append("} else {"); } @@ -177,32 +175,39 @@ public class ConditionalExpressionInspection extends BaseInspection { newStatement.append('{'); } } - appendElementTextWithoutParentheses(statement, expression, elseExpression, newStatement); + appendElementTextWithoutParentheses(statement, expression, elseExpression, newStatement, tracker); if (addBraces || elseExpression == null) { newStatement.append('}'); } - PsiReplacementUtil.replaceStatement(statement, newStatement.toString()); + PsiReplacementUtil.replaceStatement(statement, newStatement.toString(), tracker); } } - private static void appendElementTextWithoutParentheses(@NotNull PsiElement element, @NotNull PsiExpression expressionToReplace, - @Nullable PsiExpression replacementExpression, @NotNull StringBuilder out) { + private static void appendElementTextWithoutParentheses(@NotNull PsiElement element, + @NotNull PsiExpression expressionToReplace, + @Nullable PsiExpression replacementExpression, + @NotNull StringBuilder out, + CommentTracker tracker) { final PsiElement expressionParent = expressionToReplace.getParent(); if (expressionParent instanceof PsiParenthesizedExpression) { final PsiElement grandParent = expressionParent.getParent(); if (replacementExpression == null || !(grandParent instanceof PsiExpression) || !ParenthesesUtils.areParenthesesNeeded(replacementExpression, (PsiExpression) grandParent, false)) { - appendElementTextWithoutParentheses(element, (PsiExpression)expressionParent, replacementExpression, out); + appendElementTextWithoutParentheses(element, (PsiExpression)expressionParent, replacementExpression, out, tracker); return; } } final boolean needsCast = replacementExpression != null && MethodCallUtils.isNecessaryForSurroundingMethodCall(expressionToReplace, replacementExpression); - appendElementText(element, expressionToReplace, replacementExpression, needsCast, out); + appendElementText(element, expressionToReplace, replacementExpression, needsCast, out, tracker); } - private static void appendElementText(@NotNull PsiElement element, @NotNull PsiExpression elementToReplace, - @Nullable PsiExpression replacementExpression, boolean insertCast, @NotNull StringBuilder out) { + private static void appendElementText(@NotNull PsiElement element, + @NotNull PsiExpression elementToReplace, + @Nullable PsiExpression replacementExpression, + boolean insertCast, + @NotNull StringBuilder out, + CommentTracker tracker) { if (element.equals(elementToReplace)) { final String replacementText = (replacementExpression == null) ? "" : replacementExpression.getText(); final PsiType type = GenericsUtil.getVariableTypeByExpressionType(ExpectedTypeUtils.findExpectedType(elementToReplace, true)); @@ -214,18 +219,12 @@ public class ConditionalExpressionInspection extends BaseInspection { } final PsiElement[] children = element.getChildren(); if (children.length == 0) { - out.append(element.getText()); - if (element instanceof PsiComment) { - final PsiComment comment = (PsiComment)element; - final IElementType tokenType = comment.getTokenType(); - if (tokenType == JavaTokenType.END_OF_LINE_COMMENT) { - out.append('\n'); - } + if (!(element instanceof PsiComment)) { + out.append(tracker.markUnchanged(element).getText()); } - return; } for (PsiElement child : children) { - appendElementText(child, elementToReplace, replacementExpression, insertCast, out); + appendElementText(child, elementToReplace, replacementExpression, insertCast, out, tracker); } } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.after.java index 06fc5a45733d..67210578cab8 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.after.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.after.java @@ -2,7 +2,10 @@ package com.siyeh.ipp.conditional.withIf; class Comment { public String get() { - if (239 > 42) return "239";//comment - else return "42";//comment + /*before then*/ + /*after then*/ + //comment + if (239 > 42) return "239"; + else return "42"; } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.java index ed0cdd536aeb..0721f3431f34 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/Comment.java @@ -2,6 +2,6 @@ package com.siyeh.ipp.conditional.withIf; class Comment { public String get() { - return 239 > 42 ? "239" : "42";//comment + return 239 > 42 ? /*before then*/"239"/*after then*/ : "42";//comment } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.after.java new file mode 100644 index 000000000000..d083433125eb --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.after.java @@ -0,0 +1,12 @@ +package com.siyeh.ipp.conditional.withIf; + +class Comment { + public String get() { + String s;//comment + /*before then*/ + /*after then*/ + if (239 > 42) s = "239"; + else s = "42"; + return s; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.java new file mode 100644 index 000000000000..3b8497233572 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/controlflow/conditional_expression/CommentWithDeclaration.java @@ -0,0 +1,8 @@ +package com.siyeh.ipp.conditional.withIf; + +class Comment { + public String get() { + String s = 239 > 42 ? /*before then*/"239"/*after then*/ : "42";//comment + return s; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/ConditionalExpressionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/ConditionalExpressionFixTest.java index 7c07bef223e6..2524793ba3f3 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/ConditionalExpressionFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/ConditionalExpressionFixTest.java @@ -36,6 +36,7 @@ public class ConditionalExpressionFixTest extends IGQuickFixesTestCase { public void testArrayInitializer() { doTest(); } public void testCastNeeded() { doTest(); } public void testComment() { doTest(); } + public void testCommentWithDeclaration() { doTest(); } public void testConditionalAsArgument() { doTest(); } public void testConditionalInBinaryExpression() { doTest(); } public void testConditionalInIf() { doTest(); }