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 57cb4b8a45f0..51998391a999 100644 --- a/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/java19api/Java9CollectionFactoryInspection.java @@ -17,9 +17,12 @@ package com.intellij.codeInspection.java19api; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.codeInspection.ex.BaseLocalInspectionTool; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; +import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; import com.intellij.psi.controlFlow.DefUseUtil; import com.intellij.psi.util.PsiTreeUtil; @@ -27,16 +30,16 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.callMatcher.CallMapper; import com.siyeh.ig.callMatcher.CallMatcher; -import com.siyeh.ig.psiutils.ClassUtils; -import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.ConstructionUtils; -import com.siyeh.ig.psiutils.MethodCallUtils; +import com.siyeh.ig.psiutils.*; +import one.util.streamex.IntStreamEx; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import javax.swing.*; import java.util.*; +import java.util.function.Function; import static com.intellij.util.ObjectUtils.tryCast; @@ -65,6 +68,14 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { .register(UNMODIFIABLE_SET, call -> PrepopulatedCollectionModel.fromSet(call.getArgumentList().getExpressions()[0])) .register(UNMODIFIABLE_LIST, call -> PrepopulatedCollectionModel.fromList(call.getArgumentList().getExpressions()[0])); + public boolean IGNORE_NON_CONSTANT = false; + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel("Do not warn when content is non-constant", this, "IGNORE_NON_CONSTANT"); + } + @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { @@ -75,10 +86,17 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { @Override public void visitMethodCallExpression(PsiMethodCallExpression call) { PrepopulatedCollectionModel model = MAPPER.mapFirst(call); - if(model != null) { - PsiElement element = call.getMethodExpression().getReferenceNameElement(); + if (model != null && model.isValid()) { + ProblemHighlightType type = model.myConstantContent || !IGNORE_NON_CONSTANT + ? ProblemHighlightType.GENERIC_ERROR_OR_WARNING + : ProblemHighlightType.INFORMATION; + if (type == ProblemHighlightType.INFORMATION && !isOnTheFly) return; + boolean wholeStatement = isOnTheFly && + (type == ProblemHighlightType.INFORMATION || + InspectionProjectProfileManager.isInformationLevel(getShortName(), call)); + PsiElement element = wholeStatement ? call : call.getMethodExpression().getReferenceNameElement(); if(element != null) { - holder.registerProblem(element, "Can be replaced with '"+model.myType+".of' call", + holder.registerProblem(element, "Can be replaced with '" + model.myType + ".of' call", type, new ReplaceWithCollectionFactoryFix(model.myType)); } } @@ -90,11 +108,34 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { final List myContent; final List myElementsToDelete; final String myType; + final boolean myConstantContent; + final boolean myRepeatingKeys; + final boolean myHasNulls; PrepopulatedCollectionModel(List content, List delete, String type) { myContent = content; myElementsToDelete = delete; myType = type; + Map> constants = StreamEx.of(myContent) + .cross(ExpressionUtils::possibleValues).mapValues(ExpressionUtils::computeConstantExpression).distinct().grouping(); + myConstantContent = 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::possibleValues).map(PsiExpression::getType).has(PsiType.NULL); + } + + public boolean isValid() { + return !myHasNulls && !myRepeatingKeys; + } + + private StreamEx keyExpressions() { + switch (myType) { + case "Set": + return StreamEx.of(myContent); + case "Map": + return IntStreamEx.range(0, myContent.size(), 2).elements(myContent); + default: + return StreamEx.empty(); + } } public static PrepopulatedCollectionModel fromList(PsiExpression listDefinition) { @@ -254,7 +295,7 @@ public class Java9CollectionFactoryInspection extends BaseLocalInspectionTool { @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class); + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class, false); if(call == null) return; PrepopulatedCollectionModel model = MAPPER.mapFirst(call); if(model == null) return; diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterAsList.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterAsList.java index 9bc05d254d49..29392eb96ab9 100644 --- a/java/java-tests/testData/inspection/java9CollectionFactory/afterAsList.java +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterAsList.java @@ -4,5 +4,5 @@ import java.util.Collections; import java.util.List; public class Test { - public static final List EVEN = List.of(2, 4, 6, 8, 10); + public static final List EVEN = List.of(2, 4, 6, 8, 10, 2); } diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsList.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsList.java index d14c53688cf8..7f2c1ffa5879 100644 --- a/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsList.java +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsList.java @@ -5,5 +5,5 @@ import java.util.HashSet; import java.util.Set; public class Test { - public static final Set MY_SET = Set.of("a", "b", "c"); + public static final Set MY_SET = Set.of("a", "b", "c", Math.random() > 0.5 ? "d" : Math.random() > 0.5 ? "e" : "d"); } diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsListRepeating.java b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsListRepeating.java new file mode 100644 index 000000000000..a01fa043f554 --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/afterHashSetAsListRepeating.java @@ -0,0 +1,11 @@ +// "Replace with 'Set.of' call" "false" +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +public class Test { + static final String CONST = "b"; + + public static final Set MY_SET = Set.of("a", "b", "c", CONST); +} diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsList.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsList.java index ea64a58b74db..383522202477 100644 --- a/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsList.java +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsList.java @@ -4,5 +4,5 @@ import java.util.Collections; import java.util.List; public class Test { - public static final List EVEN = Collections.unmodifiableList(Arrays.asList(2,4,6,8,10)); + public static final List EVEN = Collections.unmodifiableList(Arrays.asList(2,4,6,8,10,2)); } diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsListNull.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsListNull.java new file mode 100644 index 000000000000..022087b887ee --- /dev/null +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeAsListNull.java @@ -0,0 +1,8 @@ +// "Replace with 'List.of' call" "false" +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +public class Test { + public static final List EVEN = Collections.unmodifiableList(Arrays.asList(2,4,6,8,10,Math.random() > 0.5 ? 12 : null)); +} diff --git a/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashSetAsList.java b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashSetAsList.java index e62f7e66e076..4787fb52a5c1 100644 --- a/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashSetAsList.java +++ b/java/java-tests/testData/inspection/java9CollectionFactory/beforeHashSetAsList.java @@ -5,5 +5,5 @@ import java.util.HashSet; import java.util.Set; public class Test { - public static final Set MY_SET = Collections.unmodifiableSet(new HashSet<>(Arrays.asList("a", "b", "c"))); + public static final Set MY_SET = Collections.unmodifiableSet(new HashSet<>(Arrays.asList("a", "b", "c", Math.random() > 0.5 ? "d" : Math.random() > 0.5 ? "e" : "d"))); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index 9b455cd1eb2a..0b04c00eb39f 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -26,6 +26,7 @@ import com.intellij.psi.util.InheritanceUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; import com.siyeh.HardcodedMethodConstants; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -34,6 +35,7 @@ import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.stream.Stream; public class ExpressionUtils { @NonNls static final Set convertableBoxedClassNames = new HashSet<>(3); @@ -222,11 +224,38 @@ public class ExpressionUtils { return "\"\"".equals(text); } + @Contract("null -> false") public static boolean isNullLiteral(@Nullable PsiExpression expression) { expression = ParenthesesUtils.stripParentheses(expression); return expression != null && PsiType.NULL.equals(expression.getType()); } + /** + * Returns stream of sub-expressions of supplied expression which could be equal (by ==) to resulting + * value of the expression. The expressions in returned stream are guaranteed not to be each other ancestors. + * Also the expression value is guaranteed to be equal to one of returned sub-expressions. + * + *

+ * E.g. for {@code ((a) ? b : (c))} the stream will contain b and c. + *

+ * + * @param expression expression to create a stream from + * @return a new stream + */ + public static Stream possibleValues(@NotNull PsiExpression expression) { + return StreamEx.ofTree(expression, e -> { + if (e instanceof PsiConditionalExpression) { + PsiConditionalExpression ternary = (PsiConditionalExpression)e; + return StreamEx.of(ternary.getThenExpression(), ternary.getElseExpression()).nonNull(); + } + if (e instanceof PsiParenthesizedExpression) { + return StreamEx.ofNullable(((PsiParenthesizedExpression)e).getExpression()); + } + return null; + }).remove(e -> e instanceof PsiConditionalExpression || + e instanceof PsiParenthesizedExpression); + } + public static boolean isZero(@Nullable PsiExpression expression) { if (expression == null) { return false; diff --git a/resources-en/src/inspectionDescriptions/Java9CollectionFactory.html b/resources-en/src/inspectionDescriptions/Java9CollectionFactory.html index 536407a7bdf3..fc6c57a7981a 100644 --- a/resources-en/src/inspectionDescriptions/Java9CollectionFactory.html +++ b/resources-en/src/inspectionDescriptions/Java9CollectionFactory.html @@ -3,6 +3,9 @@ This inspection helps to convert unmodifiable collections created before Java 9 to new collection factory methods like List.of or Set.of. +

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 +the inspection to warn only if original elements are compile-time constants, so the conversion is guaranteed to be correct.

This inspection is available since Java 9 only.

New in 2017.2