[java-inspections] IDEA-257647 Invalid optimization hint for PriorityQueue constructor usage

GitOrigin-RevId: bce3692127bd4115eb81e482ea177b1d2952e46f
This commit is contained in:
Tagir Valeev
2020-12-14 12:03:54 +00:00
committed by intellij-monorepo-bot
parent 2002634b32
commit ec54441d8d
3 changed files with 156 additions and 0 deletions
@@ -2,6 +2,9 @@
package com.intellij.codeInspection;
import com.intellij.codeInsight.daemon.QuickFixBundle;
import com.intellij.codeInspection.dataFlow.CommonDataflow;
import com.intellij.codeInspection.dataFlow.TypeConstraint;
import com.intellij.codeInspection.dataFlow.TypeConstraints;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.InvalidDataException;
import com.intellij.openapi.util.TextRange;
@@ -92,12 +95,38 @@ public class CollectionAddAllCanBeReplacedWithConstructorInspection extends Abst
}
if (assignmentExpression == null || !isAddAllReplaceable(expression, assignmentExpression)) return;
if (mayInheritComparator(args, argType, assignmentExpression)) return;
final PsiMethod method = expression.resolveMethod();
if (method != null) {
holder.registerProblem(nameElement, QuickFixBundle.message("collection.addall.can.be.replaced.with.constructor.fix.description"),
new ReplaceAddAllWithConstructorFix(assignmentExpression, expression, methodName));
}
}
private boolean mayInheritComparator(PsiExpression[] args, PsiType argType, PsiNewExpression assignmentExpression) {
PsiType type = assignmentExpression.getType();
PsiClass collectionType = Objects.requireNonNull(PsiUtil.resolveClassInClassTypeOnly(type));
String name = Objects.requireNonNull(collectionType.getQualifiedName());
switch (name) {
case "java.util.TreeSet":
case "java.util.concurrent.ConcurrentSkipListSet":
// If declared arg type inherits SortedSet, the (SortedSet) copy constructor will be invoked, which inherits the comparator
return InheritanceUtil.isInheritor(argType, "java.util.SortedSet");
case "java.util.TreeMap":
case "java.util.concurrent.ConcurrentSkipListMap":
// If declared arg type inherits SortedMap, the (SortedMap) copy constructor will be invoked, which inherits the comparator
return InheritanceUtil.isInheritor(argType, "java.util.SortedMap");
case "java.util.PriorityQueue":
case "java.util.concurrent.PriorityBlockingQueue":
// Here even (Collection) copy constructor inherits the comparator using runtime type checks, so we should be more conservative
TypeConstraint constraint = TypeConstraint.fromDfType(CommonDataflow.getDfType(args[0]));
PsiClassType sortedSet = JavaPsiFacade.getElementFactory(holder.getProject()).createTypeByFQClassName("java.util.SortedSet");
return constraint.meet(TypeConstraints.instanceOf(sortedSet)) != TypeConstraints.BOTTOM ||
constraint.meet(TypeConstraints.instanceOf(type)) != TypeConstraints.BOTTOM;
default:
return false;
}
}
};
}
@@ -0,0 +1,56 @@
// "Fix all 'Redundant 'Collection.addAll()' call' problems in file" "true"
import java.util.*;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.PriorityBlockingQueue;
public class InheritComparator {
public static void main(String[] args) {
SortedSet<String> c1 = new TreeSet<>(Comparator.reverseOrder());
Collection<String> c2 = new TreeSet<>(Comparator.reverseOrder());
Collection<String> c3 = new HashSet<>();
Collection<String> c4 = new PriorityQueue<>();
Collection<String> c5 = new PriorityBlockingQueue<>();
Collection<String> hs1 = new HashSet<>(c1);
Collection<String> hs2 = new HashSet<>(c2);
Collection<String> hs3 = new HashSet<>(c3);
Collection<String> hs4 = new HashSet<>(c4);
Collection<String> hs5 = new HashSet<>(c5);
Collection<String> ts1 = new TreeSet<>();
ts1.addAll(c1);
Collection<String> ts2 = new TreeSet<>(c2);
Collection<String> ts3 = new TreeSet<>(c3);
Collection<String> ts4 = new TreeSet<>(c4);
Collection<String> ts5 = new TreeSet<>(c5);
Collection<String> pq1 = new PriorityQueue<>();
pq1.addAll(c1);
Collection<String> pq2 = new PriorityQueue<>();
pq2.addAll(c2);
Collection<String> pq3 = new PriorityQueue<>(c3);
Collection<String> pq4 = new PriorityQueue<>();
pq4.addAll(c4);
Collection<String> pq5 = new PriorityQueue<>(c5);
Collection<String> pbq1 = new PriorityBlockingQueue<>();
pbq1.addAll(c1);
Collection<String> pbq2 = new PriorityBlockingQueue<>();
pbq2.addAll(c2);
Collection<String> pbq3 = new PriorityBlockingQueue<>(c3);
Collection<String> pbq4 = new PriorityBlockingQueue<>(c4);
Collection<String> pbq5 = new PriorityBlockingQueue<>();
pbq5.addAll(c5);
SortedMap<String, String> m1 = new TreeMap<>(Comparator.reverseOrder());
Map<String, String> m2 = new TreeMap<>(Comparator.reverseOrder());
Map<String, String> tm1 = new TreeMap<>();
tm1.putAll(m1);
Map<String, String> tm2 = new TreeMap<>(m2);
Map<String, String> cslm1 = new ConcurrentSkipListMap<>();
cslm1.putAll(m1);
Map<String, String> cslm2 = new ConcurrentSkipListMap<>(m2);
}
}
@@ -0,0 +1,71 @@
// "Fix all 'Redundant 'Collection.addAll()' call' problems in file" "true"
import java.util.*;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.PriorityBlockingQueue;
public class InheritComparator {
public static void main(String[] args) {
SortedSet<String> c1 = new TreeSet<>(Comparator.reverseOrder());
Collection<String> c2 = new TreeSet<>(Comparator.reverseOrder());
Collection<String> c3 = new HashSet<>();
Collection<String> c4 = new PriorityQueue<>();
Collection<String> c5 = new PriorityBlockingQueue<>();
Collection<String> hs1 = new HashSet<>();
hs1.<caret>addAll(c1);
Collection<String> hs2 = new HashSet<>();
hs2.addAll(c2);
Collection<String> hs3 = new HashSet<>();
hs3.addAll(c3);
Collection<String> hs4 = new HashSet<>();
hs4.addAll(c4);
Collection<String> hs5 = new HashSet<>();
hs5.addAll(c5);
Collection<String> ts1 = new TreeSet<>();
ts1.addAll(c1);
Collection<String> ts2 = new TreeSet<>();
ts2.addAll(c2);
Collection<String> ts3 = new TreeSet<>();
ts3.addAll(c3);
Collection<String> ts4 = new TreeSet<>();
ts4.addAll(c4);
Collection<String> ts5 = new TreeSet<>();
ts5.addAll(c5);
Collection<String> pq1 = new PriorityQueue<>();
pq1.addAll(c1);
Collection<String> pq2 = new PriorityQueue<>();
pq2.addAll(c2);
Collection<String> pq3 = new PriorityQueue<>();
pq3.addAll(c3);
Collection<String> pq4 = new PriorityQueue<>();
pq4.addAll(c4);
Collection<String> pq5 = new PriorityQueue<>();
pq5.addAll(c5);
Collection<String> pbq1 = new PriorityBlockingQueue<>();
pbq1.addAll(c1);
Collection<String> pbq2 = new PriorityBlockingQueue<>();
pbq2.addAll(c2);
Collection<String> pbq3 = new PriorityBlockingQueue<>();
pbq3.addAll(c3);
Collection<String> pbq4 = new PriorityBlockingQueue<>();
pbq4.addAll(c4);
Collection<String> pbq5 = new PriorityBlockingQueue<>();
pbq5.addAll(c5);
SortedMap<String, String> m1 = new TreeMap<>(Comparator.reverseOrder());
Map<String, String> m2 = new TreeMap<>(Comparator.reverseOrder());
Map<String, String> tm1 = new TreeMap<>();
tm1.putAll(m1);
Map<String, String> tm2 = new TreeMap<>();
tm2.putAll(m2);
Map<String, String> cslm1 = new ConcurrentSkipListMap<>();
cslm1.putAll(m1);
Map<String, String> cslm2 = new ConcurrentSkipListMap<>();
cslm2.putAll(m2);
}
}