diff --git a/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java b/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java new file mode 100644 index 000000000000..eda6cc4b7ea6 --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/OverwrittenKeyInspection.java @@ -0,0 +1,135 @@ +// Copyright 2000-2017 JetBrains s.r.o. +// Use of this source code is governed by the Apache 2.0 license that can be +// found in the LICENSE file. +package com.intellij.codeInspection; + +import com.intellij.codeInsight.PsiEquivalenceUtil; +import com.intellij.openapi.fileEditor.OpenFileDescriptor; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +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 org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import java.util.*; + +import static com.intellij.util.ObjectUtils.tryCast; + +public class OverwrittenKeyInspection extends BaseJavaBatchLocalInspectionTool { + private static final CallMatcher SET_ADD = + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_SET, "add").parameterCount(1); + private static final CallMatcher MAP_PUT = + CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "put").parameterCount(2); + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + Set analyzed = new HashSet<>(); + + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + PsiExpressionStatement statement = tryCast(call.getParent(), PsiExpressionStatement.class); + if (statement == null) return; + CallMatcher myMatcher; + if (SET_ADD.test(call)) { + myMatcher = SET_ADD; + } + 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); + } + } + } + + 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; + } + }; + } + + private static class NavigateToDuplicateFix implements LocalQuickFix { + private final SmartPsiElementPointer myPointer; + + public NavigateToDuplicateFix(PsiExpression arg) { + myPointer = SmartPointerManager.getInstance(arg.getProject()).createSmartPsiElementPointer(arg); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("navigate.to.duplicate.fix"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiExpression element = myPointer.getElement(); + if (element == null) return; + PsiFile file = element.getContainingFile(); + if (file == null) return; + int offset = element.getTextRange().getStartOffset(); + new OpenFileDescriptor(project, file.getVirtualFile(), offset).navigate(true); + } + } +} diff --git a/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java b/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java new file mode 100644 index 000000000000..065789530c44 --- /dev/null +++ b/java/java-tests/testData/inspection/overwrittenKey/OverwrittenKey.java @@ -0,0 +1,42 @@ +import java.util.*; + +class OverwrittenKey { + void fillMap(Map map) { + map.put("a", 1); + map.put("b", 2); + map.put("c", 3); + map.put("d", 4); + map.put("a", 5); + map.put("a", 6); + map.put("e", 7); + map.put("f", 8); + map.put("c", 9); + } + + void fillSet(HashSet set, HashSet set2) { + set.add(5234); + set.add(5235); + set.add(3452); + set.add(3256); + set.add(4635); + set.add(4635); + set.add(3252); + set.add(2352); + set.add(5253); + set.add(5235); + set.add(2145); + set2.add(2145); + } + + enum Test { + A,B,C,D,E + } + + Map map = new HashMap() {{ + put(Test.A, "a"); + put(Test.B, "b"); + this.put(Test.C, "c"); + put(Test.C, "d"); + put(Test.E, "e"); + }}; +} \ 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 new file mode 100644 index 000000000000..be68a237cb16 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/OverwrittenKeyInspectionTest.java @@ -0,0 +1,33 @@ +// Copyright 2000-2017 JetBrains s.r.o. +// Use of this source code is governed by the Apache 2.0 license that can be +// found in the LICENSE file. +package com.intellij.java.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.codeInspection.OverwrittenKeyInspection; +import com.intellij.testFramework.LightProjectDescriptor; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; + +public class OverwrittenKeyInspectionTest extends LightInspectionTestCase { + public void testOverwrittenKey() { + doTest(); + } + + @Override + protected InspectionProfileEntry getInspection() { + return new OverwrittenKeyInspection(); + } + + @Override + protected String getBasePath() { + return JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/overwrittenKey/"; + } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index a12b59653653..12567a37ca6e 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -909,4 +909,7 @@ unused.import.display.name=Unused import inspection.fuse.stream.operations.fix.family.name=Fuse more statements to the Stream API chain inspection.fuse.stream.operations.fix.name=Fuse {0} into the Stream API chain inspection.fuse.stream.operations.message=Stream may be extended replacing {0} -inspection.fuse.stream.operations.display.name=Subsequent steps can be fused into Stream API chain \ No newline at end of file +inspection.fuse.stream.operations.display.name=Subsequent steps can be fused into Stream API chain +inspection.overwritten.key.set.message=Duplicating Set element +inspection.overwritten.key.map.message=Duplicating Map key +navigate.to.duplicate.fix=Navigate to duplicate \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/OverwrittenKey.html b/resources-en/src/inspectionDescriptions/OverwrittenKey.html new file mode 100644 index 000000000000..23d12a19eb8c --- /dev/null +++ b/resources-en/src/inspectionDescriptions/OverwrittenKey.html @@ -0,0 +1,15 @@ + + +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: +
+  map.put("A", 1);
+  map.put("B", 2);
+  map.put("C", 3);
+  map.put("D", 4);
+  map.put("A", 5); // duplicating key "A", overwrites previously written entry
+
+ +

New in 2017.3

+ + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index e4b398603da2..0540219ae376 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -883,6 +883,11 @@ groupKey="group.names.code.style.issues" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.CollectionAddAllCanBeReplacedWithConstructorInspection" displayName="Redundant 'Collection.addAll()' call"/> +