From 9b637e541b75638d836e030388e4e70a2234d848 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Tue, 25 Feb 2014 21:59:25 +0100 Subject: [PATCH] for loop -> forEach call chain inspection --- .../analysis/HighlightControlFlowUtil.java | 2 +- .../StreamApiMigrationInspection.java | 206 ++++++++++++++++++ .../streamApiMigration/afterMethodRef.java | 12 + .../streamApiMigration/afterNormal.java | 10 + .../streamApiMigration/beforeBreakInside.java | 15 ++ .../beforeContinueInside.java | 15 ++ .../streamApiMigration/beforeMethodRef.java | 14 ++ .../streamApiMigration/beforeNoFilter.java | 11 + .../streamApiMigration/beforeNormal.java | 14 ++ .../beforeNotEffectivelyFinalVar.java | 16 ++ .../beforeReturnInside.java | 15 ++ .../StreamApiMigrationInspectionTest.java | 45 ++++ .../Convert2streamapi.html | 7 + resources/src/META-INF/IdeaPlugin.xml | 2 + 14 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterMethodRef.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterNormal.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBreakInside.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeContinueInside.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeMethodRef.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNoFilter.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNormal.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNotEffectivelyFinalVar.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeReturnInside.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/StreamApiMigrationInspectionTest.java create mode 100644 resources-en/src/inspectionDescriptions/Convert2streamapi.html diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java index 35f9416d2d8c..031590492d9f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java @@ -675,7 +675,7 @@ public class HighlightControlFlowUtil { return null; } - private static boolean isEffectivelyFinal(PsiVariable variable, PsiElement scope, PsiJavaCodeReferenceElement context) { + public static boolean isEffectivelyFinal(PsiVariable variable, PsiElement scope, PsiJavaCodeReferenceElement context) { boolean effectivelyFinal; if (variable instanceof PsiParameter) { effectivelyFinal = notAccessedForWriting(variable, new LocalSearchScope(((PsiParameter)variable).getDeclarationScope())); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java new file mode 100644 index 000000000000..8101d16f5a1e --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/StreamApiMigrationInspection.java @@ -0,0 +1,206 @@ +/* + * Copyright 2000-2014 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection; + +import com.intellij.codeInsight.daemon.GroupNames; +import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import com.intellij.pom.java.LanguageLevel; +import com.intellij.psi.*; +import com.intellij.psi.controlFlow.*; +import com.intellij.psi.util.InheritanceUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.containers.IntArrayList; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; + +/** + * User: anna + */ +public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTool { + private static final Logger LOG = Logger.getInstance("#" + StreamApiMigrationInspection.class.getName()); + + @Nls + @NotNull + @Override + public String getGroupDisplayName() { + return GroupNames.LANGUAGE_LEVEL_SPECIFIC_GROUP_NAME; + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return "foreach loop can be collapsed with stream api"; + } + + @Override + public boolean isEnabledByDefault() { + return true; + } + + @NotNull + @Override + public String getShortName() { + return "Convert2streamapi"; + } + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitForeachStatement(PsiForeachStatement statement) { + super.visitForeachStatement(statement); + if (PsiUtil.getLanguageLevel(statement).isAtLeast(LanguageLevel.JDK_1_8)) { + final PsiExpression iteratedValue = statement.getIteratedValue(); + final PsiStatement body = statement.getBody(); + if (iteratedValue != null && body != null) { + final PsiType iteratedValueType = iteratedValue.getType(); + if (InheritanceUtil.isInheritor(iteratedValueType, CommonClassNames.JAVA_LANG_ITERABLE)) { + final PsiClass iteratorClass = PsiUtil.resolveClassInType(iteratedValueType); + LOG.assertTrue(iteratorClass != null); + try { + final ControlFlow controlFlow = ControlFlowFactory.getInstance(holder.getProject()) + .getControlFlow(body, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + int startOffset = controlFlow.getStartOffset(body); + int endOffset = controlFlow.getEndOffset(body); + final Collection exitPoints = ControlFlowUtil + .findExitPointsAndStatements(controlFlow, startOffset, endOffset, new IntArrayList(), PsiContinueStatement.class, + PsiBreakStatement.class, PsiReturnStatement.class, PsiThrowStatement.class); + if (exitPoints.isEmpty()) { + + final boolean[] effectivelyFinal = new boolean[] {true}; + body.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitElement(PsiElement element) { + if (!effectivelyFinal[0]) return; + super.visitElement(element); + } + + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + if (!effectivelyFinal[0]) return; + super.visitReferenceExpression(expression); + final PsiElement resolve = expression.resolve(); + if (resolve instanceof PsiVariable && !(resolve instanceof PsiField)) { + effectivelyFinal[0] = HighlightControlFlowUtil.isEffectivelyFinal((PsiVariable)resolve, body, expression); + } + } + }); + + if (effectivelyFinal[0] && !isTrivial(body, statement.getIterationParameter(), iteratedValueType)) { + holder.registerProblem(iteratedValue, "Can be replaced with foreach call", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new ReplaceWithForeachCallFix()); + } + } + } + catch (AnalysisCanceledException ignored) { + } + } + } + } + } + }; + } + + private static boolean isTrivial(PsiStatement body, PsiParameter parameter, PsiType iteratedValueType) { + final PsiIfStatement ifStatement = extractIfStatement(body); + //stream + if (ifStatement != null && ifStatement.getElseBranch() == null && ifStatement.getThenBranch() != null && + InheritanceUtil.isInheritor(iteratedValueType, CommonClassNames.JAVA_UTIL_COLLECTION)) { + return false; + } + //method reference + return LambdaCanBeMethodReferenceInspection.canBeMethodReferenceProblem(body instanceof PsiBlockStatement ? ((PsiBlockStatement)body).getCodeBlock() : body, new PsiParameter[] {parameter}, null) == null; + } + + private static class ReplaceWithForeachCallFix implements LocalQuickFix { + @NotNull + @Override + public String getName() { + return getFamilyName(); + } + + @NotNull + @Override + public String getFamilyName() { + return "Replace with forEach"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + final PsiForeachStatement foreachStatement = PsiTreeUtil.getParentOfType(descriptor.getPsiElement(), PsiForeachStatement.class); + if (foreachStatement != null) { + PsiStatement body = foreachStatement.getBody(); + final PsiExpression iteratedValue = foreachStatement.getIteratedValue(); + if (body != null && iteratedValue != null) { + final PsiParameter parameter = foreachStatement.getIterationParameter(); + final PsiIfStatement ifStmt = extractIfStatement(body); + + String foreEachText = body.getText(); + String iterated = iteratedValue.getText(); + if (ifStmt != null && ifStmt.getElseBranch() == null) { + final PsiExpression condition = ifStmt.getCondition(); + if (condition != null) { + final PsiStatement thenBranch = ifStmt.getThenBranch(); + if (thenBranch != null && InheritanceUtil.isInheritor(iteratedValue.getType(), CommonClassNames.JAVA_UTIL_COLLECTION)) { + body = thenBranch; + foreEachText = thenBranch.getText(); + iterated += ".stream().filter(" + parameter.getName() + " -> " + condition.getText() +")"; + } + } + } + + final PsiParameter[] parameters = {parameter}; + final PsiCallExpression expression = LambdaCanBeMethodReferenceInspection.canBeMethodReferenceProblem(body instanceof PsiBlockStatement ? ((PsiBlockStatement)body).getCodeBlock() : body, parameters, null); + final String methodReferenceText = LambdaCanBeMethodReferenceInspection.createMethodReferenceText(expression, null, parameters); + final String lambdaText = parameter.getName() + " -> " + foreEachText; + final String codeBlock8 = methodReferenceText != null ? methodReferenceText : lambdaText; + PsiExpressionStatement callStatement = (PsiExpressionStatement)JavaPsiFacade.getElementFactory(project).createStatementFromText(iterated + ".forEach(" + codeBlock8 + ");", foreachStatement); + + callStatement = (PsiExpressionStatement)foreachStatement.replace(callStatement); + final PsiExpressionList argumentList = ((PsiCallExpression)callStatement.getExpression()).getArgumentList(); + LOG.assertTrue(argumentList != null, callStatement.getText()); + final PsiExpression[] expressions = argumentList.getExpressions(); + LOG.assertTrue(expressions.length == 1); + + if (expressions[0] instanceof PsiLambdaExpression && ((PsiLambdaExpression)expressions[0]).getFunctionalInterfaceType() == null || + expressions[0] instanceof PsiMethodReferenceExpression && ((PsiMethodReferenceExpression)expressions[0]).getFunctionalInterfaceType() == null) { + callStatement = (PsiExpressionStatement)callStatement.replace(JavaPsiFacade.getElementFactory(project).createStatementFromText(iterated + ".forEach((" + parameter.getText() + ") -> " + foreEachText + ");", callStatement)); + } + } + } + } + } + + public static PsiIfStatement extractIfStatement(PsiStatement body) { + PsiIfStatement ifStmt = null; + if (body instanceof PsiIfStatement) { + ifStmt = (PsiIfStatement)body; + } else if (body instanceof PsiBlockStatement) { + final PsiStatement[] statements = ((PsiBlockStatement)body).getCodeBlock().getStatements(); + if (statements.length == 1 && statements[0] instanceof PsiIfStatement) { + ifStmt = (PsiIfStatement)statements[0]; + } + } + return ifStmt; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterMethodRef.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterMethodRef.java new file mode 100644 index 000000000000..f7d27409fa25 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterMethodRef.java @@ -0,0 +1,12 @@ +// "Replace with forEach" "true" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + foo.forEach(this::bar); + } + + void bar(String s){} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterNormal.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterNormal.java new file mode 100644 index 000000000000..ab67b49343ca --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/afterNormal.java @@ -0,0 +1,10 @@ +// "Replace with forEach" "true" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + foo.stream().filter(s -> s != null).forEach(System.out::println); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBreakInside.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBreakInside.java new file mode 100644 index 000000000000..94d571e9dd74 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeBreakInside.java @@ -0,0 +1,15 @@ +// "Replace with forEach" "false" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + for (String s : foo) { + if (s == null) { + break; + } + } + + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeContinueInside.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeContinueInside.java new file mode 100644 index 000000000000..9ff6de7c05c7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeContinueInside.java @@ -0,0 +1,15 @@ +// "Replace with forEach" "false" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + for (String s : foo) { + if (s == null) { + continue; + } + } + + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeMethodRef.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeMethodRef.java new file mode 100644 index 000000000000..cca5c1e75270 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeMethodRef.java @@ -0,0 +1,14 @@ +// "Replace with forEach" "true" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + for (String s : foo) { + bar(s) + } + } + + void bar(String s){} +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNoFilter.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNoFilter.java new file mode 100644 index 000000000000..6bf6d9ec7705 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNoFilter.java @@ -0,0 +1,11 @@ +// "Replace with forEach" "false" +class Sample { + void foo(It it){ + for (String s : it) { + if (s == null) { + } + } + } +} + +abstract class It implements Iterable {} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNormal.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNormal.java new file mode 100644 index 000000000000..20a815b04ca3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNormal.java @@ -0,0 +1,14 @@ +// "Replace with forEach" "true" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + { + for (String s : foo) { + if (s != null) { + System.out.println(s); + } + } + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNotEffectivelyFinalVar.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNotEffectivelyFinalVar.java new file mode 100644 index 000000000000..7e5903fda0ad --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeNotEffectivelyFinalVar.java @@ -0,0 +1,16 @@ +// "Replace with forEach" "false" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + String foo(){ + boolean b = true; + for (String s : foo) { + if (s == null) { + b = false; + } + } + return null; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeReturnInside.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeReturnInside.java new file mode 100644 index 000000000000..f1279eebde9e --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/beforeReturnInside.java @@ -0,0 +1,15 @@ +// "Replace with forEach" "false" +import java.util.ArrayList; +import java.util.List; + +class Sample { + List foo = new ArrayList<>(); + String foo(){ + for (String s : foo) { + if (s == null) { + return s; + } + } + return null; + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/StreamApiMigrationInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/StreamApiMigrationInspectionTest.java new file mode 100644 index 000000000000..a112ddfa5045 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/StreamApiMigrationInspectionTest.java @@ -0,0 +1,45 @@ +/* + * Copyright 2000-2014 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInsight.daemon.quickFix; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.StreamApiMigrationInspection; +import com.intellij.openapi.projectRoots.Sdk; +import com.intellij.testFramework.IdeaTestUtil; +import org.jetbrains.annotations.NotNull; + + +public class StreamApiMigrationInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{ + new StreamApiMigrationInspection(), + }; + } + + public void test() throws Exception { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration"; + } + + @Override + protected Sdk getProjectJDK() { + return IdeaTestUtil.getMockJdk18(); + } +} \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/Convert2streamapi.html b/resources-en/src/inspectionDescriptions/Convert2streamapi.html new file mode 100644 index 000000000000..55eacf61a9e8 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/Convert2streamapi.html @@ -0,0 +1,7 @@ + + +This inspection reports foreach loops which can be replaced with stream api calls. +

+ Stream api is not available under Java 1.7 or earlier JVMs. + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 967283cbc4cd..d04e05907fa8 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -626,6 +626,8 @@ +