MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-04-20 13:31:28 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-253512: Fixes after review

Browse Source 
GitOrigin-RevId: 0c9d42259d6a5169c449b536aa4e7b982446b10a
This commit is contained in:
Andrey.Cherkasov
2020-11-27 04:15:48 +03:00
committed by intellij-monorepo-bot
parent ecd1b3aafe
commit 95a12db895
19 changed files with 204 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
java/java-impl/src/META-INF/JavaPlugin.xml
View File

@@ -1399,6 +1399,12 @@
bundle="messages.JavaBundle"
key="inspection.redundant.file.creation.display.name"
implementationClass="com.intellij.codeInspection.RedundantFileCreationInspection"/>
<localInspection groupPath="Java" language="JAVA" shortName="SlowAbstractSetRemoveAll"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
bundle="messages.JavaBundle"
key="inspection.slow.abstract.set.remove.all.display.name"
implementationClass="com.intellij.codeInspection.SlowAbstractSetRemoveAllInspection"/>
<localInspection groupPath="Java" language="JAVA" shortName="RedundantUnmodifiable"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.verbose.or.redundant.code.constructs" enabledByDefault="true" level="WARNING"

44
java/java-impl/src/com/intellij/codeInspection/SlowAbstractSetRemoveAllInspection.java
View File

@@ -2,14 +2,20 @@
package com.intellij.codeInspection;
import com.intellij.codeInspection.dataFlow.CommonDataflow;
import com.intellij.codeInspection.dataFlow.SpecialField;
import com.intellij.codeInspection.dataFlow.TypeConstraint;
import com.intellij.codeInspection.dataFlow.types.DfIntegralType;
import com.intellij.codeInspection.dataFlow.types.DfType;
import com.intellij.java.JavaBundle;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.InheritanceUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
@@ -31,19 +37,47 @@ public class SlowAbstractSetRemoveAllInspection extends AbstractBaseJavaLocalIns
public void visitMethodCallExpression(PsiMethodCallExpression call) {
super.visitMethodCallExpression(call);
if (!SET_REMOVE_ALL.test(call)) return;
final PsiExpression arg = call.getArgumentList().getExpressions()[0];
if (!TypeUtils.expressionHasTypeOrSubtype(arg, CommonClassNames.JAVA_UTIL_LIST)) return;
final PsiExpression qualifier = call.getMethodExpression().getQualifierExpression();
final PsiExpression qualifier = ExpressionUtils.getEffectiveQualifier(call.getMethodExpression());
if (qualifier == null) return;
final String s = arg.getText() + ".forEach(" + qualifier.getText() + "::remove)";
final PsiExpression arg = call.getArgumentList().getExpressions()[0];
final TypeConstraint constraint = TypeConstraint.fromDfType(CommonDataflow.getDfType(arg));
final PsiType type = constraint.getPsiType(call.getProject());
final PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(type);
if (aClass == null || !InheritanceUtil.isInheritor(aClass, CommonClassNames.JAVA_UTIL_LIST)) return;
final Long setSize = getMaxSizeOfCollection(qualifier);
if (setSize != null && setSize <= 1) return;
final Long listSize = getMaxSizeOfCollection(arg);
if (listSize != null && listSize <= 2) return;
if (setSize != null && listSize != null && setSize > listSize) return;
final String replacement;
final LocalQuickFix fix;
if (PsiUtil.isLanguageLevel8OrHigher(call) && ExpressionUtils.isVoidContext(call)) {
replacement = ParenthesesUtils.getText(arg, ParenthesesUtils.POSTFIX_PRECEDENCE) + ".forEach(" + qualifier.getText() + "::remove)";
fix = new ReplaceWithListForEachFix(replacement);
} else {
replacement = null;
fix = null;
}
final PsiElement nameElement = call.getMethodExpression().getReferenceNameElement();
if (nameElement == null) return;
holder.registerProblem(call,
JavaBundle.message("inspection.slow.abstract.set.remove.all.description"),
ProblemHighlightType.WARNING,
new ReplaceWithListForEachFix(s));
nameElement.getTextRangeInParent(),
fix);
}
};
}
private static Long getMaxSizeOfCollection(PsiExpression expression) {
final SpecialField lengthField = SpecialField.COLLECTION_SIZE;
final DfType origType = CommonDataflow.getDfType(expression);
final DfType length = lengthField.getFromQualifier(origType);
final DfIntegralType dfType = ObjectUtils.tryCast(length, DfIntegralType.class);
if (dfType == null || dfType.getRange().isEmpty()) return null;
return dfType.getRange().max();
}
private static class ReplaceWithListForEachFix implements LocalQuickFix {
final String myExpressionText;

17
java/java-impl/src/inspectionDescriptions/SlowAbstractSetRemoveAll.html Normal file
View File

@@ -0,0 +1,17 @@
<html>
<body>
The implementation of the <code>'java.util.AbstractSet#removeAll'</code> method determines which is the smaller of this set and the
specified collection, by invoking the size method on each. If this set has fewer elements, then the implementation iterates over
this set, checking each element returned by the iterator in turn to see if it is contained in the specified collection. If it is
so contained, it is removed from this set with the iterator's remove method. If the specified collection has fewer elements, then
the implementation iterates over the specified collection, removing from this set each element returned by the iterator, using this
set's remove method.
<p>It means that if the collection to remove is of equal or larger size than the set, then the implementation of the
<code>'java.util.List#contains'</code> method is called, which in many implementations they will perform costly linear
searches.
</p>
<!-- tooltip end -->
<p><small>New in 2020.3</small></p>
</body>
</html>

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/afterCollection.java Normal file
View File

@@ -0,0 +1,11 @@
// "Replace with 'getRemovals().forEach(source::remove)'" "true"
import java.util.*;
class Test {
native List<String> getRemovals();
void foo(Set<Integer> source) {
getRemovals().forEach(source::remove);
}
}

9
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/afterNonTrivialArg.java Normal file
View File

@@ -0,0 +1,9 @@
// "Replace with '(flag ? removals1 : removals2).forEach(source::remove)'" "true"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals1, List<Integer> removals2, boolean flag) {
(flag ? removals1 : removals2).forEach(source::remove);
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeArrayList.java
View File

@@ -4,6 +4,6 @@ import java.util.*;
class Test {
void foo(Set<Integer> source, ArrayList<Integer> removals) {
source.removeAll(removals)<caret>;
source.removeAll<caret>(removals);
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeCollection.java Normal file
View File

@@ -0,0 +1,11 @@
// "Replace with 'getRemovals().forEach(source::remove)'" "true"
import java.util.*;
class Test {
native List<String> getRemovals();
void foo(Set<Integer> source) {
source.removeAll<caret>(getRemovals());
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeEmptyList.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (removals.isEmpty()) {
source.removeAll<caret>(removals);
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeEmptySet.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (source.isEmpty()) {
source.removeAll<caret>(removals);
}
}
}

2
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeList.java
View File

@@ -4,6 +4,6 @@ import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
source.removeAll(removals)<caret>;
source.removeAll<caret>(removals);
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeListWithAtMostTwoElements.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (removals.isEmpty() || removals.size() == 1 || removals.size() == 2) {
source.removeAll<caret>(removals);
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeListWithOneElement.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (removals.size() == 1) {
source.removeAll<caret>(removals);
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeListWithTwoElement.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (removals.size() == 2) {
source.removeAll<caret>(removals);
}
}
}

9
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeNonTrivialArg.java Normal file
View File

@@ -0,0 +1,9 @@
// "Replace with '(flag ? removals1 : removals2).forEach(source::remove)'" "true"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals1, List<Integer> removals2, boolean flag) {
source.removeAll<caret>(flag ? removals1 : removals2);
}
}

9
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeNonVoidContext.java Normal file
View File

@@ -0,0 +1,9 @@
// "Replace with 'removals.forEach(source::remove)'" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
boolean b = source.removeAll<caret>(removals);
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeSetBiggerThanList.java Normal file
View File

@@ -0,0 +1,11 @@
// "Replace with 'removals.forEach(source::remove)'" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (source.size() > removals.size()) {
source.removeAll<caret>(removals);
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeSetWithAtMostOneElement.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (source.isEmpty() || source.size() == 1) {
source.removeAll<caret>(removals);
}
}
}

