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 088f942a0f32..215388a4cb2e 100644 --- a/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java @@ -40,29 +40,22 @@ import javax.swing.*; import java.util.*; import java.util.function.Function; +import static com.intellij.psi.CommonClassNames.*; import static com.intellij.util.ObjectUtils.tryCast; +import static com.siyeh.ig.callMatcher.CallMatcher.instanceCall; +import static com.siyeh.ig.callMatcher.CallMatcher.staticCall; public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { - private static final CallMatcher UNMODIFIABLE_SET = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_COLLECTIONS, "unmodifiableSet").parameterCount(1); - private static final CallMatcher UNMODIFIABLE_MAP = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_COLLECTIONS, "unmodifiableMap").parameterCount(1); - private static final CallMatcher UNMODIFIABLE_LIST = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_COLLECTIONS, "unmodifiableList").parameterCount(1); - private static final CallMatcher ARRAYS_AS_LIST = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_ARRAYS, "asList"); - private static final CallMatcher COLLECTION_ADD = - CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "add").parameterCount(1); - private static final CallMatcher MAP_PUT = - CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "put").parameterCount(2); - private static final CallMatcher STREAM_COLLECT = - CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "collect").parameterCount(1); - private static final CallMatcher STREAM_OF = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "of"); - private static final CallMatcher COLLECTORS_TO_SET = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toSet").parameterCount(0); - private static final CallMatcher COLLECTORS_TO_LIST = - CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, "toList").parameterCount(0); + private static final CallMatcher UNMODIFIABLE_SET = staticCall(JAVA_UTIL_COLLECTIONS, "unmodifiableSet").parameterCount(1); + private static final CallMatcher UNMODIFIABLE_MAP = staticCall(JAVA_UTIL_COLLECTIONS, "unmodifiableMap").parameterCount(1); + private static final CallMatcher UNMODIFIABLE_LIST = staticCall(JAVA_UTIL_COLLECTIONS, "unmodifiableList").parameterCount(1); + private static final CallMatcher ARRAYS_AS_LIST = staticCall(JAVA_UTIL_ARRAYS, "asList"); + private static final CallMatcher COLLECTION_ADD = instanceCall(JAVA_UTIL_COLLECTION, "add").parameterCount(1); + private static final CallMatcher MAP_PUT = instanceCall(JAVA_UTIL_MAP, "put").parameterCount(2); + private static final CallMatcher STREAM_COLLECT = instanceCall(JAVA_UTIL_STREAM_STREAM, "collect").parameterCount(1); + private static final CallMatcher STREAM_OF = staticCall(JAVA_UTIL_STREAM_STREAM, "of"); + private static final CallMatcher COLLECTORS_TO_SET = staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toSet").parameterCount(0); + private static final CallMatcher COLLECTORS_TO_LIST = staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toList").parameterCount(0); private static final CallMapper MAPPER = new CallMapper() .register(UNMODIFIABLE_SET, call -> PrepopulatedCollectionModel.fromSet(call.getArgumentList().getExpressions()[0])) @@ -101,7 +94,7 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { InspectionProjectProfileManager.isInformationLevel(getShortName(), call)); PsiElement element = wholeStatement ? call : call.getMethodExpression().getReferenceNameElement(); if(element != null) { - String replacementMethod = model.hasTooManyMapEntries() ? "ofEntries" : "of"; + String replacementMethod = model.hasTooManyMapEntries() ? "ofEntries" : model.myCopy ? "copyOf" : "of"; String fixMessage = InspectionsBundle.message("inspection.collection.factories.fix.name", model.myType, replacementMethod); String inspectionMessage = InspectionsBundle.message("inspection.collection.factories.message", model.myType, replacementMethod); @@ -116,17 +109,23 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { final List myContent; final List myElementsToDelete; final String myType; + final boolean myCopy; final boolean myConstantContent; final boolean myRepeatingKeys; final boolean myHasNulls; PrepopulatedCollectionModel(List content, List delete, String type) { + this(content, delete, type, false); + } + + PrepopulatedCollectionModel(List content, List delete, String type, boolean copy) { myContent = content; myElementsToDelete = delete; myType = type; + myCopy = copy; Map> constants = StreamEx.of(myContent) .cross(ExpressionUtils::nonStructuralChildren).mapValues(ExpressionUtils::computeConstantExpression).distinct().grouping(); - myConstantContent = StreamEx.ofValues(constants).flatCollection(Function.identity()).allMatch(Objects::nonNull); + myConstantContent = !copy && StreamEx.ofValues(constants).flatCollection(Function.identity()).allMatch(Objects::nonNull); myRepeatingKeys = keyExpressions().flatCollection(constants::get).nonNull().distinct(2).findAny().isPresent(); myHasNulls = StreamEx.of(myContent).flatMap(ExpressionUtils::nonStructuralChildren).map(PsiExpression::getType).has(PsiType.NULL); } @@ -160,10 +159,10 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { return fromCollect(call, "List", COLLECTORS_TO_LIST); } if(listDefinition instanceof PsiNewExpression) { - return fromNewExpression((PsiNewExpression)listDefinition, "List", CommonClassNames.JAVA_UTIL_ARRAY_LIST); + return fromNewExpression((PsiNewExpression)listDefinition, "List", JAVA_UTIL_ARRAY_LIST); } if (listDefinition instanceof PsiReferenceExpression) { - return fromVariable((PsiReferenceExpression)listDefinition, "List", CommonClassNames.JAVA_UTIL_ARRAY_LIST, COLLECTION_ADD); + return fromVariable((PsiReferenceExpression)listDefinition, "List", JAVA_UTIL_ARRAY_LIST, COLLECTION_ADD); } return null; } @@ -174,10 +173,10 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { return fromCollect((PsiMethodCallExpression)setDefinition, "Set", COLLECTORS_TO_SET); } if (setDefinition instanceof PsiNewExpression) { - return fromNewExpression((PsiNewExpression)setDefinition, "Set", CommonClassNames.JAVA_UTIL_HASH_SET); + return fromNewExpression((PsiNewExpression)setDefinition, "Set", JAVA_UTIL_HASH_SET); } if (setDefinition instanceof PsiReferenceExpression) { - return fromVariable((PsiReferenceExpression)setDefinition, "Set", CommonClassNames.JAVA_UTIL_HASH_SET, COLLECTION_ADD); + return fromVariable((PsiReferenceExpression)setDefinition, "Set", JAVA_UTIL_HASH_SET, COLLECTION_ADD); } return null; } @@ -185,15 +184,31 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { public static PrepopulatedCollectionModel fromMap(PsiExpression mapDefinition) { mapDefinition = PsiUtil.skipParenthesizedExprDown(mapDefinition); if (mapDefinition instanceof PsiReferenceExpression) { - return fromVariable((PsiReferenceExpression)mapDefinition, "Map", CommonClassNames.JAVA_UTIL_HASH_MAP, MAP_PUT); + return fromVariable((PsiReferenceExpression)mapDefinition, "Map", JAVA_UTIL_HASH_MAP, MAP_PUT); } if (mapDefinition instanceof PsiNewExpression) { - PsiAnonymousClass anonymousClass = ((PsiNewExpression)mapDefinition).getAnonymousClass(); - PsiExpressionList argumentList = ((PsiNewExpression)mapDefinition).getArgumentList(); - if (anonymousClass != null && argumentList != null && argumentList.isEmpty()) { - PsiJavaCodeReferenceElement baseClassReference = anonymousClass.getBaseClassReference(); - if (CommonClassNames.JAVA_UTIL_HASH_MAP.equals(baseClassReference.getQualifiedName())) { - return fromInitializer(anonymousClass, "Map", MAP_PUT); + PsiNewExpression newExpression = (PsiNewExpression)mapDefinition; + PsiAnonymousClass anonymousClass = newExpression.getAnonymousClass(); + PsiExpressionList argumentList = newExpression.getArgumentList(); + if (argumentList != null) { + PsiExpression[] args = argumentList.getExpressions(); + PsiJavaCodeReferenceElement classReference = newExpression.getClassReference(); + if (classReference != null && PsiUtil.isLanguageLevel10OrHigher(mapDefinition) && + JAVA_UTIL_HASH_MAP.equals(classReference.getQualifiedName()) && args.length == 1) { + PsiExpression arg = PsiUtil.skipParenthesizedExprDown(args[0]); + if (arg != null) { + PsiType sourceType = arg.getType(); + PsiType targetType = newExpression.getType(); + if (targetType != null && sourceType != null && sourceType.isAssignableFrom(targetType)) { + return new PrepopulatedCollectionModel(Collections.singletonList(arg), Collections.emptyList(), "Map", true); + } + } + } + if (anonymousClass != null && argumentList.isEmpty()) { + PsiJavaCodeReferenceElement baseClassReference = anonymousClass.getBaseClassReference(); + if (JAVA_UTIL_HASH_MAP.equals(baseClassReference.getQualifiedName())) { + return fromInitializer(anonymousClass, "Map", MAP_PUT); + } } } } @@ -255,7 +270,7 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { PsiExpression[] args = argumentList.getExpressions(); PsiJavaCodeReferenceElement classReference = newExpression.getClassReference(); if (classReference != null && className.equals(classReference.getQualifiedName())) { - return fromArraysAsList(args, type); + return fromCopyConstructor(newExpression, args, type); } PsiAnonymousClass anonymousClass = newExpression.getAnonymousClass(); if (anonymousClass != null && args.length == 0) { @@ -269,11 +284,21 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { } @Nullable - private static PrepopulatedCollectionModel fromArraysAsList(PsiExpression[] args, String type) { + private static PrepopulatedCollectionModel fromCopyConstructor(PsiNewExpression newExpression, + PsiExpression[] args, + String type) { if (args.length == 1) { - PsiMethodCallExpression arg = tryCast(PsiUtil.skipParenthesizedExprDown(args[0]), PsiMethodCallExpression.class); - if (ARRAYS_AS_LIST.test(arg)) { - return new PrepopulatedCollectionModel(Arrays.asList(arg.getArgumentList().getExpressions()), Collections.emptyList(), type); + PsiExpression arg = PsiUtil.skipParenthesizedExprDown(args[0]); + PsiMethodCallExpression call = tryCast(arg, PsiMethodCallExpression.class); + if (ARRAYS_AS_LIST.test(call)) { + return new PrepopulatedCollectionModel(Arrays.asList(call.getArgumentList().getExpressions()), Collections.emptyList(), type); + } + if(arg != null && PsiUtil.isLanguageLevel10OrHigher(arg)) { + PsiType sourceType = arg.getType(); + PsiType targetType = newExpression.getType(); + if (targetType != null && sourceType != null && sourceType.isAssignableFrom(targetType)) { + return new PrepopulatedCollectionModel(Collections.singletonList(arg), Collections.emptyList(), type, true); + } } } return null; @@ -328,7 +353,11 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { String typeArgument = getTypeArguments(call.getType(), model.myType); CommentTracker ct = new CommentTracker(); String replacementText; - if (model.hasTooManyMapEntries()) { + if (model.myCopy) { + assert model.myContent.size() == 1; + replacementText = "java.util." + model.myType + "." + typeArgument + "copyOf(" + model.myContent.get(0).getText() + ")"; + } + else if (model.hasTooManyMapEntries()) { replacementText = StreamEx.ofSubLists(model.myContent, 2) .prepend(Collections.emptyList()) .pairMap((prev, next) -> { @@ -355,14 +384,14 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { @NotNull private static String getTypeArguments(PsiType type, String typeName) { if (typeName.equals("Map")) { - PsiType keyType = PsiUtil.substituteTypeParameter(type, CommonClassNames.JAVA_UTIL_MAP, 0, false); - PsiType valueType = PsiUtil.substituteTypeParameter(type, CommonClassNames.JAVA_UTIL_MAP, 1, false); + PsiType keyType = PsiUtil.substituteTypeParameter(type, JAVA_UTIL_MAP, 0, false); + PsiType valueType = PsiUtil.substituteTypeParameter(type, JAVA_UTIL_MAP, 1, false); if (keyType != null && valueType != null) { return "<" + keyType.getCanonicalText() + "," + valueType.getCanonicalText() + ">"; } } else { - PsiType elementType = PsiUtil.substituteTypeParameter(type, CommonClassNames.JAVA_UTIL_COLLECTION, 0, false); + PsiType elementType = PsiUtil.substituteTypeParameter(type, JAVA_UTIL_COLLECTION, 0, false); if (elementType != null) { return "<" + elementType.getCanonicalText() + ">"; } diff --git a/java/java-impl/src/inspectionDescriptions/Java9CollectionFactory.html b/java/java-impl/src/inspectionDescriptions/Java9CollectionFactory.html index fc6c57a7981a..2ff870ef16c1 100644 --- a/java/java-impl/src/inspectionDescriptions/Java9CollectionFactory.html +++ b/java/java-impl/src/inspectionDescriptions/Java9CollectionFactory.html @@ -1,7 +1,7 @@ This inspection helps to convert unmodifiable collections created before Java 9 to new collection factory methods -like List.of or Set.of. +like List.of or Set.of. Also since Java 10 the conversion to List.copyOf, etc. could be suggested.

