From ebf320b288853d74d9fce868e8d18c22440693e4 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 7 Oct 2016 11:57:53 +0700 Subject: [PATCH] Stream API migration various fixes 1. findFirst() scenario can pull previous assignment (not declaration) now 2. anyMatch() fix did not work if there's single assignment to non-variable (e.g. array element) 3. if non-adjacent return becomes unreachable after findFirst()/anyMatch(), it returned automatically now --- .../streamMigration/MigrateToStreamFix.java | 13 +++++ .../ReplaceWithFindFirstFix.java | 18 ++++++- .../streamMigration/ReplaceWithMatchFix.java | 49 +++++++++++-------- .../afterAnyMatchArrayAssignment.java | 12 +++++ .../afterAnyMatchUnreachableReturn.java | 13 +++++ .../afterFindFirstReAssignment.java | 20 ++++++++ .../afterFindFirstReturnUnreachable.java | 15 ++++++ .../beforeAnyMatchArrayAssignment.java | 16 ++++++ .../beforeAnyMatchUnreachableReturn.java | 19 +++++++ .../beforeFindFirstReAssignment.java | 25 ++++++++++ .../beforeFindFirstReturnUnreachable.java | 21 ++++++++ 11 files changed, 198 insertions(+), 23 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchArrayAssignment.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchUnreachableReturn.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReAssignment.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReturnUnreachable.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchArrayAssignment.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchUnreachableReturn.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReAssignment.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReturnUnreachable.java diff --git a/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java b/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java index 5a1a4c2d0583..2bdbe16a8973 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java @@ -24,6 +24,7 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.controlFlow.*; import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.ig.psiutils.ExpressionUtils; import one.util.streamex.StreamEx; @@ -152,4 +153,16 @@ abstract class MigrateToStreamFix implements LocalQuickFix { statement.delete(); } } + + static boolean isReachable(PsiReturnStatement target) { + ControlFlow flow; + try { + flow = ControlFlowFactory.getInstance(target.getProject()) + .getControlFlow(target.getParent(), LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + } + catch (AnalysisCanceledException e) { + return true; + } + return ControlFlowUtil.isInstructionReachable(flow, flow.getStartOffset(target), 0); + } } diff --git a/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithFindFirstFix.java b/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithFindFirstFix.java index ff9b77733dfe..2a60250cc194 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithFindFirstFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithFindFirstFix.java @@ -19,6 +19,7 @@ import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.codeInspection.streamMigration.StreamApiMigrationInspection.InitializerUsageStatus; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.util.RefactoringUtil; import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NotNull; @@ -52,10 +53,12 @@ class ReplaceWithFindFirstFix extends MigrateToStreamFix { if (!ExpressionUtils.isSimpleExpression(orElseExpression)) return null; stream = generateOptionalUnwrap(stream, tb, value, orElseExpression, null); restoreComments(loopStatement, body); - if (nextReturnStatement.getParent() == loopStatement.getParent()) { + boolean sibling = nextReturnStatement.getParent() == loopStatement.getParent(); + PsiElement replacement = loopStatement.replace(elementFactory.createStatementFromText("return " + stream + ";", loopStatement)); + if(sibling || !isReachable(nextReturnStatement)) { nextReturnStatement.delete(); } - return loopStatement.replace(elementFactory.createStatementFromText("return " + stream + ";", loopStatement)); + return replacement; } else { PsiStatement[] statements = tb.getStatements(); @@ -84,6 +87,17 @@ class ReplaceWithFindFirstFix extends MigrateToStreamFix { return replaceInitializer(loopStatement, var, initializer, replacementText, status); } } + PsiAssignmentExpression previousAssignment = + ExpressionUtils.getAssignment(PsiTreeUtil.skipSiblingsBackward(loopStatement, PsiWhiteSpace.class, PsiComment.class)); + if(previousAssignment != null) { + PsiExpression prevRValue = previousAssignment.getRExpression(); + PsiExpression prevLValue = previousAssignment.getLExpression(); + if(prevRValue != null && prevLValue instanceof PsiReferenceExpression && ((PsiReferenceExpression)prevLValue).resolve() == var) { + previousAssignment.delete(); + return loopStatement.replace(elementFactory.createStatementFromText( + var.getName() + " = " + generateOptionalUnwrap(stream, tb, value, prevRValue, var.getType()) + ";", loopStatement)); + } + } return loopStatement.replace(elementFactory.createStatementFromText( var.getName() + " = " + generateOptionalUnwrap(stream, tb, value, lValue, var.getType()) + ";", loopStatement)); } diff --git a/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithMatchFix.java b/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithMatchFix.java index 1053a306f89c..1d0b0a14bfff 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithMatchFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/ReplaceWithMatchFix.java @@ -69,7 +69,11 @@ class ReplaceWithMatchFix extends MigrateToStreamFix { removeLoop(loopStatement); return returnValue.replace(elementFactory.createExpressionFromText(streamText, nextReturnStatement)); } - return loopStatement.replace(elementFactory.createStatementFromText("return " + streamText + ";", loopStatement)); + PsiElement result = loopStatement.replace(elementFactory.createStatementFromText("return " + streamText + ";", loopStatement)); + if(!isReachable(nextReturnStatement)) { + nextReturnStatement.delete(); + } + return result; } } } @@ -84,27 +88,30 @@ class ReplaceWithMatchFix extends MigrateToStreamFix { if(assignment != null) { PsiExpression lValue = assignment.getLExpression(); PsiExpression rValue = assignment.getRExpression(); - if (!(lValue instanceof PsiReferenceExpression) || rValue == null) return null; - PsiElement maybeVar = ((PsiReferenceExpression)lValue).resolve(); - if(maybeVar instanceof PsiVariable) { - // Simplify single assignments like this: - // boolean flag = false; - // for(....) if(...) {flag = true; break;} - PsiVariable var = (PsiVariable)maybeVar; - PsiExpression initializer = var.getInitializer(); - InitializerUsageStatus status = StreamApiMigrationInspection.getInitializerUsageStatus(var, loopStatement); - if(initializer != null && status != InitializerUsageStatus.UNKNOWN) { - String replacement; - if(ExpressionUtils.isLiteral(initializer, Boolean.FALSE) && - ExpressionUtils.isLiteral(rValue, Boolean.TRUE)) { - replacement = streamText; - } else if(ExpressionUtils.isLiteral(initializer, Boolean.TRUE) && - ExpressionUtils.isLiteral(rValue, Boolean.FALSE)) { - replacement = "!"+streamText; - } else { - replacement = streamText + "?" + rValue.getText() + ":" + initializer.getText(); + if ((lValue instanceof PsiReferenceExpression) && rValue != null) { + PsiElement maybeVar = ((PsiReferenceExpression)lValue).resolve(); + if (maybeVar instanceof PsiVariable) { + // Simplify single assignments like this: + // boolean flag = false; + // for(....) if(...) {flag = true; break;} + PsiVariable var = (PsiVariable)maybeVar; + PsiExpression initializer = var.getInitializer(); + InitializerUsageStatus status = StreamApiMigrationInspection.getInitializerUsageStatus(var, loopStatement); + if (initializer != null && status != InitializerUsageStatus.UNKNOWN) { + String replacement; + if (ExpressionUtils.isLiteral(initializer, Boolean.FALSE) && + ExpressionUtils.isLiteral(rValue, Boolean.TRUE)) { + replacement = streamText; + } + else if (ExpressionUtils.isLiteral(initializer, Boolean.TRUE) && + ExpressionUtils.isLiteral(rValue, Boolean.FALSE)) { + replacement = "!" + streamText; + } + else { + replacement = streamText + "?" + rValue.getText() + ":" + initializer.getText(); + } + return replaceInitializer(loopStatement, var, initializer, replacement, status); } - return replaceInitializer(loopStatement, var, initializer, replacement, status); } } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchArrayAssignment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchArrayAssignment.java new file mode 100644 index 000000000000..6364503ce775 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchArrayAssignment.java @@ -0,0 +1,12 @@ +// "Replace with anyMatch()" "true" + +import java.util.List; + +public class Main { + public void testAssignment(List data) { + String[] found = {"no"}; + if (data.stream().map(String::trim).anyMatch(trimmed -> !trimmed.isEmpty())) { + found[0] = "yes"; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchUnreachableReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchUnreachableReturn.java new file mode 100644 index 000000000000..93a1eb0c5706 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterAnyMatchUnreachableReturn.java @@ -0,0 +1,13 @@ +// "Replace with anyMatch()" "true" + +import java.util.List; + +public class Main { + boolean find(List data) { + if(data != null) { + return data.stream().map(String::trim).anyMatch(trimmed -> trimmed.startsWith("xyz")); + } else { + throw new IllegalArgumentException(); + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReAssignment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReAssignment.java new file mode 100644 index 000000000000..4851693aa34b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReAssignment.java @@ -0,0 +1,20 @@ +// "Replace with findFirst()" "true" + +import java.util.List; +import java.util.Map; +import java.util.Objects; + +public class Main { + private int getInitialSize() {return 0;} + + public void testMap(Map> map) throws Exception { + int firstSize = 10; + + System.out.println(firstSize); + + // loop + // comment + firstSize = map.values().stream().filter(Objects::nonNull).findFirst().map(List::size).orElse(getInitialSize()); + System.out.println(firstSize); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReturnUnreachable.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReturnUnreachable.java new file mode 100644 index 000000000000..45ebed51b091 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterFindFirstReturnUnreachable.java @@ -0,0 +1,15 @@ +// "Replace with findFirst()" "true" + +import java.util.Collection; +import java.util.List; + +public class Main { + public static String find(List> list) { + if(list == null) { + System.out.println("oops"); + return ""; + } else { + return list.stream().flatMap(Collection::stream).filter(string -> string.startsWith("ABC")).findFirst().orElse(null); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchArrayAssignment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchArrayAssignment.java new file mode 100644 index 000000000000..1fe81627f086 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchArrayAssignment.java @@ -0,0 +1,16 @@ +// "Replace with anyMatch()" "true" + +import java.util.List; + +public class Main { + public void testAssignment(List data) { + String[] found = {"no"}; + for(String str : data) { + String trimmed = str.trim(); + if(!trimmed.isEmpty()) { + found[0] = "yes"; + break; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchUnreachableReturn.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchUnreachableReturn.java new file mode 100644 index 000000000000..2fa660e42389 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeAnyMatchUnreachableReturn.java @@ -0,0 +1,19 @@ +// "Replace with anyMatch()" "true" + +import java.util.List; + +public class Main { + boolean find(List data) { + if(data != null) { + for (String e : data) { + String trimmed = e.trim(); + if (trimmed.startsWith("xyz")) { + return true; + } + } + } else { + throw new IllegalArgumentException(); + } + return false; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReAssignment.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReAssignment.java new file mode 100644 index 000000000000..d1cd5f35eff2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReAssignment.java @@ -0,0 +1,25 @@ +// "Replace with findFirst()" "true" + +import java.util.List; +import java.util.Map; + +public class Main { + private int getInitialSize() {return 0;} + + public void testMap(Map> map) throws Exception { + int firstSize = 10; + + System.out.println(firstSize); + + firstSize = getInitialSize(); + // loop + for(List list : map.values()) { + if(list != null) { + firstSize = list.size(); + // comment + break; + } + } + System.out.println(firstSize); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReturnUnreachable.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReturnUnreachable.java new file mode 100644 index 000000000000..a47317fbd733 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeFindFirstReturnUnreachable.java @@ -0,0 +1,21 @@ +// "Replace with findFirst()" "true" + +import java.util.List; + +public class Main { + public static String find(List> list) { + if(list == null) { + System.out.println("oops"); + return ""; + } else { + for (List innerList : list) { + for (String string : innerList) { + if (string.startsWith("ABC")) { + return string; + } + } + } + } + return null; + } +} \ No newline at end of file