From 1a40a9f877d549298e9ea97e3d79ee07e310ee82 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 22 Jan 2018 13:39:09 +0700 Subject: [PATCH] RedundantCollectionOperation: warn on if(map.containsKey(x)) map.remove(x) Review ID: IDEA-CR-28150 --- .../afterContainsKeyRemove.java | 9 ++++++++ .../beforeContainsKeyRemove.java | 10 +++++++++ .../siyeh/InspectionGadgetsBundle.properties | 4 ++-- ...edundantCollectionOperationInspection.java | 22 ++++++++++++++----- 4 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterContainsKeyRemove.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeContainsKeyRemove.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterContainsKeyRemove.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterContainsKeyRemove.java new file mode 100644 index 000000000000..d17a220a7165 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/afterContainsKeyRemove.java @@ -0,0 +1,9 @@ +// "Remove the 'containsKey' check" "true" +import java.util.Map; + +class Test { + void test(Map map, String key) { + /*contains!!!*/ + map.remove(/*remove!!!*/key); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeContainsKeyRemove.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeContainsKeyRemove.java new file mode 100644 index 000000000000..624956affe1f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantCollectionOperation/beforeContainsKeyRemove.java @@ -0,0 +1,10 @@ +// "Remove the 'containsKey' check" "true" +import java.util.Map; + +class Test { + void test(Map map, String key) { + if(map.containsKey(/*contains!!!*/key)) { + map.remove(/*remove!!!*/key); + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index a8339d0f3713..6e9e503f10eb 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -2249,8 +2249,8 @@ inspection.redundant.collection.operation.fix.family.name=Simplify collection op inspection.redundant.collection.operation.problem.arraycopy=Unnecessary collection created to copy an array inspection.redundant.collection.removal.by.index.problem=Removal by index can be replaced with removal by object inspection.redundant.collection.removal.by.index.fix=Use removal by object -inspection.redundant.collection.unnecessary.contains.problem=Unnecessary 'contains' check -inspection.redundant.collection.unnecessary.contains.fix=Remove the 'contains' check +inspection.redundant.collection.unnecessary.contains.problem=Unnecessary ''{0}'' check +inspection.redundant.collection.unnecessary.contains.fix=Remove the ''{0}'' check comments.as.content.option=Comments count as content copy.constructor.misses.field.display.name=Copy constructor misses field diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java index 9f0bce707d87..d6ca19b45523 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantCollectionOperationInspection.java @@ -37,8 +37,12 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "containsAll").parameterTypes(CommonClassNames.JAVA_UTIL_COLLECTION); private static final CallMatcher CONTAINS = instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "contains").parameterTypes(CommonClassNames.JAVA_LANG_OBJECT); - private static final CallMatcher REMOVE = + private static final CallMatcher CONTAINS_KEY = + instanceCall(CommonClassNames.JAVA_UTIL_MAP, "containsKey").parameterTypes(CommonClassNames.JAVA_LANG_OBJECT); + private static final CallMatcher COLLECTION_REMOVE = instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "remove").parameterTypes(CommonClassNames.JAVA_LANG_OBJECT); + private static final CallMatcher MAP_REMOVE = + instanceCall(CommonClassNames.JAVA_UTIL_MAP, "remove").parameterTypes(CommonClassNames.JAVA_LANG_OBJECT); private static final CallMatcher SET_ADD = instanceCall(CommonClassNames.JAVA_UTIL_SET, "add").parameterTypes("E"); private static final CallMatcher REMOVE_BY_INDEX = @@ -51,7 +55,7 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca .register(TO_ARRAY, AsListToArrayHandler::handler) .register(CONTAINS_ALL, ContainsAllSingletonHandler::handler) .register(CONTAINS, SingletonContainsHandler::handler) - .register(CONTAINS, ContainsBeforeAddRemoveHandler::handler) + .register(anyOf(CONTAINS, CONTAINS_KEY), ContainsBeforeAddRemoveHandler::handler) .register(REMOVE_BY_INDEX, RedundantIndexOfHandler::handler) .register(AS_LIST, RedundantAsListForIterationHandler::handler); @@ -141,15 +145,21 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca } private static class ContainsBeforeAddRemoveHandler implements RedundantCollectionOperationHandler { + private final String myName; + + public ContainsBeforeAddRemoveHandler(String name) { + myName = name; + } + @Override public String getProblemName() { - return InspectionGadgetsBundle.message("inspection.redundant.collection.unnecessary.contains.problem"); + return InspectionGadgetsBundle.message("inspection.redundant.collection.unnecessary.contains.problem", myName); } @NotNull @Override public String getFixName() { - return InspectionGadgetsBundle.message("inspection.redundant.collection.unnecessary.contains.fix"); + return InspectionGadgetsBundle.message("inspection.redundant.collection.unnecessary.contains.fix", myName); } @Override @@ -182,7 +192,7 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca public static RedundantCollectionOperationHandler handler(PsiMethodCallExpression call) { PsiExpression qualifier1 = call.getMethodExpression().getQualifierExpression(); if (qualifier1 == null) return null; - CallMatcher wantedMethod = REMOVE; + CallMatcher wantedMethod = anyOf(COLLECTION_REMOVE, MAP_REMOVE); PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); if (parent instanceof PsiExpression && BoolUtils.isNegation((PsiExpression)parent)) { wantedMethod = SET_ADD; @@ -202,7 +212,7 @@ public class RedundantCollectionOperationInspection extends AbstractBaseJavaLoca PsiExpression qualifier2 = thenCall.getMethodExpression().getQualifierExpression(); if (qualifier2 == null || !PsiEquivalenceUtil.areElementsEquivalent(qualifier1, qualifier2)) return null; if (!PsiEquivalenceUtil.areElementsEquivalent(call.getArgumentList(), thenCall.getArgumentList())) return null; - return new ContainsBeforeAddRemoveHandler(); + return new ContainsBeforeAddRemoveHandler(call.getMethodExpression().getReferenceName()); } }