diff --git a/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapForEachInspection.java b/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapForEachInspection.java new file mode 100644 index 000000000000..5b1ea7361f47 --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapForEachInspection.java @@ -0,0 +1,223 @@ +/* + * Copyright 2000-2017 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.java18api; + +import com.intellij.codeInspection.*; +import com.intellij.codeInspection.util.LambdaGenerationUtil; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.refactoring.util.LambdaRefactoringUtil; +import com.intellij.util.ObjectUtils; +import com.siyeh.ig.callMatcher.CallMatcher; +import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.EquivalenceChecker; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.MethodCallUtils; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; +import java.util.Objects; + +/** + * @author Tagir Valeev + */ +public class Java8MapForEachInspection extends BaseJavaBatchLocalInspectionTool { + private static final String JAVA_UTIL_MAP_ENTRY = CommonClassNames.JAVA_UTIL_MAP + ".Entry"; + + private static final CallMatcher ITERABLE_FOREACH = + CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_ITERABLE, "forEach").parameterTypes(CommonClassNames.JAVA_UTIL_FUNCTION_CONSUMER); + private static final CallMatcher MAP_ENTRY_SET = + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "entrySet").parameterCount(0); + private static final CallMatcher ENTRY_GETTER = + CallMatcher.instanceCall(JAVA_UTIL_MAP_ENTRY, "getValue", "getKey").parameterCount(0); + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + if (!PsiUtil.isLanguageLevel8OrHigher(holder.getFile())) { + return PsiElementVisitor.EMPTY_VISITOR; + } + return new JavaElementVisitor() { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + if (ITERABLE_FOREACH.test(call)) { + PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call); + if (MAP_ENTRY_SET.test(qualifierCall)) { + PsiLambdaExpression lambda = ObjectUtils.tryCast(call.getArgumentList().getExpressions()[0], PsiLambdaExpression.class); + if (lambda != null) { + PsiParameter[] lambdaParameters = lambda.getParameterList().getParameters(); + if (lambdaParameters.length == 1) { + PsiParameter entry = lambdaParameters[0]; + if (allUsagesAllowed(entry)) { + PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()); + holder.registerProblem(nameElement, InspectionsBundle.message("inspection.map.foreach.message"), + new ReplaceWithMapForEachFix()); + } + } + } + } + } + } + + private boolean allUsagesAllowed(@NotNull PsiParameter entry) { + return ReferencesSearch.search(entry).forEach(entryRef -> { + PsiMethodCallExpression entryCall = + ExpressionUtils.getCallForQualifier(ObjectUtils.tryCast(entryRef.getElement(), PsiExpression.class)); + return ENTRY_GETTER.test(entryCall); + }); + } + + @Override + public void visitForeachStatement(PsiForeachStatement loop) { + PsiMethodCallExpression call = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(loop.getIteratedValue()), PsiMethodCallExpression.class); + if (MAP_ENTRY_SET.test(call) && + LambdaGenerationUtil.canBeUncheckedLambda(loop.getBody()) && + allUsagesAllowed(loop.getIterationParameter())) { + holder.registerProblem(loop.getFirstChild(), InspectionsBundle.message("inspection.map.foreach.message"), + new ReplaceWithMapForEachFix()); + } + } + }; + } + + private static class ReplaceWithMapForEachFix implements LocalQuickFix { + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.map.foreach.fix.name"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement element = descriptor.getStartElement(); + if (element.getParent() instanceof PsiForeachStatement) { + fixInForeach((PsiForeachStatement)element.getParent()); + return; + } + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class); + if (call == null) return; + PsiMethodCallExpression entrySetCall = MethodCallUtils.getQualifierMethodCall(call); + if (entrySetCall == null) return; + PsiExpression[] args = call.getArgumentList().getExpressions(); + if (args.length != 1) return; + PsiLambdaExpression lambda = ObjectUtils.tryCast(args[0], PsiLambdaExpression.class); + if (lambda == null) return; + PsiElement body = lambda.getBody(); + if (body == null) return; + PsiParameterList parameterList = lambda.getParameterList(); + PsiParameter[] lambdaParameters = parameterList.getParameters(); + if (lambdaParameters.length != 1) return; + CommentTracker ct = new CommentTracker(); + PsiParameter entryParameter = lambdaParameters[0]; + String replacement = createReplacementExpression(entrySetCall, entryParameter, body, ct); + ct.replaceAndRestoreComments(call, replacement); + } + + private static String createReplacementExpression(PsiMethodCallExpression entrySetCall, + PsiParameter entryParameter, + PsiElement body, + CommentTracker ct) { + PsiType entryType = entryParameter.getType(); + ParameterCandidate key = new ParameterCandidate(entryType, true); + ParameterCandidate value = new ParameterCandidate(entryType, false); + Collection references = ReferencesSearch.search(entryParameter).findAll(); + for (PsiReference ref : references) { + PsiMethodCallExpression entryCall = ExpressionUtils.getCallForQualifier(ObjectUtils.tryCast(ref.getElement(), PsiExpression.class)); + if (ENTRY_GETTER.test(entryCall)) { + ParameterCandidate.select(entryCall, key, value).accept(entryCall); + } + } + key.createName(body, ct); + value.createName(body, ct); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(entrySetCall.getProject()); + for (PsiReference ref : references) { + PsiExpression expression = ObjectUtils.tryCast(ref.getElement(), PsiExpression.class); + if (expression == null || !expression.isValid()) continue; + PsiMethodCallExpression entryCall = ExpressionUtils.getCallForQualifier(expression); + if (ENTRY_GETTER.test(entryCall)) { + ct.replace(entryCall, factory.createIdentifier(ParameterCandidate.select(entryCall, key, value).myName)); + } + } + String lambdaBody; + if (body instanceof PsiExpression || body instanceof PsiCodeBlock || body instanceof PsiBlockStatement) { + lambdaBody = ct.text(body); + } + else { + lambdaBody = "{" + ct.text(body) + "}"; + } + PsiLambdaExpression newLambda = + (PsiLambdaExpression)factory.createExpressionFromText("(" + key.myName + "," + value.myName + ")->" + lambdaBody, body); + LambdaRefactoringUtil.simplifyToExpressionLambda(newLambda); + entrySetCall.getArgumentList().add(newLambda); + entrySetCall.getMethodExpression().handleElementRename("forEach"); + return entrySetCall.getText(); + } + + private static void fixInForeach(PsiForeachStatement loop) { + PsiMethodCallExpression entrySetCall = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(loop.getIteratedValue()), PsiMethodCallExpression.class); + if (entrySetCall == null) return; + PsiElement body = loop.getBody(); + if (body == null) return; + PsiParameter entryParameter = loop.getIterationParameter(); + CommentTracker ct = new CommentTracker(); + String replacementExpression = createReplacementExpression(entrySetCall, entryParameter, body, ct); + ct.replaceAndRestoreComments(loop, replacementExpression + ";"); + } + + private static class ParameterCandidate { + PsiVariable myOriginalVar; + final PsiType myType; + String myName; + + public ParameterCandidate(PsiType entryType, boolean isKey) { + myName = isKey ? "key" : "value"; + myType = GenericsUtil + .getVariableTypeByExpressionType(PsiUtil.substituteTypeParameter(entryType, JAVA_UTIL_MAP_ENTRY, isKey ? 0 : 1, true)); + } + + private void createName(PsiElement context, CommentTracker ct) { + if (myOriginalVar != null) { + myName = myOriginalVar.getName(); + ct.delete(myOriginalVar); + } + else { + myName = JavaCodeStyleManager.getInstance(context.getProject()).suggestUniqueVariableName(myName, context, true); + } + } + + public void accept(PsiMethodCallExpression call) { + if (myOriginalVar != null) return; + PsiLocalVariable variable = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(call.getParent()), PsiLocalVariable.class); + if (variable != null && EquivalenceChecker.getCanonicalPsiEquivalence().typesAreEquivalent(variable.getType(), myType)) { + myOriginalVar = variable; + } + } + + static ParameterCandidate select(PsiMethodCallExpression entryCall, ParameterCandidate key, ParameterCandidate value) { + String methodName = entryCall.getMethodExpression().getReferenceName(); + return "getKey".equals(methodName) ? key : value; + } + } + } +} \ No newline at end of file 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 aecda0333c38..87daff2cbf90 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamMigration/StreamApiMigrationInspection.java @@ -29,7 +29,6 @@ import com.intellij.openapi.util.TextRange; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; import com.intellij.psi.controlFlow.*; -import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.InheritanceUtil; @@ -303,7 +302,7 @@ public class StreamApiMigrationInspection extends BaseJavaBatchLocalInspectionTo @Nullable private static PsiClassType createDefaultConsumerType(Project project, PsiVariable variable) { final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project); - final PsiClass consumerClass = psiFacade.findClass("java.util.function.Consumer", GlobalSearchScope.allScope(project)); + final PsiClass consumerClass = psiFacade.findClass(CommonClassNames.JAVA_UTIL_FUNCTION_CONSUMER, variable.getResolveScope()); return consumerClass != null ? psiFacade.getElementFactory().createType(consumerClass, variable.getType()) : null; } diff --git a/java/java-tests/testData/inspection/java8MapForEach/afterForEach.java b/java/java-tests/testData/inspection/java8MapForEach/afterForEach.java new file mode 100644 index 000000000000..b2925561df4f --- /dev/null +++ b/java/java-tests/testData/inspection/java8MapForEach/afterForEach.java @@ -0,0 +1,89 @@ +// "Fix all 'Replace with Map.forEach' problems in file" "true" +import java.util.Map; +import java.util.function.Supplier; + +public class Test { + public static void testInline(Map map) { + map.forEach((key, value) -> System.out.println(key + ":" + value)); + } + + public static void testKey(Map map) { + map.forEach((str, value) -> System.out.println(str + ":" + value)); + } + + public static void testValue(Map map) { + map.forEach((str, num) -> System.out.println(str + ":" + num)); + } + + public static void testTwoVarsWildcard(Map map) { + map.forEach((str, num) -> { + System.out.println(str + ":" + num); + String str2 = str; + System.out.println(str2); + }); + } + + public static > void testGeneric(Supplier map) { + map.get().forEach((key, value) -> System.out.println(key + ":" + value)); + } + + public static > void testUsedHashCode(Supplier map) { + map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue()+":"+e.hashCode())); + } + + public static void testForLoop(Map map) { + map.forEach((key, value) -> System.out.println(key + ":" + value)); + } + + public static void testForLoop2(Map map) { + map.forEach((str, num) -> System.out.println(str + ":" + num)); + } + + public static void testForLoop3(Map map) { + map.forEach((str, num) -> { + System.out.println(str + ":" + num); + System.out.println(num + ":" + str); + }); + } + + public static void testForLoopSet(Map map) { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str + ":" + entry.getValue()); + entry.setValue(1); + } + } + + public static void testForLoopPrimitive(Map map) { + map.forEach((str, value) -> { + int num = value; + System.out.println(str + ":" + num); + }); + } + + public static void testForLoopSideEffect(Map map) { + Integer num; + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + num = entry.getValue(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoopThrow(Map map) throws Exception { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + if(num > 0) throw new Exception(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoopThrowRuntime(Map map) throws Exception { + map.forEach((str, num) -> { + if (num > 0) throw new RuntimeException(); + System.out.println(str + ":" + num); + }); + } +} diff --git a/java/java-tests/testData/inspection/java8MapForEach/beforeForEach.java b/java/java-tests/testData/inspection/java8MapForEach/beforeForEach.java new file mode 100644 index 000000000000..77a27e10fe2c --- /dev/null +++ b/java/java-tests/testData/inspection/java8MapForEach/beforeForEach.java @@ -0,0 +1,111 @@ +// "Fix all 'Replace with Map.forEach' problems in file" "true" +import java.util.Map; +import java.util.function.Supplier; + +public class Test { + public static void testInline(Map map) { + map.entrySet().forEach(entry -> + System.out.println(entry.getKey() +":"+entry.getValue()) + ); + } + + public static void testKey(Map map) { + map.entrySet().forEach(entry -> { + String str = entry.getKey(); + System.out.println(str +":"+entry.getValue()); + }); + } + + public static void testValue(Map map) { + map.entrySet().forEach(entry -> { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str +":"+ num); + }); + } + + public static void testTwoVarsWildcard(Map map) { + map.entrySet().forEach(entry -> { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str +":"+ num); + String str2 = entry.getKey(); + System.out.println(str2); + }); + } + + public static > void testGeneric(Supplier map) { + map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue())); + } + + public static > void testUsedHashCode(Supplier map) { + map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue()+":"+e.hashCode())); + } + + public static void testForLoop(Map map) { + for (Map.Entry entry : map.entrySet()) { + System.out.println(entry.getKey() + ":" + entry.getValue()); + } + } + + public static void testForLoop2(Map map) { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoop3(Map map) { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str + ":" + entry.getValue()); + System.out.println(num + ":" + str); + } + } + + public static void testForLoopSet(Map map) { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + System.out.println(str + ":" + entry.getValue()); + entry.setValue(1); + } + } + + public static void testForLoopPrimitive(Map map) { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + int num = entry.getValue(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoopSideEffect(Map map) { + Integer num; + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + num = entry.getValue(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoopThrow(Map map) throws Exception { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + if(num > 0) throw new Exception(); + System.out.println(str + ":" + num); + } + } + + public static void testForLoopThrowRuntime(Map map) throws Exception { + for (Map.Entry entry : map.entrySet()) { + String str = entry.getKey(); + Integer num = entry.getValue(); + if(num > 0) throw new RuntimeException(); + System.out.println(str + ":" + num); + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/java18api/Java8MapForEachInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/java18api/Java8MapForEachInspectionTest.java new file mode 100644 index 000000000000..d19d530a208f --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/java18api/Java8MapForEachInspectionTest.java @@ -0,0 +1,40 @@ +/* + * Copyright 2000-2017 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.java18api; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +/** + * @author Tagir Valeev + */ +public class Java8MapForEachInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new Java8MapForEachInspection()}; + } + + public void test() throws Exception { + doAllTests(); + } + + @Override + protected String getBasePath() { + return "/inspection/java8MapForEach"; + } +} diff --git a/platform/core-api/src/com/intellij/psi/CommonClassNames.java b/platform/core-api/src/com/intellij/psi/CommonClassNames.java index e894f69a1d77..fb277f6ea4ea 100644 --- a/platform/core-api/src/com/intellij/psi/CommonClassNames.java +++ b/platform/core-api/src/com/intellij/psi/CommonClassNames.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -110,6 +110,7 @@ public interface CommonClassNames { @NonNls String JAVA_UTIL_STREAM_DOUBLE_STREAM = "java.util.stream.DoubleStream"; @NonNls String JAVA_UTIL_STREAM_COLLECTORS = "java.util.stream.Collectors"; @NonNls String JAVA_UTIL_FUNCTION_PREDICATE = "java.util.function.Predicate"; + @NonNls String JAVA_UTIL_FUNCTION_CONSUMER = "java.util.function.Consumer"; @NonNls String JAVA_UTIL_FUNCTION_FUNCTION = "java.util.function.Function"; @NonNls String JAVA_UTIL_FUNCTION_BIFUNCTION = "java.util.function.BiFunction"; diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index b9beb42039b8..21ed071c92d9 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -775,3 +775,6 @@ inspection.redundant.stream.optional.call.explanation.parallel=there''s subseque inspection.redundant.stream.optional.call.fix.family.name=Remove redundant chain call inspection.redundant.stream.optional.call.fix.name=Remove ''{0}'' call inspection.redundant.stream.optional.call.option.streamboxing=Report useless boxing in Stream.map + +inspection.map.foreach.message=Can be replaced with 'Map.forEach' +inspection.map.foreach.fix.name=Replace with Map.forEach diff --git a/resources-en/src/inspectionDescriptions/Java8MapForEach.html b/resources-en/src/inspectionDescriptions/Java8MapForEach.html new file mode 100644 index 000000000000..6302a70a74a1 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/Java8MapForEach.html @@ -0,0 +1,9 @@ + + +This inspection suggests to replace for(Entry<?,?> entry : map.entrySet()) {...} or +map.entrySet().forEach(entry -> ...) with map.forEach((key, value) -> ...). + +

This inspection is available since Java 8 only.

+New in 2017.1 + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 350ec891c52f..b5b47ff5e73d 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -855,6 +855,11 @@ groupKey="group.names.language.level.specific.issues.and.migration.aids" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.java18api.Java8MapApiInspection" displayName="Replace with single Map method"/> +