11
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/slowAbstractSetRemoveAll/beforeSetWithOneElement.java Normal file
View File

@@ -0,0 +1,11 @@
// "Fix all 'Call to 'set.removeAll(list)' may work slowly' problems in file" "false"
import java.util.*;
class Test {
void foo(Set<Integer> source, List<Integer> removals) {
if (source.size() == 1) {
source.removeAll<caret>(removals);
}
}
}

5
java/openapi/resources/messages/JavaBundle.properties
View File

@@ -1250,8 +1250,9 @@ inspection.condition.covered.by.further.condition.display.name=Condition is cove
inspection.move.field.assignment.to.initializer.display.name=Field assignment can be moved to initializer
inspection.test.failed.line.display.name=Highlight problem line in test
inspection.frequently.used.inheritor.inspection.display.name=Class may extend a commonly used base class
inspection.slow.abstract.set.remove.all.description=Possible quadratic time complexity
inspection.slow.abstract.set.remove.all.fix.family.name=Replace with 'list.forEach(set::remove)'
inspection.slow.abstract.set.remove.all.display.name=Possible quadratic time complexity in AbstractSet#removeAll calls
inspection.slow.abstract.set.remove.all.description=Call to 'set.removeAll(list)' may work slowly
inspection.slow.abstract.set.remove.all.fix.family.name=Use 'Set.remove' instead of 'Set.removeAll'
slice.filter.parse.error.null.filter.not.applicable.for.primitive.type=''null'' filter is not applicable for primitive type {0}
slice.filter.parse.error.not.null.filter.not.applicable.for.primitive.type=''!null'' filter is not applicable for primitive type {0}
slice.filter.parse.error.enum.constant.not.found=Enum constant not found: {0}