RedundantCollectionOperation: warn on if(map.containsKey(x)) map.remove(x)

Review ID: IDEA-CR-28150

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<String, Integer> map, String key) {
+      /*contains!!!*/
+      map.remove(/*remove!!!*/key);
+  }
+}

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<String, Integer> map, String key) {
+    if(map.co<caret>ntainsKey(/*contains!!!*/key)) {
+      map.remove(/*remove!!!*/key);
+    }
+  }
+}

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

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());
     }
   }