IDEA-146716 Add inspection for "Iteration over entrySet() can be replaced with forEach"

This commit is contained in:
Tagir Valeev
2017-02-07 11:40:25 +07:00
parent c41f0a2020
commit 5c7ab507f9
9 changed files with 483 additions and 3 deletions
@@ -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<PsiReference> 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;
}
}
}
}
@@ -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;
}
@@ -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<String, Integer> map) {
map.forEach((key, value) -> System.out.println(key + ":" + value));
}
public static void testKey(Map<String, Integer> map) {
map.forEach((str, value) -> System.out.println(str + ":" + value));
}
public static void testValue(Map<String, Integer> map) {
map.forEach((str, num) -> System.out.println(str + ":" + num));
}
public static void testTwoVarsWildcard(Map<? extends String, Integer> map) {
map.forEach((str, num) -> {
System.out.println(str + ":" + num);
String str2 = str;
System.out.println(str2);
});
}
public static <T extends Map<?, ?>> void testGeneric(Supplier<T> map) {
map.get().forEach((key, value) -> System.out.println(key + ":" + value));
}
public static <T extends Map<?, ?>> void testUsedHashCode(Supplier<T> map) {
map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue()+":"+e.hashCode()));
}
public static void testForLoop(Map<String, Integer> map) {
map.forEach((key, value) -> System.out.println(key + ":" + value));
}
public static void testForLoop2(Map<String, Integer> map) {
map.forEach((str, num) -> System.out.println(str + ":" + num));
}
public static void testForLoop3(Map<String, Integer> map) {
map.forEach((str, num) -> {
System.out.println(str + ":" + num);
System.out.println(num + ":" + str);
});
}
public static void testForLoopSet(Map<String, Integer> map) {
for (Map.Entry<String, Integer> 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<String, Integer> map) {
map.forEach((str, value) -> {
int num = value;
System.out.println(str + ":" + num);
});
}
public static void testForLoopSideEffect(Map<String, Integer> map) {
Integer num;
for (Map.Entry<String, Integer> entry : map.entrySet()) {
String str = entry.getKey();
num = entry.getValue();
System.out.println(str + ":" + num);
}
}
public static void testForLoopThrow(Map<String, Integer> map) throws Exception {
for (Map.Entry<String, Integer> 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<String, Integer> map) throws Exception {
map.forEach((str, num) -> {
if (num > 0) throw new RuntimeException();
System.out.println(str + ":" + num);
});
}
}
@@ -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<String, Integer> map) {
map.entrySet().for<caret>Each(entry ->
System.out.println(entry.getKey() +":"+entry.getValue())
);
}
public static void testKey(Map<String, Integer> map) {
map.entrySet().forEach(entry -> {
String str = entry.getKey();
System.out.println(str +":"+entry.getValue());
});
}
public static void testValue(Map<String, Integer> map) {
map.entrySet().forEach(entry -> {
String str = entry.getKey();
Integer num = entry.getValue();
System.out.println(str +":"+ num);
});
}
public static void testTwoVarsWildcard(Map<? extends String, Integer> 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 <T extends Map<?, ?>> void testGeneric(Supplier<T> map) {
map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue()));
}
public static <T extends Map<?, ?>> void testUsedHashCode(Supplier<T> map) {
map.get().entrySet().forEach(e -> System.out.println(e.getKey()+":"+e.getValue()+":"+e.hashCode()));
}
public static void testForLoop(Map<String, Integer> map) {
for (Map.Entry<String, Integer> entry : map.entrySet()) {
System.out.println(entry.getKey() + ":" + entry.getValue());
}
}
public static void testForLoop2(Map<String, Integer> map) {
for (Map.Entry<String, Integer> entry : map.entrySet()) {
String str = entry.getKey();
Integer num = entry.getValue();
System.out.println(str + ":" + num);
}
}
public static void testForLoop3(Map<String, Integer> map) {
for (Map.Entry<String, Integer> 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<String, Integer> map) {
for (Map.Entry<String, Integer> 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<String, Integer> map) {
for (Map.Entry<String, Integer> entry : map.entrySet()) {
String str = entry.getKey();
int num = entry.getValue();
System.out.println(str + ":" + num);
}
}
public static void testForLoopSideEffect(Map<String, Integer> map) {
Integer num;
for (Map.Entry<String, Integer> entry : map.entrySet()) {
String str = entry.getKey();
num = entry.getValue();
System.out.println(str + ":" + num);
}
}
public static void testForLoopThrow(Map<String, Integer> map) throws Exception {
for (Map.Entry<String, Integer> 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<String, Integer> map) throws Exception {
for (Map.Entry<String, Integer> entry : map.entrySet()) {
String str = entry.getKey();
Integer num = entry.getValue();
if(num > 0) throw new RuntimeException();
System.out.println(str + ":" + num);
}
}
}
@@ -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";
}
}
@@ -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";
@@ -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
@@ -0,0 +1,9 @@
<html>
<body>
This inspection suggests to replace <code>for(Entry&lt;?,?&gt; entry : map.entrySet()) {...}</code> or
<code>map.entrySet().forEach(entry -&gt; ...)</code> with <code>map.forEach((key, value) -> ...)</code>.
<!-- tooltip end -->
<p>This inspection is available since Java 8 only.</p>
<small>New in 2017.1</small>
</body>
</html>
+5
View File
@@ -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"/>
<localInspection groupPath="Java" language="JAVA" shortName="Java8MapForEach"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.code.style.issues" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.java18api.Java8MapForEachInspection"
displayName="Replace with Map.forEach"/>
<localInspection groupPath="Java" language="JAVA" shortName="ExcessiveLambdaUsage"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.verbose.or.redundant.code.constructs" enabledByDefault="true" level="WARNING"