Note that Java 9 collection factory methods do not accept null values. Also set elements and map keys are required to be different. It's not always possible to statically check whether original elements are different and not null. Using the checkbox you may enforce diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterCopyOfJava10.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterCopyOfJava10.java new file mode 100644 index 000000000000..f60df0b4db11 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterCopyOfJava10.java @@ -0,0 +1,15 @@ +// "Fix all 'Immutable collection creation can be replaced with collection factory call' problems in file" "true" +import java.util.*; + +class Main { + private final List myList; + private final Map myMap; + private final Set mySet; + + Main(Collection list, Map map, + Set set) { + myList = List.copyOf(list); + myMap = Map.copyOf(map); + mySet = Set.copyOf(set); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOf.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOf.java new file mode 100644 index 000000000000..d8b403b16aa5 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOf.java @@ -0,0 +1,15 @@ +// "Fix all 'Immutable collection creation can be replaced with collection factory call' problems in file" "false" +import java.util.*; + +class Main { + private final List myList; + private final Map myMap; + private final Set mySet; + + Main(Collection list, Map map, + Set set) { + myList = Collections.unmodifiableList(new ArrayList<>(list)); + myMap = Collections.unmodifiableMap(new HashMap<>(map)); + mySet = Collections.unmodifiableSet(new HashSet<>(set)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOfJava10.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOfJava10.java new file mode 100644 index 000000000000..674528333a7f --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeCopyOfJava10.java @@ -0,0 +1,15 @@ +// "Fix all 'Immutable collection creation can be replaced with collection factory call' problems in file" "true" +import java.util.*; + +class Main { + private final List myList; + private final Map myMap; + private final Set mySet; + + Main(Collection list, Map map, + Set set) { + myList = Collections.unmodifiableList(new ArrayList<>(list)); + myMap = Collections.unmodifiableMap(new HashMap<>(map)); + mySet = Collections.unmodifiableSet(new HashSet<>(set)); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/java19api/Java9CollectionFactoryInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/java19api/Java9CollectionFactoryInspectionTest.java index 377eaa2c149b..49abdc86883d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/java19api/Java9CollectionFactoryInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/java19api/Java9CollectionFactoryInspectionTest.java @@ -25,7 +25,7 @@ import org.jetbrains.annotations.NotNull; public class Java9CollectionFactoryInspectionTest extends LightQuickFixParameterizedTestCase { @Override - protected LanguageLevel getLanguageLevel() { + protected LanguageLevel getDefaultLanguageLevel() { return LanguageLevel.JDK_1_9; }