diff --git a/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java b/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java index 6da489d1ae20..7b5d4516c90b 100644 --- a/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java @@ -115,10 +115,18 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool { if (!condition.isKeyAccess(key)) return; PsiExpression value = args[1]; if (condition.isEntrySet() && isUsedAsReference(value, condition)) return; + if (hasMapUsages(condition, value)) return; + if (!LambdaGenerationUtil.canBeUncheckedLambda(value, variable -> condition.getMap().equals(variable))) return; ReplaceWithSingleMapOperation fix = ReplaceWithSingleMapOperation.create("replaceAll", putCall, value); - holder.registerProblem(statement.getFirstChild(), QuickFixBundle.message("java.8.map.api.inspection.description", fix.myMethodName), - fix); + holder.registerProblem(statement.getFirstChild(), + QuickFixBundle.message("java.8.map.api.inspection.description", fix.myMethodName), fix); + } + + private boolean hasMapUsages(@NotNull MapLoopCondition condition, @Nullable PsiExpression value) { + return !VariableAccessUtils.getVariableReferences(condition.getMap(), value).stream() + .map(ExpressionUtils::getCallForQualifier) + .allMatch(call -> condition.isValueAccess(call)); } private boolean isUsedAsReference(@NotNull PsiElement value, @NotNull MapLoopCondition condition) { @@ -389,23 +397,21 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool { @NotNull MapLoopCondition loopCondition, @NotNull PsiExpression value, @NotNull CommentTracker tracker) { + if (value instanceof PsiMethodCallExpression) { + if (loopCondition.isKeyAccess(value)) return factory.createExpressionFromText("(" + kVar + "," + vVar + ") ->" + kVar, value); + if (loopCondition.isValueAccess(value)) return factory.createExpressionFromText("(" + kVar + "," + vVar + ") ->" + vVar, value); + } if (!loopCondition.isEntrySet()) { PsiParameter param = loopCondition.getIterParam(); VariableAccessUtils.getVariableReferences(param, value).forEach(ref -> ExpressionUtils.bindReferenceTo(ref, kVar)); } - else { - if (value instanceof PsiMethodCallExpression) { - if (loopCondition.isKeyAccess(value)) return factory.createExpressionFromText("(" + kVar + "," + vVar + ") ->" + kVar, value); - if (loopCondition.isValueAccess(value)) return factory.createExpressionFromText("(" + kVar + "," + vVar + ") ->" + vVar, value); + Collection calls = PsiTreeUtil.collectElementsOfType(value, PsiMethodCallExpression.class); + for (PsiMethodCallExpression call : calls) { + if (loopCondition.isKeyAccess(call)) { + tracker.replace(call, kVar); } - Collection calls = PsiTreeUtil.collectElementsOfType(value, PsiMethodCallExpression.class); - for (PsiMethodCallExpression call : calls) { - if (loopCondition.isKeyAccess(call)) { - tracker.replace(call, kVar); - } - else if (loopCondition.isValueAccess(call)) { - tracker.replace(call, vVar); - } + else if (loopCondition.isValueAccess(call)) { + tracker.replace(call, vVar); } } return factory.createExpressionFromText("(" + kVar + "," + vVar + ") ->" + tracker.text(value), value); diff --git a/java/java-tests/testData/inspection/java8MapApi/afterReplaceAllKeySetMapUsage.java b/java/java-tests/testData/inspection/java8MapApi/afterReplaceAllKeySetMapUsage.java new file mode 100644 index 000000000000..5bbfe7d1aa99 --- /dev/null +++ b/java/java-tests/testData/inspection/java8MapApi/afterReplaceAllKeySetMapUsage.java @@ -0,0 +1,12 @@ +// "Replace with 'replaceAll' method call" "true" + +import java.util.HashMap; +import java.util.Map; + +public class Main { + public void test() { + Map map = new HashMap<>(); + map.put("foo", "bar"); + map.replaceAll((k, v) -> v); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetMapUsage.java b/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetMapUsage.java new file mode 100644 index 000000000000..8bfdee221ff8 --- /dev/null +++ b/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetMapUsage.java @@ -0,0 +1,14 @@ +// "Replace with 'replaceAll' method call" "true" + +import java.util.HashMap; +import java.util.Map; + +public class Main { + public void test() { + Map map = new HashMap<>(); + map.put("foo", "bar"); + for (String key : map.keySet()) { + map.put(key, map.get(key)); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetNonFinalVarUsage.java b/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetNonFinalVarUsage.java new file mode 100644 index 000000000000..cfff7790b08a --- /dev/null +++ b/java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllKeySetNonFinalVarUsage.java @@ -0,0 +1,13 @@ +// "Replace with 'replaceAll' method call" "false" + +public class Main { + public void test() { + Map map = new HashMap<>(); + map.put("foo", "bar"); + String s = "foo"; + s += "bar"; + for (String k : map.keySet()) { + map.put(k, s); + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/Java8MigrationUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/Java8MigrationUtils.java index 9434e3000b66..157062ff6381 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/Java8MigrationUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/Java8MigrationUtils.java @@ -298,11 +298,14 @@ public class Java8MigrationUtils { private final PsiParameter myIterParam; private final boolean myIsEntrySet; private final PsiReferenceExpression myMapExpression; + private final PsiVariable myMap; - private MapLoopCondition(@NotNull PsiParameter iterParam, boolean isEntrySet, @NotNull PsiReferenceExpression mapExpression) { + private MapLoopCondition(@NotNull PsiParameter iterParam, boolean isEntrySet, + @NotNull PsiReferenceExpression mapExpression, @NotNull PsiVariable map) { myIterParam = iterParam; myIsEntrySet = isEntrySet; myMapExpression = mapExpression; + myMap = map; } /** @@ -317,7 +320,7 @@ public class Java8MigrationUtils { ObjectUtils.tryCast(ControlFlowUtils.stripBraces(statement.getBody()), PsiExpressionStatement.class); if (putStatement == null) return null; PsiMethodCallExpression putCall = extractMapMethodCall(putStatement.getExpression(), "put"); - if (putCall == null || !isMap(putCall.getMethodExpression().getQualifierExpression())) return null; + if (putCall == null || !isMapRef(putCall.getMethodExpression().getQualifierExpression())) return null; return putCall; } @@ -334,10 +337,20 @@ public class Java8MigrationUtils { } /** - * Check if given expression is entry.getValue() call (for entry set based loop). + * Check if given expression is entry.getValue() call (for entry set based loop) or map.get(key) (for key based loop). */ public boolean isValueAccess(@NotNull PsiExpression expression) { - return myIsEntrySet && isParamCall(expression, "getValue"); + if (myIsEntrySet) return isParamCall(expression, "getValue"); + return isGetCall(expression); + } + + private boolean isGetCall(@NotNull PsiExpression expression) { + PsiMethodCallExpression call = ObjectUtils.tryCast(expression, PsiMethodCallExpression.class); + if (call == null) return false; + String name = call.getMethodExpression().getReferenceName(); + if (!"get".equals(name) || !isMapRef(call.getMethodExpression().getQualifierExpression())) return false; + PsiExpression[] args = call.getArgumentList().getExpressions(); + return args.length == 1 && ExpressionUtils.isReferenceTo(args[0], myIterParam); } /** @@ -353,6 +366,10 @@ public class Java8MigrationUtils { return myIterParam; } + public PsiVariable getMap() { + return myMap; + } + public boolean isEntrySet() { return myIsEntrySet; } @@ -364,7 +381,7 @@ public class Java8MigrationUtils { return expectedName.equals(name) && isParamCall(call); } - private boolean isMap(@Nullable PsiElement element) { + private boolean isMapRef(@Nullable PsiElement element) { return element != null && PsiEquivalenceUtil.areElementsEquivalent(myMapExpression, element); } @@ -388,7 +405,9 @@ public class Java8MigrationUtils { private static MapLoopCondition create(@NotNull PsiParameter iterParam, boolean isEntrySet, @Nullable PsiExpression qualifier) { PsiReferenceExpression ref = ObjectUtils.tryCast(qualifier, PsiReferenceExpression.class); if (ref == null) return null; - return new MapLoopCondition(iterParam, isEntrySet, ref); + PsiVariable map = ObjectUtils.tryCast(ref.resolve(), PsiVariable.class); + if (map == null) return null; + return new MapLoopCondition(iterParam, isEntrySet, ref, map); } } } \ No newline at end of file