Added settings for 'Collection without initial capacity' and 'Collection.addAll can be replaced with parametrized constructor' inspections (IDEA-148262)

2026-09-27 10:03:11 +07:00 · 2015-11-20 15:59:49 +03:00
parent 3d631c549f
commit 9ef7a5df22
14 changed files with 195 additions and 53 deletions
@@ -20,16 +20,21 @@ import com.intellij.codeInsight.FileModificationService;
 import com.intellij.codeInsight.daemon.QuickFixBundle;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.InvalidDataException;
 import com.intellij.openapi.util.Pair;
+import com.intellij.openapi.util.WriteExternalException;
 import com.intellij.psi.*;
 import com.intellij.psi.util.InheritanceUtil;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.util.NullableFunction;
 import com.intellij.util.containers.ContainerUtil;
+import com.siyeh.ig.performance.CollectionsListSettings;
+import org.jdom.Element;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

+import javax.swing.*;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -40,6 +45,29 @@ import java.util.List;
 public class CollectionAddAllCanBeReplacedWithConstructorInspection extends BaseJavaBatchLocalInspectionTool {
  private final static Logger LOG = Logger.getInstance(CollectionAddAllCanBeReplacedWithConstructorInspection.class);

+  private final CollectionsListSettings mySettings = new CollectionsListSettings() {
+    @Override
+    protected Collection<String> createDefaultSettings() {
+      return DEFAULT_COLLECTION_LIST;
+    }
+  };
+
+  @Override
+  public void writeSettings(@NotNull Element node) throws WriteExternalException {
+    mySettings.writeSettings(node);
+  }
+
+  @Nullable
+  @Override
+  public JComponent createOptionsPanel() {
+    return mySettings.createOptionsPanel();
+  }
+
+  @Override
+  public void readSettings(@NotNull Element node) throws InvalidDataException {
+    mySettings.readSettings(node);
+  }
+
  @NotNull
  @Override
  public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder,
@@ -49,7 +77,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
      @Override
      public void visitMethodCallExpression(PsiMethodCallExpression expression) {
        final String methodName = expression.getMethodExpression().getReferenceName();
-        if ("addAll".equals(methodName)) {
+        if ("addAll".equals(methodName) || "putAll".equals(methodName)) {
          if (expression.getArgumentList().getExpressions().length != 1) {
            return;
          }
@@ -67,7 +95,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
            return;
          }
          final PsiClass variableClass = ((PsiClassType)variableType).resolve();
-          if (variableClass == null || !InheritanceUtil.isInheritor(variableClass, CommonClassNames.JAVA_UTIL_COLLECTION)) {
+          if (variableClass == null) {
            return;
          }
          PsiNewExpression assignmentExpression;
@@ -90,7 +118,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
          final PsiMethod method = expression.resolveMethod();
          if (method != null) {
            //noinspection DialogTitleCapitalization
-            holder.registerProblem(expression, QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.description"),
+            holder.registerProblem(expression, QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.description", methodName),
                                   new ReplaceAddAllWithConstructorFix(assignmentExpression, expression));
          }
        }
@@ -98,7 +126,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
    };
  }

-  private static boolean checkLocalVariableAssignmentOrInitializer(PsiExpression initializer) {
+  private boolean checkLocalVariableAssignmentOrInitializer(PsiExpression initializer) {
    if (!(initializer instanceof PsiNewExpression)) {
      return false;
    }
@@ -108,14 +136,16 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
      return false;
    }
    final PsiClass initializerClass = (PsiClass)classReference.resolve();
-    if (initializerClass == null || !hasProperConstructor(initializerClass)) {
+    if (initializerClass == null ||
+        !mySettings.getCollectionClassesRequiringCapacity().contains(initializerClass.getQualifiedName()) ||
+        !hasProperConstructor(initializerClass)) {
      return false;
    }
    final PsiExpressionList argumentList = newExpression.getArgumentList();
    return argumentList != null && argumentList.getExpressions().length == 0;
  }

-  private static boolean hasProperConstructor(PsiClass psiClass) {
+  private boolean hasProperConstructor(PsiClass psiClass) {
    for (PsiMethod psiMethod : psiClass.getConstructors()) {
      PsiParameterList parameterList = psiMethod.getParameterList();
      if(parameterList.getParametersCount() == 1) {
@@ -123,7 +153,8 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
        PsiTypeElement typeElement = parameter.getTypeElement();
        if (typeElement != null) {
          PsiType type = typeElement.getType();
-          if (InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_UTIL_COLLECTION)) {
+          if (InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_UTIL_COLLECTION) ||
+              InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_UTIL_MAP)) {
            return true;
          }
        }
@@ -132,7 +163,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
    return false;
  }

-  private static Pair<Boolean, PsiNewExpression> isProperAssignmentStatementFound(PsiLocalVariable localVariable, PsiMethodCallExpression addAllExpression) {
+  private Pair<Boolean, PsiNewExpression> isProperAssignmentStatementFound(PsiLocalVariable localVariable, PsiMethodCallExpression addAllExpression) {
    PsiStatement currentStatement = PsiTreeUtil.getParentOfType(addAllExpression, PsiStatement.class);
    final PsiStatement localVariableDefinitionStatement = PsiTreeUtil.getParentOfType(localVariable, PsiStatement.class);
    while (currentStatement != null) {
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "false"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "false"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -1,4 +1,4 @@
-// "Replace 'addAll()' method with parametrized constructor call" "true"
+// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
 import java.lang.String;
 import java.util.ArrayList;
 import java.util.List;
@@ -36,7 +36,7 @@ public class CollectionListModel<T> extends AbstractListModel implements Editabl
  }

  @SuppressWarnings("UnusedParameters")
-  protected CollectionListModel(@NotNull List<T> items, boolean useListAsIs) {
+  public CollectionListModel(@NotNull List<T> items, boolean useListAsIs) {
    myItems = items;
  }

@@ -0,0 +1,113 @@
+/*
+ * Copyright 2000-2015 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.siyeh.ig.performance;
+
+import com.intellij.codeInsight.daemon.QuickFixBundle;
+import com.intellij.codeInspection.ui.ListTable;
+import com.intellij.codeInspection.ui.ListWrappingTableModel;
+import com.intellij.icons.AllIcons;
+import com.intellij.openapi.actionSystem.LangDataKeys;
+import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.InvalidDataException;
+import com.intellij.openapi.util.WriteExternalException;
+import com.intellij.psi.CommonClassNames;
+import com.intellij.ui.AnActionButton;
+import com.intellij.ui.AnActionButtonRunnable;
+import com.intellij.ui.CollectionListModel;
+import com.intellij.ui.ToolbarDecorator;
+import com.intellij.ui.components.JBList;
+import com.intellij.util.SmartList;
+import com.intellij.util.containers.ContainerUtil;
+import com.intellij.util.containers.SortedList;
+import com.intellij.util.ui.EditableModel;
+import com.siyeh.ig.ui.UiUtils;
+import org.jdom.Element;
+import org.jetbrains.annotations.NonNls;
+import org.jetbrains.annotations.NotNull;
+
+import javax.swing.*;
+import java.util.*;
+
+/**
+ * @author Dmitry Batkovich
+ */
+public abstract class CollectionsListSettings {
+  @NonNls
+  public static final SortedSet<String> DEFAULT_COLLECTION_LIST = new TreeSet<String>();
+
+  static {
+    DEFAULT_COLLECTION_LIST.add("java.util.concurrent.ConcurrentHashMap");
+    DEFAULT_COLLECTION_LIST.add("java.util.concurrent.PriorityBlockingQueue");
+    DEFAULT_COLLECTION_LIST.add("java.util.ArrayDeque");
+    DEFAULT_COLLECTION_LIST.add("java.util.ArrayList");
+    DEFAULT_COLLECTION_LIST.add("java.util.HashMap");
+    DEFAULT_COLLECTION_LIST.add("java.util.Hashtable");
+    DEFAULT_COLLECTION_LIST.add("java.util.HashSet");
+    DEFAULT_COLLECTION_LIST.add("java.util.IdentityHashMap");
+    DEFAULT_COLLECTION_LIST.add("java.util.LinkedHashMap");
+    DEFAULT_COLLECTION_LIST.add("java.util.LinkedHashSet");
+    DEFAULT_COLLECTION_LIST.add("java.util.PriorityQueue");
+    DEFAULT_COLLECTION_LIST.add("java.util.Vector");
+    DEFAULT_COLLECTION_LIST.add("java.util.WeakHashMap");
+  }
+
+  private final List<String> myCollectionClassesRequiringCapacity;
+
+  public CollectionsListSettings() {
+    myCollectionClassesRequiringCapacity = new SmartList<String>(createDefaultSettings());
+  }
+
+  public void readSettings(@NotNull Element node) throws InvalidDataException {
+    myCollectionClassesRequiringCapacity.clear();
+    myCollectionClassesRequiringCapacity.addAll(createDefaultSettings());
+    for (Element classElement : node.getChildren("cls")) {
+      final String className = classElement.getText();
+      if (classElement.getAttributeValue("remove", Boolean.FALSE.toString()).equals(Boolean.TRUE.toString())) {
+        myCollectionClassesRequiringCapacity.remove(className);
+      }
+      else {
+        myCollectionClassesRequiringCapacity.add(className);
+      }
+    }
+  }
+
+  public void writeSettings(@NotNull Element node) throws WriteExternalException {
+    final Collection<String> defaultToRemoveSettings = createDefaultSettings();
+    defaultToRemoveSettings.removeAll(myCollectionClassesRequiringCapacity);
+
+    final Set<String> toAdd = new HashSet<String>(myCollectionClassesRequiringCapacity);
+    toAdd.removeAll(createDefaultSettings());
+
+    for (String className : defaultToRemoveSettings) {
+      node.addContent(new Element("cls").setText(className).setAttribute("remove", Boolean.TRUE.toString()));
+    }
+    for (String className : toAdd) {
+      node.addContent(new Element("cls").setText(className));
+    }
+  }
+
+  protected abstract Collection<String> createDefaultSettings();
+
+  public Collection<String> getCollectionClassesRequiringCapacity() {
+    return myCollectionClassesRequiringCapacity;
+  }
+
+  public JComponent createOptionsPanel() {
+    final String title = QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.options.title");
+    final ListTable table = new ListTable(new ListWrappingTableModel(myCollectionClassesRequiringCapacity, title));
+    return UiUtils.createAddRemoveTreeClassChooserPanel(table, title, CommonClassNames.JAVA_LANG_OBJECT);
+  }
+}
@@ -15,37 +15,47 @@
 */
 package com.siyeh.ig.performance;

+import com.intellij.openapi.util.InvalidDataException;
+import com.intellij.openapi.util.WriteExternalException;
 import com.intellij.psi.*;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
-import org.jetbrains.annotations.NonNls;
+import org.jdom.Element;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

+import javax.swing.*;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.TreeSet;

 public class CollectionsMustHaveInitialCapacityInspection
  extends BaseInspection {

-  @NonNls
-  private static final Set<String> collectionClassesRequiringCapacity = new HashSet<String>();
-  static {
-    collectionClassesRequiringCapacity.add("java.util.concurrent.ConcurrentHashMap");
-    collectionClassesRequiringCapacity.add("java.util.concurrent.PriorityBlockingQueue");
-    collectionClassesRequiringCapacity.add("java.util.ArrayDeque");
-    collectionClassesRequiringCapacity.add("java.util.ArrayList");
-    collectionClassesRequiringCapacity.add("java.util.BitSet");
-    collectionClassesRequiringCapacity.add("java.util.HashMap");
-    collectionClassesRequiringCapacity.add("java.util.Hashtable");
-    collectionClassesRequiringCapacity.add("java.util.HashSet");
-    collectionClassesRequiringCapacity.add("java.util.IdentityHashMap");
-    collectionClassesRequiringCapacity.add("java.util.LinkedHashMap");
-    collectionClassesRequiringCapacity.add("java.util.LinkedHashSet");
-    collectionClassesRequiringCapacity.add("java.util.PriorityQueue");
-    collectionClassesRequiringCapacity.add("java.util.Vector");
-    collectionClassesRequiringCapacity.add("java.util.WeakHashMap");
+  private final CollectionsListSettings mySettings = new CollectionsListSettings() {
+    @Override
+    protected Set<String> createDefaultSettings() {
+      final Set<String> classes = new TreeSet<String>(DEFAULT_COLLECTION_LIST);
+      classes.add("java.util.BitSet");
+      return classes;
+    }
+  };
+
+  @Override
+  public void readSettings(@NotNull Element node) throws InvalidDataException {
+    mySettings.readSettings(node);
+  }
+
+  @Override
+  public void writeSettings(@NotNull Element node) throws WriteExternalException {
+    mySettings.writeSettings(node);
+  }
+
+  @Nullable
+  @Override
+  public JComponent createOptionsPanel() {
+    return mySettings.createOptionsPanel();
  }

  @Override
@@ -73,7 +83,7 @@ public class CollectionsMustHaveInitialCapacityInspection
    return new CollectionInitialCapacityVisitor();
  }

-  private static class CollectionInitialCapacityVisitor
+  private class CollectionInitialCapacityVisitor
    extends BaseInspectionVisitor {

    @Override
@@ -90,7 +100,7 @@ public class CollectionsMustHaveInitialCapacityInspection
      registerNewExpressionError(expression);
    }

-    public static boolean isCollectionWithInitialCapacity(@Nullable PsiType type) {
+    private boolean isCollectionWithInitialCapacity(@Nullable PsiType type) {
      if (!(type instanceof PsiClassType)) {
        return false;
      }
@@ -100,7 +110,7 @@ public class CollectionsMustHaveInitialCapacityInspection
        return false;
      }
      final String className = resolved.getQualifiedName();
-      return collectionClassesRequiringCapacity.contains(className);
+      return mySettings.getCollectionClassesRequiringCapacity().contains(className);
    }
  }
 }
@@ -3,18 +3,6 @@
 Reports any attempt to instantiate a new <b>Collection</b> object without specifying
 an initial capacity. If no initial capacity is specified, a default capacity is used, which will rarely be optimal. Failing
 to specify initial capacities for collections may result in performance issues, if space needs to be reallocated and
-memory copied when capacity is exceeded. This inspection checks allocations of the following classes:
-<ul>
-<li><b>java.util.ArrayList</b>
-<li><b>java.util.BitSet</b>
-<li><b>java.util.HashMap</b>
-<li><b>java.util.HashSet</b>
-<li><b>java.util.Hashtable</b>
-<li><b>java.util.Vector</b>
-<li><b>java.util.WeakHashMap</b>
-</ul>
-<!-- tooltip end -->
-<p>
-
+memory copied when capacity is exceeded. This inspection checks allocations of the classes which are listed in inspection settings.
 </body>
 </html>
@@ -1,5 +1,5 @@
 <html>
 <body>
-Inspection reports usages of <b>Collection.addAll()</b> method after instantiation of object using parameter-less constructor.
+Inspection reports usages of <b>Collection.addAll()</b> and <b>Map.putAll()</b> methods after instantiation of object using parameter-less constructor.
 </body>
 </html>
@@ -269,8 +269,8 @@ add.missing.annotation.single.parameter.fix=Add missing annotation parameter ''{
 add.method.qualifier.fix.text=Add qualifier {0} to method

 collection.addall.can.be.replaced.with.constructor.fix.options.title=Classes to check
-collection.addall.can.be.replaced.with.constructor.fix.description='addAll()' method can be replaced with parametrized constructor
-collection.addall.can.be.replaced.with.constructor.fix.title=Replace 'addAll()' method with parametrized constructor call
+collection.addall.can.be.replaced.with.constructor.fix.description=''{0}()'' method can be replaced with parametrized constructor
+collection.addall.can.be.replaced.with.constructor.fix.title=Replace 'addAll/putAll' method with parametrized constructor call

 wrap.long.with.math.to.int.text=Wrap using 'Math.toIntExact()'
 wrap.long.with.math.to.int.parameter.single.text=Wrap parameter using 'Math.toIntExact()'