improve inspection wording, description and highlighting

This commit is contained in:
Bas Leijdekkers
2017-09-12 21:21:44 +02:00
parent 17ebd12891
commit fb5bf4994b
12 changed files with 91 additions and 69 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 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.
@@ -16,7 +16,6 @@
package com.intellij.codeInspection;
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;
@@ -31,6 +30,7 @@ import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.performance.CollectionsListSettings;
import com.siyeh.ig.psiutils.ExpressionUtils;
import org.jdom.Element;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -43,7 +43,6 @@ import java.util.List;
* @author Dmitry Batkovich
*/
public class CollectionAddAllCanBeReplacedWithConstructorInspection extends BaseJavaBatchLocalInspectionTool {
private final static Logger LOG = Logger.getInstance(CollectionAddAllCanBeReplacedWithConstructorInspection.class);
private final CollectionsListSettings mySettings = new CollectionsListSettings() {
@Override
@@ -76,55 +75,58 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
return new JavaElementVisitor() {
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
final String methodName = expression.getMethodExpression().getReferenceName();
if ("addAll".equals(methodName) || "putAll".equals(methodName)) {
if (expression.getArgumentList().getExpressions().length != 1) {
return;
}
final PsiExpression qualifierExpression = expression.getMethodExpression().getQualifierExpression();
if (!(qualifierExpression instanceof PsiReferenceExpression)) {
return;
}
final PsiElement parent = expression.getParent();
if (!(parent instanceof PsiExpressionStatement)) {
return;
}
final PsiElement resolvedReference = ((PsiReferenceExpression)qualifierExpression).resolve();
if (!(resolvedReference instanceof PsiLocalVariable)) {
return;
}
PsiLocalVariable variable = (PsiLocalVariable)resolvedReference;
final PsiType variableType = variable.getType();
if (!(variableType instanceof PsiClassType) || statementHasSubsequentAddAll(parent, variable, methodName)) {
return;
}
final PsiClass variableClass = ((PsiClassType)variableType).resolve();
if (variableClass == null) {
return;
}
PsiNewExpression assignmentExpression;
final Pair<Boolean, PsiNewExpression> pair = isProperAssignmentStatementFound(variable, expression);
if (pair.getFirst()) {
assignmentExpression = pair.getSecond();
if (assignmentExpression == null) {
if (checkLocalVariableAssignmentOrInitializer(variable.getInitializer())) {
assignmentExpression = (PsiNewExpression)variable.getInitializer();
} else {
return;
}
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
final PsiElement nameElement = methodExpression.getReferenceNameElement();
final String methodName = methodExpression.getReferenceName();
if (nameElement == null || !"addAll".equals(methodName) && !"putAll".equals(methodName)) {
return;
}
if (expression.getArgumentList().getExpressions().length != 1) {
return;
}
final PsiExpression qualifierExpression = methodExpression.getQualifierExpression();
if (!(qualifierExpression instanceof PsiReferenceExpression)) {
return;
}
final PsiElement parent = expression.getParent();
if (!(parent instanceof PsiExpressionStatement)) {
return;
}
final PsiElement resolvedReference = ((PsiReferenceExpression)qualifierExpression).resolve();
if (!(resolvedReference instanceof PsiLocalVariable)) {
return;
}
PsiLocalVariable variable = (PsiLocalVariable)resolvedReference;
final PsiType variableType = variable.getType();
if (!(variableType instanceof PsiClassType) || statementHasSubsequentAddAll(parent, variable, methodName)) {
return;
}
final PsiClass variableClass = ((PsiClassType)variableType).resolve();
if (variableClass == null) {
return;
}
PsiNewExpression assignmentExpression;
final Pair<Boolean, PsiNewExpression> pair = isProperAssignmentStatementFound(variable, expression);
if (pair.getFirst()) {
assignmentExpression = pair.getSecond();
if (assignmentExpression == null) {
if (checkLocalVariableAssignmentOrInitializer(variable.getInitializer())) {
assignmentExpression = (PsiNewExpression)variable.getInitializer();
} else {
return;
}
} else {
return;
}
if (!isAddAllReplaceable(expression, assignmentExpression) || !checkUsages(variable, expression, assignmentExpression)) {
return;
}
final PsiMethod method = expression.resolveMethod();
if (method != null) {
//noinspection DialogTitleCapitalization
holder.registerProblem(expression, QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.description", methodName),
new ReplaceAddAllWithConstructorFix(assignmentExpression, expression));
}
} else {
return;
}
if (!isAddAllReplaceable(expression, assignmentExpression) || !checkUsages(variable, expression, assignmentExpression)) {
return;
}
final PsiMethod method = expression.resolveMethod();
if (method != null) {
//noinspection DialogTitleCapitalization
holder.registerProblem(nameElement, QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.description"),
new ReplaceAddAllWithConstructorFix(assignmentExpression, expression, methodName));
}
}
};
@@ -169,7 +171,7 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
return argumentList != null && argumentList.getExpressions().length == 0;
}
private boolean hasProperConstructor(PsiClass psiClass) {
private static boolean hasProperConstructor(PsiClass psiClass) {
for (PsiMethod psiMethod : psiClass.getConstructors()) {
PsiParameterList parameterList = psiMethod.getParameterList();
if(parameterList.getParametersCount() == 1) {
@@ -231,17 +233,26 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Base
private static class ReplaceAddAllWithConstructorFix implements LocalQuickFix {
private final SmartPsiElementPointer<PsiMethodCallExpression> myMethodCallExpression;
private final SmartPsiElementPointer<PsiNewExpression> myAssignmentExpression;
private final String methodName;
private ReplaceAddAllWithConstructorFix(PsiNewExpression assignmentExpression, PsiMethodCallExpression expression) {
ReplaceAddAllWithConstructorFix(PsiNewExpression assignmentExpression, PsiMethodCallExpression expression, String methodName) {
final SmartPointerManager smartPointerManager = SmartPointerManager.getInstance(assignmentExpression.getProject());
myMethodCallExpression = smartPointerManager.createSmartPsiElementPointer(expression);
myAssignmentExpression = smartPointerManager.createSmartPsiElementPointer(assignmentExpression);
this.methodName = methodName;
}
@Nls
@NotNull
@Override
public String getName() {
return QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.name", methodName);
}
@NotNull
@Override
public String getFamilyName() {
return QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.title");
return QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.family.name");
}
@Override
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.Arrays;
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -11,6 +11,6 @@ class C {
s.add(101);
s.add(102);
final List<String> strings = new ArrayList<String>();
string<caret>s.addAll(s);
strings.<caret>addAll(s);
}
}
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -8,6 +8,6 @@ import java.util.Collection;
class C {
void m() {
final Collection<String> strings = new ArrayList<String>();
string<caret>s.addAll(new HashSet<String>());
strings.<caret>addAll(new HashSet<String>());
}
}
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.List;
@@ -8,6 +8,6 @@ class C {
void m() {
final List<String> strings;
strings = new ArrayList<String>();
string<caret>s.addAll(new HashSet<String>());
strings.addAll<caret>(new HashSet<String>());
}
}
@@ -1,4 +1,4 @@
// "Replace 'addAll/putAll' method with parametrized constructor call" "true"
// "Replace 'addAll()' call with parametrized constructor call" "true"
import java.lang.String;
import java.util.ArrayList;
import java.util.Arrays;
@@ -9,6 +9,6 @@ class C {
void m(String s) {
final List<String> strings;
strings = new ArrayList<String>();
string<caret>s.addAll(Arrays.asList(s, ","));
strings.<caret>addAll(Arrays.asList(s, ","));
}
}
@@ -1,5 +1,15 @@
<html>
<body>
Inspection reports usages of <b>Collection.addAll()</b> and <b>Map.putAll()</b> methods after instantiation of object using parameter-less constructor.
Reports <b>Collection.addAll()</b> and <b>Map.putAll()</b> calls after instantiation of a collection using a constructor call without arguments.
Such constructs can be replaced with a single call to a parametrized constructor.
For example:
<pre><code>
Set&lt;String&gt; set = new HashSet&lt;&gt;();
set.addAll(Arrays.asList("alpha", "beta", "gamma"));
</code></pre>
can be replaced with:
<pre><code>
Set&lt;String&gt; set = new HashSet&lt;&gt;(Arrays.asList("alpha", "beta", "gamma"));
</code></pre>
</body>
</html>
@@ -277,8 +277,9 @@ add.method.qualifier.fix.text=Add qualifier ''{0}'' to method
add.method.qualifier.fix.family=Add method qualifier
collection.addall.can.be.replaced.with.constructor.fix.options.title=Classes to check
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
collection.addall.can.be.replaced.with.constructor.fix.description='#ref()' call can be replaced with parametrized constructor call
collection.addall.can.be.replaced.with.constructor.fix.family.name=Replace 'addAll()/putAll()' call with parametrized constructor call
collection.addall.can.be.replaced.with.constructor.fix.name=Replace ''{0}()'' call with parametrized constructor call
add.exception.from.field.initializer.to.constructor.throws.text=Add exception to class {0, choice, 0#default constructor|1#constructor|2#constructors} signature
add.exception.from.field.initializer.to.constructor.throws.family.text=Add exception to class constructors signature
+1 -1
View File
@@ -873,7 +873,7 @@
groupBundle="messages.InspectionsBundle"
groupKey="group.names.performance.issues" enabledByDefault="false" level="WARNING"
implementationClass="com.intellij.codeInspection.CollectionAddAllCanBeReplacedWithConstructorInspection"
displayName="Collection.addAll() can be replaced with parametrized constructor"/>
displayName="Redundant 'Collection.addAll()' call"/>
<localInspection groupPath="Java,Java language level migration aids" language="JAVA" shortName="AnonymousHasLambdaAlternative" displayName="Anonymous type has shorter lambda alternative"
groupKey="group.names.language.level.specific.issues.and.migration.aids8" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.AnonymousHasLambdaAlternativeInspection" />