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 2bdbe16a8973..2a34ec766033 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/MigrateToStreamFix.java @@ -56,6 +56,7 @@ abstract class MigrateToStreamFix implements LocalQuickFix { if (!FileModificationService.getInstance().preparePsiElementForWrite(loopStatement)) return; PsiElement result = migrate(project, loopStatement, body, tb); if(result != null) { + source.cleanUpSource(); simplifyAndFormat(project, result); } } 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 13281bd34db4..e0506fa893ee 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java @@ -390,7 +390,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo return consumerClass != null ? psiFacade.getElementFactory().createType(consumerClass, variable.getType()) : null; } - static boolean isVariableSuitableForStream(PsiVariable variable, PsiStatement statement) { + static boolean isVariableSuitableForStream(PsiVariable variable, PsiStatement statement, StreamSource source) { PsiElement declaration = variable.getParent(); // For-loop initializer is not effectively final, but suitable for stream conversion if(declaration instanceof PsiDeclarationStatement) { @@ -408,6 +408,14 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo } } } + if(statement instanceof PsiWhileStatement && source.getVariable() == variable) { + return ReferencesSearch.search(variable, variable.getUseScope()).forEach(ref -> { + PsiElement element = ref.getElement(); + return !(element instanceof PsiExpression) || + PsiTreeUtil.isAncestor(((PsiWhileStatement)statement).getCondition(), element, false) || + !PsiUtil.isAccessedForWriting((PsiExpression)element); + }); + } return HighlightControlFlowUtil.isEffectivelyFinal(variable, statement, null); } @@ -429,6 +437,58 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo return null; } + /** + * Checks whether variable can be referenced between start and loop entry. Back-edges are also considered, so the actual place + * where it referenced might be outside of (start, loop entry) interval. + * + * @param flow ControlFlow to analyze + * @param start start point + * @param loop loop to check + * @param variable variable to analyze + * @return true if variable can be referenced between start and stop points + */ + private static boolean isVariableReferencedBeforeLoopEntry(final ControlFlow flow, + final int start, + final PsiLoopStatement loop, + final PsiVariable variable) { + final int loopStart = flow.getStartOffset(loop); + final int loopEnd = flow.getEndOffset(loop); + if(start == loopStart) return false; + + List edges = ControlFlowUtil.getEdges(flow, start); + // DFS visits instructions mainly in backward direction while here visiting in forward direction + // greatly reduces number of iterations. + Collections.reverse(edges); + + BitSet referenced = new BitSet(); + boolean changed = true; + while(changed) { + changed = false; + for(ControlFlowUtil.ControlFlowEdge edge: edges) { + int from = edge.myFrom; + int to = edge.myTo; + if(referenced.get(from)) { + // jump to the loop start from within the loop is not considered as loop entry + if(to == loopStart && (from < loopStart || from >= loopEnd)) { + return true; + } + if(!referenced.get(to)) { + referenced.set(to); + changed = true; + } + continue; + } + if(ControlFlowUtil.isVariableAccess(flow, from, variable)) { + referenced.set(from); + referenced.set(to); + if(to == loopStart) return true; + changed = true; + } + } + } + return false; + } + enum InitializerUsageStatus { // Variable is declared just before the wanted place DECLARED_JUST_BEFORE, @@ -440,7 +500,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo UNKNOWN } - static InitializerUsageStatus getInitializerUsageStatus(PsiVariable var, PsiStatement nextStatement) { + static InitializerUsageStatus getInitializerUsageStatus(PsiVariable var, PsiLoopStatement nextStatement) { if(!(var instanceof PsiLocalVariable) || var.getInitializer() == null) return UNKNOWN; if(isDeclarationJustBefore(var, nextStatement)) return DECLARED_JUST_BEFORE; // Check that variable is declared in the same method or the same lambda expression @@ -458,7 +518,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo } int start = controlFlow.getEndOffset(var.getInitializer())+1; int stop = controlFlow.getStartOffset(nextStatement); - if(ControlFlowUtil.isVariableReferencedBetween(controlFlow, start, stop, var)) return UNKNOWN; + if(isVariableReferencedBeforeLoopEntry(controlFlow, start, nextStatement, var)) return UNKNOWN; if (!ControlFlowUtil.isValueUsedWithoutVisitingStop(controlFlow, start, stop, var)) return AT_WANTED_PLACE_ONLY; return var.hasModifierProperty(PsiModifier.FINAL) ? UNKNOWN : AT_WANTED_PLACE; } @@ -494,6 +554,12 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo processLoop(statement); } + @Override + public void visitWhileStatement(PsiWhileStatement statement) { + super.visitWhileStatement(statement); + processLoop(statement); + } + @Override public void visitForStatement(PsiForStatement statement) { super.visitForStatement(statement); @@ -524,7 +590,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo int startOffset = controlFlow.getStartOffset(body); int endOffset = controlFlow.getEndOffset(body); final List nonFinalVariables = StreamEx.of(ControlFlowUtil.getUsedVariables(controlFlow, startOffset, endOffset)) - .remove(variable -> isVariableSuitableForStream(variable, statement)).toList(); + .remove(variable -> isVariableSuitableForStream(variable, statement, source)).toList(); if (exitPoints.isEmpty()) { if(getIncrementedVariable(tb, nonFinalVariables) != null) { @@ -679,6 +745,12 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo PsiStatement initialization = ((PsiForStatement)statement).getInitialization(); LOG.assertTrue(initialization != null); return initialization.getTextRange(); + } else if(statement instanceof PsiWhileStatement) { + PsiJavaToken rParenth = ((PsiWhileStatement)statement).getRParenth(); + if (wholeStatement && rParenth != null) { + return new TextRange(statement.getTextOffset(), rParenth.getTextOffset() + 1); + } + return statement.getFirstChild().getTextRange(); } else { throw new IllegalStateException("Unexpected statement type: "+statement); } @@ -964,6 +1036,9 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo super(null, expression, variable); } + void cleanUpSource() { + } + @Contract("null -> null") static StreamSource tryCreate(PsiLoopStatement statement) { if(statement instanceof PsiForStatement) { @@ -973,10 +1048,69 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo ArrayStream source = ArrayStream.from((PsiForeachStatement)statement); return source == null ? CollectionStream.from((PsiForeachStatement)statement) : source; } + if(statement instanceof PsiWhileStatement) { + return BufferedReaderLines.from((PsiWhileStatement)statement); + } return null; } } + static class BufferedReaderLines extends StreamSource { + private BufferedReaderLines(PsiVariable variable, PsiExpression expression) { + super(variable, expression); + } + + @Override + String createReplacement() { + return myExpression.getText()+".lines()"; + } + + @Override + void cleanUpSource() { + myVariable.delete(); + } + + @Nullable + public static BufferedReaderLines from(PsiWhileStatement whileLoop) { + // while ((line = br.readLine()) != null) + PsiExpression condition = PsiUtil.skipParenthesizedExprDown(whileLoop.getCondition()); + if(!(condition instanceof PsiBinaryExpression)) return null; + PsiBinaryExpression binOp = (PsiBinaryExpression)condition; + if(!JavaTokenType.NE.equals(binOp.getOperationTokenType())) return null; + PsiExpression operand = null; + if(ExpressionUtils.isNullLiteral(binOp.getROperand())) { + operand = binOp.getLOperand(); + } else if(ExpressionUtils.isNullLiteral(binOp.getLOperand())) { + operand = binOp.getROperand(); + } + if(operand == null) return null; + PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(PsiUtil.skipParenthesizedExprDown(operand)); + if(assignment == null) return null; + PsiExpression lValue = assignment.getLExpression(); + if(!(lValue instanceof PsiReferenceExpression)) return null; + PsiElement element = ((PsiReferenceExpression)lValue).resolve(); + if(!(element instanceof PsiLocalVariable)) return null; + PsiLocalVariable var = (PsiLocalVariable)element; + if(!ReferencesSearch.search(var, var.getUseScope()).forEach(ref -> { + return PsiTreeUtil.isAncestor(whileLoop, ref.getElement(), true); + })) { + return null; + } + PsiExpression rValue = PsiUtil.skipParenthesizedExprDown(assignment.getRExpression()); + if(!(rValue instanceof PsiMethodCallExpression)) return null; + PsiMethodCallExpression call = (PsiMethodCallExpression)rValue; + if(call.getArgumentList().getExpressions().length != 0) return null; + if(!"readLine".equals(call.getMethodExpression().getReferenceName())) return null; + PsiExpression readerExpression = call.getMethodExpression().getQualifierExpression(); + if(readerExpression == null) return null; + PsiMethod method = call.resolveMethod(); + if(method == null) return null; + PsiClass aClass = method.getContainingClass(); + if(aClass == null || !"java.io.BufferedReader".equals(aClass.getQualifiedName())) return null; + return new BufferedReaderLines(var, readerExpression); + } + } + static class ArrayStream extends StreamSource { private ArrayStream(PsiVariable variable, PsiExpression expression) { super(variable, expression); @@ -1224,7 +1358,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo if(source == null || body == null) return null; // flatMap from primitive to primitive is supported only if primitive types match // otherwise it would be necessary to create bogus step like - // .mapToObj(var -> blahblah.stream()).flatMap(Function.identity()) + // .mapToObj(var -> collection.stream()).flatMap(Function.identity()) if(myVariable.getType() instanceof PsiPrimitiveType && !myVariable.getType().equals(source.getVariable().getType())) return null; FlatMapOp op = new FlatMapOp(myPreviousOp, source, myVariable, loopStatement); TerminalBlock withFlatMap = new TerminalBlock(op, source.getVariable(), body); diff --git a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java index ddab945cff0f..4e66505942e4 100644 --- a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java @@ -29,7 +29,6 @@ import com.intellij.util.containers.IntArrayList; import com.intellij.util.containers.IntStack; import gnu.trove.THashMap; import gnu.trove.THashSet; -import gnu.trove.TIntArrayList; import gnu.trove.TIntHashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -1330,30 +1329,47 @@ public class ControlFlowUtil { } /** - * Checks whether variable can be referenced between start and stop points. Back-edges are also considered, so the actual place - * where it referenced might be outside of (start, stop) interval. + * Checks if the control flow instruction at given offset accesses (reads or writes) given variable * - * @param flow ControlFlow to analyze - * @param start start point - * @param stop stop point - * @param variable variable to analyze - * @return true if variable can be referenced between start and stop points + * @param flow control flow + * @param offset offset inside given control flow + * @param variable a variable the access to which is to be checked + * @return true if the given instruction is actually a variable access */ - public static boolean isVariableReferencedBetween(final ControlFlow flow, - final int start, - final int stop, - final PsiVariable variable) { - if(start == stop) return false; + public static boolean isVariableAccess(ControlFlow flow, int offset, PsiVariable variable) { + Instruction instruction = flow.getInstructions().get(offset); + return instruction instanceof ReadVariableInstruction && ((ReadVariableInstruction)instruction).variable == variable || + instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == variable; + } - // DFS visits instructions mainly in backward direction while here visiting in forward direction - // greatly reduces number of iterations. So first we just collect edges, then reverse their order. - // contains (from, to) pairs representing control flow arcs - final TIntArrayList list = new TIntArrayList(); + public static class ControlFlowEdge { + public final int myFrom; + public final int myTo; + + public ControlFlowEdge(int from, int to) { + myFrom = from; + myTo = to; + } + + @Override + public String toString() { + return myFrom+"->"+myTo; + } + } + + /** + * Returns control flow edges which are potentially reachable from start instruction + * + * @param flow control flow to analyze + * @param start starting instruction offset + * @return a list of edges + */ + public static List getEdges(ControlFlow flow, int start) { + final List list = new ArrayList(); depthFirstSearch(flow, new InstructionClientVisitor() { @Override public void visitInstruction(Instruction instruction, int offset, int nextOffset) { - list.add(offset); - list.add(nextOffset); + list.add(new ControlFlowEdge(offset, nextOffset)); } @Override @@ -1361,34 +1377,7 @@ public class ControlFlowUtil { return null; } }, start, flow.getSize()); - BitSet violated = new BitSet(); - List instructions = flow.getInstructions(); - boolean changed = true; - while(changed) { - changed = false; - for(int i=list.size()-2; i>=0; i-=2) { - int from = list.get(i); - int to = list.get(i+1); - if(from == stop) continue; - if(violated.get(from)) { - if(!violated.get(to)) { - if(to == stop) return true; - violated.set(to); - changed = true; - } - continue; - } - Instruction instruction = instructions.get(from); - if((instruction instanceof ReadVariableInstruction && ((ReadVariableInstruction)instruction).variable == variable) || - (instruction instanceof WriteVariableInstruction && ((WriteVariableInstruction)instruction).variable == variable)) { - violated.set(from); - violated.set(to); - if(to == stop) return true; - changed = true; - } - } - } - return false; + return list; } /** diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollect.java new file mode 100644 index 000000000000..25f7274c3172 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollect.java @@ -0,0 +1,15 @@ +// "Replace with collect" "true" + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + List test(BufferedReader br) throws IOException { + List result; + result = br.lines().map(String::trim).collect(Collectors.toList()); + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollectNestedOk.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollectNestedOk.java new file mode 100644 index 000000000000..53c710fd73a3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderCollectNestedOk.java @@ -0,0 +1,19 @@ +// "Replace with collect" "true" + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + List test(List readers) throws IOException { + for(BufferedReader br : readers) { + List result; + result = br.lines().map(String::trim).collect(Collectors.toList()); + if(result.size() > 10) { + return result; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderSum.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderSum.java new file mode 100644 index 000000000000..93c99a83b3e0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterBufferedReaderSum.java @@ -0,0 +1,11 @@ +// "Replace with sum()" "true" + +import java.io.BufferedReader; +import java.io.IOException; + +public class Main { + void test(BufferedReader br) throws IOException { + long count = br.lines().map(String::trim).mapToLong(String::length).sum(); + System.out.println(count); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollect.java new file mode 100644 index 000000000000..23d877d08403 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollect.java @@ -0,0 +1,17 @@ +// "Replace with collect" "true" + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class Main { + List test(BufferedReader br) throws IOException { + List result = new ArrayList<>(); + String line = ""; + while(null != (line = br.readLine())) { + result.add(line.trim()); + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNested.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNested.java new file mode 100644 index 000000000000..6d050b10678d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNested.java @@ -0,0 +1,19 @@ +// "Replace with collect" "false" + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class Main { + List test(List readers) throws IOException { + List result = new ArrayList<>(); + for(BufferedReader br : readers) { + String line = ""; + while (null != (line = br.readLine())) { + result.add(line.trim()); + } + } + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNestedOk.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNestedOk.java new file mode 100644 index 000000000000..46d62990f876 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderCollectNestedOk.java @@ -0,0 +1,21 @@ +// "Replace with collect" "true" + +import java.io.BufferedReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public class Main { + List test(List readers) throws IOException { + for(BufferedReader br : readers) { + List result = new ArrayList<>(); + String line = ""; + while (null != (line = br.readLine())) { + result.add(line.trim()); + } + if(result.size() > 10) { + return result; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderModifiedLine.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderModifiedLine.java new file mode 100644 index 000000000000..dda6b3327432 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderModifiedLine.java @@ -0,0 +1,16 @@ +// "Replace with sum()" "false" + +import java.io.BufferedReader; +import java.io.IOException; + +public class Main { + void test(BufferedReader br) throws IOException { + String line = ""; + long count = 0; + while((line = br.readLine()) != null) { + line = line.trim(); + count+=line.length(); + } + System.out.println(count); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSum.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSum.java new file mode 100644 index 000000000000..d04658e420c8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSum.java @@ -0,0 +1,16 @@ +// "Replace with sum()" "true" + +import java.io.BufferedReader; +import java.io.IOException; + +public class Main { + void test(BufferedReader br) throws IOException { + String line = ""; + long count = 0; + while((line = br.readLine()) != null) { + String trimmed = line.trim(); + count+=trimmed.length(); + } + System.out.println(count); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSumLineReused.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSumLineReused.java new file mode 100644 index 000000000000..369771058f97 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBufferedReaderSumLineReused.java @@ -0,0 +1,16 @@ +// "Replace with sum()" "false" + +import java.io.BufferedReader; +import java.io.IOException; + +public class Main { + void test(BufferedReader br) throws IOException { + String line = ""; + long count = 0; + while((line = br.readLine()) != null) { + String trimmed = line.trim(); + count+=trimmed.length(); + } + System.out.println(count+":"+line); + } +} \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/Convert2streamapi.html b/resources-en/src/inspectionDescriptions/Convert2streamapi.html index ff190f897875..3e2b1d886ea2 100644 --- a/resources-en/src/inspectionDescriptions/Convert2streamapi.html +++ b/resources-en/src/inspectionDescriptions/Convert2streamapi.html @@ -1,6 +1,6 @@ -This inspection reports foreach loops which can be replaced with stream API calls. +This inspection reports loops which can be replaced with stream API calls.

Stream API is not available under Java 1.7 or earlier JVMs. diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 8193fde74466..005b554bb44e 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -760,7 +760,7 @@ -