diff --git a/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java b/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java index 73564ed37099..98143aa94c43 100644 --- a/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java @@ -17,7 +17,7 @@ package com.intellij.codeInspection.java19api; import com.intellij.codeInspection.*; import com.intellij.codeInspection.ex.BaseLocalInspectionTool; -import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; +import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; @@ -73,12 +73,15 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { .register(UNMODIFIABLE_LIST, call -> PrepopulatedCollectionModel.fromList(call.getArgumentList().getExpressions()[0])); public boolean IGNORE_NON_CONSTANT = false; + public boolean SUGGEST_MAP_OF_ENTRIES = true; @Nullable @Override public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel(InspectionsBundle.message("inspection.collection.factories.option.ignore.non.constant"), this, - "IGNORE_NON_CONSTANT"); + MultipleCheckboxOptionsPanel panel = new MultipleCheckboxOptionsPanel(this); + panel.addCheckbox(InspectionsBundle.message("inspection.collection.factories.option.ignore.non.constant"), "IGNORE_NON_CONSTANT"); + panel.addCheckbox(InspectionsBundle.message("inspection.collection.factories.option.suggest.ofentries"), "SUGGEST_MAP_OF_ENTRIES"); + return panel; } @NotNull @@ -91,7 +94,7 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { @Override public void visitMethodCallExpression(PsiMethodCallExpression call) { PrepopulatedCollectionModel model = MAPPER.mapFirst(call); - if (model != null && model.isValid()) { + if (model != null && model.isValid(SUGGEST_MAP_OF_ENTRIES)) { ProblemHighlightType type = model.myConstantContent || !IGNORE_NON_CONSTANT ? ProblemHighlightType.GENERIC_ERROR_OR_WARNING : ProblemHighlightType.INFORMATION; @@ -101,8 +104,11 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { InspectionProjectProfileManager.isInformationLevel(getShortName(), call)); PsiElement element = wholeStatement ? call : call.getMethodExpression().getReferenceNameElement(); if(element != null) { - holder.registerProblem(element, InspectionsBundle.message("inspection.collection.factories.message", model.myType), type, - new ReplaceWithCollectionFactoryFix(model.myType)); + String replacementMethod = model.hasTooManyMapEntries() ? "ofEntries" : "of"; + String fixMessage = InspectionsBundle.message("inspection.collection.factories.fix.name", model.myType, replacementMethod); + String inspectionMessage = + InspectionsBundle.message("inspection.collection.factories.message", model.myType, replacementMethod); + holder.registerProblem(element, inspectionMessage, type, new ReplaceWithCollectionFactoryFix(fixMessage)); } } } @@ -128,9 +134,12 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { myHasNulls = StreamEx.of(myContent).flatMap(ExpressionUtils::nonStructuralChildren).map(PsiExpression::getType).has(PsiType.NULL); } - public boolean isValid() { - boolean mapOfTooManyParameters = myType.equals("Map") && myContent.size() > 20; - return !myHasNulls && !myRepeatingKeys && !mapOfTooManyParameters; + boolean isValid(boolean suggestMapOfEntries) { + return !myHasNulls && !myRepeatingKeys && (suggestMapOfEntries || !hasTooManyMapEntries()); + } + + private boolean hasTooManyMapEntries() { + return myType.equals("Map") && myContent.size() > 20; } private StreamEx keyExpressions() { @@ -293,15 +302,17 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { } private static class ReplaceWithCollectionFactoryFix implements LocalQuickFix { - private String myType; + private String myMessage; - public ReplaceWithCollectionFactoryFix(String type) {myType = type;} + public ReplaceWithCollectionFactoryFix(String message) { + myMessage = message; + } @Nls @NotNull @Override public String getName() { - return InspectionsBundle.message("inspection.collection.factories.fix.name", myType); + return myMessage; } @Nls @@ -319,10 +330,23 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { if(model == null) return; String typeArgument = getTypeArguments(call.getType(), model.myType); CommentTracker ct = new CommentTracker(); - String replacementText = StreamEx.of(model.myContent) - .prepend((PsiExpression)null) - .pairMap((prev, next) -> (prev == null ? "" : CommentTracker.commentsBetween(prev, next)) + ct.text(next)) - .joining(",", "java.util." + model.myType + "." + typeArgument + "of(", ")"); + String replacementText; + if (model.hasTooManyMapEntries()) { + replacementText = StreamEx.ofSubLists(model.myContent, 2) + .prepend(Collections.emptyList()) + .pairMap((prev, next) -> { + String prevComment = prev.isEmpty() ? "" : CommentTracker.commentsBetween(prev.get(1), next.get(0)); + String midComment = CommentTracker.commentsBetween(next.get(0), next.get(1)); + return prevComment + "java.util.Map.entry(" + ct.text(next.get(0)) + "," + midComment + ct.text(next.get(1)) + ")"; + }) + .joining(",", "java.util.Map." + typeArgument + "ofEntries(", ")"); + } + else { + replacementText = StreamEx.of(model.myContent) + .prepend((PsiExpression)null) + .pairMap((prev, next) -> (prev == null ? "" : CommentTracker.commentsBetween(prev, next)) + ct.text(next)) + .joining(",", "java.util." + model.myType + "." + typeArgument + "of(", ")"); + } List vars = StreamEx.of(model.myElementsToDelete).map(PsiElement::getParent).select(PsiLocalVariable.class).toList(); model.myElementsToDelete.forEach(ct::delete); diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMap11.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMap11.java new file mode 100644 index 000000000000..2d5b8543b9a7 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMap11.java @@ -0,0 +1,9 @@ +// "Replace with 'Map.ofEntries' call" "true" +import java.util.*; + +public class Test { + public void test() { + Map myMap; + myMap = Map.ofEntries(Map.entry("a", "1"), Map.entry("b", "1"), Map.entry("c", "1"), Map.entry("d", "1"), Map.entry("e", "1"), Map.entry("f", "1"), Map.entry("g", "1"), Map.entry("h", "1"), Map.entry("i", "1"), Map.entry("j", "1"), Map.entry("k", "1")); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMapOfEntriesComments.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMapOfEntriesComments.java new file mode 100644 index 000000000000..4d4cacccabd5 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashMapOfEntriesComments.java @@ -0,0 +1,17 @@ +// "Replace with 'Map.ofEntries' call" "true" +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class Test { + static final Map map; + + static { + //_map.put("a", "b"); + map = Map.ofEntries(Map.entry("c", "d"), Map.entry("e", "f"), Map.entry("g", "h"), Map.entry("i", "j"), Map.entry("k", "l"), + // and m + Map.entry("m", "n"), Map.entry("o", "p"), Map.entry("r", "s"), Map.entry("t", /*uuuu*/"u"), Map.entry("v", "w"), Map.entry("x", /*you*/ "y"), + // q was forgotten! + Map.entry("q", "z")); + } +} diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMap11.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMap11.java index 2fb562b37e1d..f30f666c6409 100644 --- a/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMap11.java +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMap11.java @@ -1,4 +1,4 @@ -// "Replace with 'Map.of' call" "false" +// "Replace with 'Map.ofEntries' call" "true" import java.util.*; public class Test { diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMapOfEntriesComments.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMapOfEntriesComments.java new file mode 100644 index 000000000000..309f91d41a22 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashMapOfEntriesComments.java @@ -0,0 +1,28 @@ +// "Replace with 'Map.ofEntries' call" "true" +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class Test { + static final Map map; + + static { + Map _map = new HashMap<>(); + //_map.put("a", "b"); + _map.put("c", "d"); + _map.put("e", "f"); + _map.put("g", "h"); + _map.put("i", "j"); + _map.put("k", "l"); + // and m + _map.put("m", "n"); + _map.put("o", "p"); + _map.put("r", "s"); + _map.put("t" /*uuuu*/, "u"); + _map.put("v", "w"); + _map.put("x", /*you*/ "y"); + // q was forgotten! + _map.put("q", "z"); + map = Collections.unmodifiableMap(_map); + } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index c2ccfad8ef8e..63e0738dc114 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -814,10 +814,11 @@ inspection.map.foreach.message=Can be replaced with 'Map.forEach' inspection.map.foreach.fix.name=Replace with Map.forEach inspection.map.foreach.option.no.loops=Do not report loops -inspection.collection.factories.message=Can be replaced with ''{0}.of'' call +inspection.collection.factories.message=Can be replaced with ''{0}.{1}'' call inspection.collection.factories.option.ignore.non.constant=Do not warn when content is non-constant +inspection.collection.factories.option.suggest.ofentries=Suggest 'Map.ofEntries' inspection.collection.factories.fix.family.name=Replace with collection factory call -inspection.collection.factories.fix.name=Replace with ''{0}.of'' call +inspection.collection.factories.fix.name=Replace with ''{0}.{1}'' call inspection.null.value.for.optional.message=Null is used for ''Optional'' type in {0} inspection.null.value.for.optional.fix.family.name=Replace with empty Optional method