diff --git a/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java b/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java index eda6cc4b7ea6..9be326a3748d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java @@ -12,10 +12,13 @@ import com.intellij.psi.util.PsiUtil; import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.VariableAccessUtils; +import one.util.streamex.IntStreamEx; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import java.util.*; +import java.util.function.Function; import static com.intellij.util.ObjectUtils.tryCast; @@ -24,88 +27,126 @@ public class OverwrittenKeyInspection extends BaseJavaBatchLocalInspectionTool { CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_SET, "add").parameterCount(1); private static final CallMatcher MAP_PUT = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "put").parameterCount(2); + private static final CallMatcher SET_OF = + CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_SET, "of"); + private static final CallMatcher MAP_OF = + CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_MAP, "of"); + private static final CallMatcher MAP_OF_ENTRIES = + CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_MAP, "ofEntries"); + private static final CallMatcher MAP_ENTRY = + CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_MAP, "entry"); @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { - return new JavaElementVisitor() { - Set analyzed = new HashSet<>(); + return new OverwrittenKeyVisitor(holder, isOnTheFly); + } - @Override - public void visitMethodCallExpression(PsiMethodCallExpression call) { - PsiExpressionStatement statement = tryCast(call.getParent(), PsiExpressionStatement.class); - if (statement == null) return; - CallMatcher myMatcher; + private static class OverwrittenKeyVisitor extends JavaElementVisitor { + private final ProblemsHolder myHolder; + private final boolean myIsOnTheFly; + private final Set analyzed = new HashSet<>(); + + public OverwrittenKeyVisitor(ProblemsHolder holder, boolean isOnTheFly) { + myHolder = holder; + myIsOnTheFly = isOnTheFly; + } + + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + PsiExpressionStatement statement = tryCast(call.getParent(), PsiExpressionStatement.class); + if (statement != null) { if (SET_ADD.test(call)) { - myMatcher = SET_ADD; + processCallSequence(call, statement, SET_ADD, InspectionsBundle.message("inspection.overwritten.key.set.message")); } else if (MAP_PUT.test(call)) { - myMatcher = MAP_PUT; - } - else { - return; - } - if (!analyzed.add(call)) return; - - Object key = getKey(call); - if (key == null) return; - PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(call.getMethodExpression())); - if (qualifier == null) return; - PsiVariable qualifierVar = - qualifier instanceof PsiReferenceExpression ? tryCast(((PsiReferenceExpression)qualifier).resolve(), PsiVariable.class) : null; - Map> map = new HashMap<>(); - map.computeIfAbsent(key, k -> new ArrayList<>()).add(call); - while (true) { - PsiExpressionStatement nextStatement = - tryCast(PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class), PsiExpressionStatement.class); - if (nextStatement == null) break; - PsiMethodCallExpression nextCall = tryCast(nextStatement.getExpression(), PsiMethodCallExpression.class); - if (!myMatcher.test(nextCall)) break; - PsiExpression nextQualifier = - PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(nextCall.getMethodExpression())); - if (nextQualifier == null || !PsiEquivalenceUtil.areElementsEquivalent(qualifier, nextQualifier)) break; - analyzed.add(nextCall); - if (qualifierVar != null && VariableAccessUtils.variableIsUsed(qualifierVar, nextCall.getArgumentList())) break; - Object nextKey = getKey(nextCall); - if (nextKey != null) { - map.computeIfAbsent(nextKey, k -> new ArrayList<>()).add(nextCall); - } - statement = nextStatement; - } - for (List calls : map.values()) { - if (calls.size() < 2) continue; - for (int i = 0; i < calls.size(); i++) { - PsiMethodCallExpression dup = calls.get(i); - PsiExpression arg = dup.getArgumentList().getExpressions()[0]; - LocalQuickFix fix = null; - if (isOnTheFly) { - PsiExpression nextArg = calls.get((i + 1) % calls.size()).getArgumentList().getExpressions()[0]; - fix = new NavigateToDuplicateFix(nextArg); - } - String message = myMatcher == SET_ADD ? - InspectionsBundle.message("inspection.overwritten.key.set.message") : - InspectionsBundle.message("inspection.overwritten.key.map.message"); - holder.registerProblem(arg, message, fix); - } + processCallSequence(call, statement, MAP_PUT, InspectionsBundle.message("inspection.overwritten.key.map.message")); } } - - private Object getKey(PsiMethodCallExpression call) { - PsiExpression key = call.getArgumentList().getExpressions()[0]; - Object constant = ExpressionUtils.computeConstantExpression(key); - if (constant != null) { - return constant; - } - if (key instanceof PsiReferenceExpression) { - PsiField field = tryCast(((PsiReferenceExpression)key).resolve(), PsiField.class); - if (field instanceof PsiEnumConstant || - field != null && field.hasModifierProperty(PsiModifier.FINAL) && field.hasModifierProperty(PsiModifier.STATIC)) { - return field; - } - } - return null; + if(SET_OF.test(call)) { + findDuplicates(call.getArgumentList().getExpressions(), InspectionsBundle.message("inspection.overwritten.key.set.message")); } - }; + else if (MAP_OF.test(call)) { + PsiExpression[] args = call.getArgumentList().getExpressions(); + findDuplicates(IntStreamEx.range(0, args.length, 2).elements(args).toArray(PsiExpression[]::new), + InspectionsBundle.message("inspection.overwritten.key.map.message")); + } + else if (MAP_OF_ENTRIES.test(call)) { + PsiExpression[] keys = StreamEx.of(call.getArgumentList().getExpressions()).map(PsiUtil::skipParenthesizedExprDown) + .select(PsiMethodCallExpression.class).filter(MAP_ENTRY).map(entryCall -> entryCall.getArgumentList().getExpressions()[0]) + .toArray(PsiExpression[]::new); + findDuplicates(keys, InspectionsBundle.message("inspection.overwritten.key.map.message")); + } + } + + private void findDuplicates(PsiExpression[] expressions, String message) { + Map> groups = StreamEx.of(expressions).mapToEntry(OverwrittenKeyVisitor::getKey, Function.identity()) + .nonNullKeys().grouping(); + registerDuplicates(message, groups); + } + + private void processCallSequence(PsiMethodCallExpression call, PsiExpressionStatement statement, CallMatcher myMatcher, String message) { + if (!analyzed.add(call)) return; + + PsiExpression arg = call.getArgumentList().getExpressions()[0]; + Object key = getKey(arg); + if (key == null) return; + PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(call.getMethodExpression())); + if (qualifier == null) return; + PsiVariable qualifierVar = + qualifier instanceof PsiReferenceExpression ? tryCast(((PsiReferenceExpression)qualifier).resolve(), PsiVariable.class) : null; + Map> map = new HashMap<>(); + map.computeIfAbsent(key, k -> new ArrayList<>()).add(arg); + while (true) { + PsiExpressionStatement nextStatement = + tryCast(PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class), PsiExpressionStatement.class); + if (nextStatement == null) break; + PsiMethodCallExpression nextCall = tryCast(nextStatement.getExpression(), PsiMethodCallExpression.class); + if (!myMatcher.test(nextCall)) break; + PsiExpression nextQualifier = + PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(nextCall.getMethodExpression())); + if (nextQualifier == null || !PsiEquivalenceUtil.areElementsEquivalent(qualifier, nextQualifier)) break; + analyzed.add(nextCall); + if (qualifierVar != null && VariableAccessUtils.variableIsUsed(qualifierVar, nextCall.getArgumentList())) break; + PsiExpression nextArg = nextCall.getArgumentList().getExpressions()[0]; + Object nextKey = getKey(nextArg); + if (nextKey != null) { + map.computeIfAbsent(nextKey, k -> new ArrayList<>()).add(nextArg); + } + statement = nextStatement; + } + registerDuplicates(message, map); + } + + private void registerDuplicates(String message, Map> map) { + for (List args : map.values()) { + if (args.size() < 2) continue; + for (int i = 0; i < args.size(); i++) { + PsiExpression arg = args.get(i); + LocalQuickFix fix = null; + if (myIsOnTheFly) { + PsiExpression nextArg = args.get((i + 1) % args.size()); + fix = new NavigateToDuplicateFix(nextArg); + } + myHolder.registerProblem(arg, message, fix); + } + } + } + + private static Object getKey(PsiExpression key) { + Object constant = ExpressionUtils.computeConstantExpression(key); + if (constant != null) { + return constant; + } + if (key instanceof PsiReferenceExpression) { + PsiField field = tryCast(((PsiReferenceExpression)key).resolve(), PsiField.class); + if (field instanceof PsiEnumConstant || + field != null && field.hasModifierProperty(PsiModifier.FINAL) && field.hasModifierProperty(PsiModifier.STATIC)) { + return field; + } + } + return null; + } } private static class NavigateToDuplicateFix implements LocalQuickFix { diff --git a/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java b/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java index 065789530c44..33a5451409fe 100644 --- a/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java +++ b/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java @@ -39,4 +39,11 @@ class OverwrittenKey { put(Test.C, "d"); put(Test.E, "e"); }}; + + void java9() { + Set set = Set.of("a", "b", "c", "a"); + Map map = Map.of("a", "a", "b", "b", "c", "b", "b", "d"); + Map map2 = Map.ofEntries(Map.entry("a", "a"), Map.entry("b", "b"), + Map.entry("c", "b"), Map.entry("b", "d")); + } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/OverwrittenKeyInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/OverwrittenKeyInspectionTest.java index be68a237cb16..c5fbd2d75c14 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/OverwrittenKeyInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/OverwrittenKeyInspectionTest.java @@ -28,6 +28,6 @@ public class OverwrittenKeyInspectionTest extends LightInspectionTestCase { @NotNull @Override protected LightProjectDescriptor getProjectDescriptor() { - return JAVA_8; + return JAVA_9; } } diff --git a/resources-en/src/inspectionDescriptions/OverwrittenKey.html b/resources-en/src/inspectionDescriptions/OverwrittenKey.html index 23d12a19eb8c..9608aa7c5577 100644 --- a/resources-en/src/inspectionDescriptions/OverwrittenKey.html +++ b/resources-en/src/inspectionDescriptions/OverwrittenKey.html @@ -1,7 +1,7 @@ -Warns if Map key or Set element was overwritten in the sequence of add/put calls. This usually -occurs due to copy-paste error. Example: +Warns if Map key or Set element was overwritten in the sequence of add/put calls or using a Java 9 factory method +like Set.of (which will result in runtime exception). This usually occurs due to copy-paste error. Example: 
   map.put("A", 1);
   map.put("B", 2);