From 355973cb563362512bdbcaa92bea1ea8e7bb02fd Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 30 Sep 2016 16:31:06 +0700 Subject: [PATCH] IDEA-CR-13980: comments handling is simplified (they just extracted and placed before the condition) --- .../OptionalIsPresentInspection.java | 47 +++++++------------ .../optionalIsPresent/afterAssignmentMap.java | 8 ++-- .../afterReturnComments.java | 4 +- .../beforeAssignmentMap.java | 3 +- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java index 20f4b79bea85..5f9e509785f0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java @@ -38,8 +38,6 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Collection; - /** * @author Tagir Valeev */ @@ -154,27 +152,16 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo }); } - static String getComments(PsiStatement statement) { - Collection comments = PsiTreeUtil.collectElementsOfType(statement, PsiComment.class); - return StreamEx.of(comments) - .filter(c -> c.getParent() == statement || - (statement instanceof PsiExpressionStatement && c.getParent() == ((PsiExpressionStatement)statement).getExpression())) - .flatMap(c -> StreamEx.of(c.getPrevSibling(), c, c.getNextSibling())) // add both siblings for every comment - .filter(e -> e instanceof PsiComment || e instanceof PsiWhiteSpace) // select only comments and whitespace - .distinct() - .map(PsiElement::getText).joining(""); - } - @NotNull static String generateMapIfNeeded(PsiElementFactory factory, PsiVariable optionalVariable, - PsiStatement trueStatement, PsiExpression trueValue) { - String trueComments = getComments(trueStatement); + String name = optionalVariable.getName(); + LOG.assertTrue(name != null); if(isOptionalGetCall(trueValue, optionalVariable)) { - return trueComments + optionalVariable.getName(); + return name; } else { - return optionalVariable.getName() + ".map(" + trueComments + generateOptionalLambda(factory, optionalVariable, trueValue) + ")"; + return name + ".map(" + generateOptionalLambda(factory, optionalVariable, trueValue) + ")"; } } @@ -235,15 +222,15 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo if (!myScenario.isApplicable(optionalVariable, thenStatement, elseStatement)) return; if (!FileModificationService.getInstance().preparePsiElementForWrite(element.getContainingFile())) return; PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - String replacement = myScenario.generateReplacement(factory, optionalVariable, thenStatement, elseStatement); - final PsiElement parent = statement.getParent(); - for (PsiElement comment : PsiTreeUtil.findChildrenOfType(statement, PsiComment.class)) { - // Comments inside then/else statements should be handled by scenario - if((thenStatement == null || !PsiTreeUtil.isAncestor(thenStatement, comment, true)) && - (elseStatement == null || !PsiTreeUtil.isAncestor(elseStatement, comment, true))) { + PsiElement parent = statement.getParent(); + StreamEx.of(statement, thenStatement, elseStatement).nonNull() + .flatCollection(st -> PsiTreeUtil.findChildrenOfType(st, PsiComment.class)) + .distinct() + .forEach(comment -> { parent.addBefore(comment, statement); - } - } + comment.delete(); + }); + String replacement = myScenario.generateReplacement(factory, optionalVariable, thenStatement, elseStatement); if(thenStatement != null && !PsiTreeUtil.isAncestor(statement, thenStatement, true)) thenStatement.delete(); if(elseStatement != null && !PsiTreeUtil.isAncestor(statement, elseStatement, true)) elseStatement.delete(); PsiElement result = statement.replace(factory.createStatementFromText(replacement, statement)); @@ -285,8 +272,8 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo PsiExpression falseValue = ((PsiReturnStatement)falseStatement).getReturnValue(); LOG.assertTrue(trueValue != null); LOG.assertTrue(falseValue != null); - String trueBranch = generateMapIfNeeded(factory, optionalVariable, trueStatement, trueValue); - return "return " + trueBranch + ".orElse(" + getComments(falseStatement) + falseValue.getText() + ");"; + String trueBranch = generateMapIfNeeded(factory, optionalVariable, trueValue); + return "return " + trueBranch + ".orElse(" + falseValue.getText() + ");"; } } @@ -326,12 +313,12 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo PsiExpression trueValue = trueAssignment.getRExpression(); PsiExpression falseValue = falseAssignment.getRExpression(); LOG.assertTrue(falseValue != null); - String trueBranch = generateMapIfNeeded(factory, optionalVariable, trueStatement, trueValue); + String trueBranch = generateMapIfNeeded(factory, optionalVariable, trueValue); String falseBranch; if(ExpressionUtils.isSimpleExpression(falseValue)) { - falseBranch = "orElse(" + getComments(falseStatement) + falseValue.getText() + ")"; + falseBranch = "orElse(" + falseValue.getText() + ")"; } else { - falseBranch = "orElseGet(" + getComments(falseStatement) + "() -> " + falseValue.getText() + ")"; + falseBranch = "orElseGet(" + "() -> " + falseValue.getText() + ")"; } return lValue.getText() + " = " + trueBranch + "." + falseBranch + ";"; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterAssignmentMap.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterAssignmentMap.java index b3546b6f6c05..6d07d1bfd856 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterAssignmentMap.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterAssignmentMap.java @@ -5,9 +5,11 @@ import java.util.*; public class Main { public void testOptional(Optional str) { String val; - val = str.map( // line comment - // another line comment - /* block comment *//*block comment*/String::trim).orElse(""); + // line comment +// another line comment +//before trim +/* block comment *//*block comment*/ + val = str.map(String::trim).orElse(""); System.out.println(val); } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterReturnComments.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterReturnComments.java index 5d9b230ad733..73b28dd516d9 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterReturnComments.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterReturnComments.java @@ -11,7 +11,7 @@ public class Main { } public Number testOptionalComments(Optional strList) { - /* optional is present *//* optional is absent */ - return strList.map( /*return something */ myList -> myList.size() > /*too big*/ 1 ? myList.get(1) : 1.0).orElse( /* return null*/ null); + /* optional is present *//*return something *//*too big*//* optional is absent *//* return null*/ + return strList.map(myList -> myList.size() > 1 ? myList.get(1) : 1.0).orElse(null); } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeAssignmentMap.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeAssignmentMap.java index b815ad8f3fee..23ca35a23f77 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeAssignmentMap.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeAssignmentMap.java @@ -8,7 +8,8 @@ public class Main { if (str.isPresent()) { val = // line comment // another line comment - str.get().trim() /* block comment *//*block comment*/; + str.get()//before trim + .trim() /* block comment *//*block comment*/; } else { val = ""; }