diff --git a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java index 9a7db8ab87c7..ed633dcd448d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java +++ b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java @@ -21,6 +21,7 @@ import static com.intellij.psi.PsiBinaryExpression.BOOLEAN_OPERATION_TOKENS; */ class ComplexityCalculator { private static final int IDENTIFIER = 10; + private static final int QUALIFIER = 6; private static final int CONSTANT = 1; private static final int OPERATOR = 10; private static final int SIMPLE_OPERATOR = 2; @@ -51,18 +52,19 @@ class ComplexityCalculator { } private int calculateComplexity(@NotNull PsiExpression e) { - if (e instanceof PsiThisExpression) { - return 0; - } if (e instanceof PsiLiteralExpression) { return CONSTANT; } if (e instanceof PsiInstanceOfExpression || - e instanceof PsiTypeCastExpression || - e instanceof PsiClassObjectAccessExpression || - e instanceof PsiQualifiedExpression) { + e instanceof PsiTypeCastExpression) { return IDENTIFIER; } + if (e instanceof PsiClassObjectAccessExpression) { + return QUALIFIER; + } + if (e instanceof PsiQualifiedExpression) { + return ((PsiQualifiedExpression)e).getQualifier() != null ? QUALIFIER : 0; + } if (e instanceof PsiParenthesizedExpression) { return getComplexity(((PsiParenthesizedExpression)e).getExpression()); } @@ -107,6 +109,9 @@ class ComplexityCalculator { if (e instanceof PsiReferenceExpression) { PsiReferenceExpression ref = (PsiReferenceExpression)e; PsiElement resolved = ref.resolve(); + if (resolved instanceof PsiClass) { + return QUALIFIER; + } int w = REFERENCE; if (resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter) { w = IDENTIFIER; @@ -176,9 +181,12 @@ class ComplexityCalculator { if (expression instanceof PsiLiteral) { return true; } - if (expression instanceof PsiReferenceExpression && ((PsiReferenceExpression)expression).getQualifierExpression() == null) { - PsiElement resolved = ((PsiReferenceExpression)expression).resolve(); - return resolved instanceof PsiVariable || resolved instanceof PsiClass; + if (expression instanceof PsiReferenceExpression) { + PsiElement qualifier = ((PsiReferenceExpression)expression).getQualifier(); + if (qualifier instanceof PsiQualifiedExpression || !(qualifier instanceof PsiExpression)) { + PsiElement resolved = ((PsiReferenceExpression)expression).resolve(); + return resolved instanceof PsiVariable || resolved instanceof PsiClass; + } } return false; } diff --git a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java index d3973119b07f..0fbd1428f50e 100644 --- a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java +++ b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java @@ -5,6 +5,7 @@ import com.intellij.codeInspection.dataFlow.CommonDataflow; import com.intellij.codeInspection.dataFlow.DfaFactType; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.ObjectIntHashMap; import com.siyeh.ig.psiutils.ClassUtils; import com.siyeh.ig.psiutils.MethodUtils; @@ -13,6 +14,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Arrays; +import java.util.Set; import static com.intellij.psi.CommonClassNames.*; @@ -29,6 +31,22 @@ import static com.intellij.psi.CommonClassNames.*; class SideEffectCalculator { private final ObjectIntHashMap myCache = new ObjectIntHashMap<>(); + private static final Set SIDE_EFFECTS_FREE_CLASSES = ContainerUtil.set( + JAVA_LANG_BOOLEAN, + JAVA_LANG_CHARACTER, + JAVA_LANG_SHORT, + JAVA_LANG_INTEGER, + JAVA_LANG_LONG, + JAVA_LANG_FLOAT, + JAVA_LANG_DOUBLE, + JAVA_LANG_BYTE, + JAVA_LANG_STRING, + "java.math.BigDecimal", + "java.math.BigInteger", + "java.math.MathContext", + "java.util.UUID", + JAVA_UTIL_OBJECTS); + @Contract("null -> false") boolean mayHaveSideEffect(@Nullable PsiExpression expression) { if (expression == null || @@ -101,14 +119,14 @@ class SideEffectCalculator { } if (resolved instanceof PsiMethod) { PsiMethod method = (PsiMethod)resolved; - return methodHasSideEffect(method); + return methodMayHaveSideEffect(method); } return true; } private boolean calculateCallSideEffect(@NotNull PsiMethodCallExpression call) { PsiMethod method = call.resolveMethod(); - return methodHasSideEffect(method) || + return methodMayHaveSideEffect(method) || calculateSideEffect(call, call.getMethodExpression().getQualifierExpression()); } @@ -140,28 +158,30 @@ class SideEffectCalculator { } @Contract("null -> true") - private static boolean methodHasSideEffect(@Nullable PsiMethod method) { + private static boolean methodMayHaveSideEffect(@Nullable PsiMethod method) { if (method == null) return true; PsiClass psiClass = method.getContainingClass(); if (psiClass == null) return true; String className = psiClass.getQualifiedName(); + if (className == null) return true; if (MethodUtils.isEquals(method) || MethodUtils.isHashCode(method) || MethodUtils.isToString(method) || MethodUtils.isCompareTo(method) || MethodUtils.isComparatorCompare(method) || - ClassUtils.isImmutableClass(psiClass) && !JAVA_IO_FILE.equals(className)) { // methods of File have or are sensitive to side effects + SIDE_EFFECTS_FREE_CLASSES.contains(className)) { return false; } - if (JAVA_UTIL_OBJECTS.equals(className)) { - return false; - } if (JAVA_LANG_MATH.equals(className) || JAVA_LANG_STRICT_MATH.equals(className)) { return "random".equals(method.getName()); // it's the only exception } + if (JAVA_UTIL_COLLECTIONS.equals(className)) { + String name = method.getName(); + return !name.equals("min") && !name.equals("max") && !name.startsWith("unmodifiable"); + } return true; } diff --git a/java/java-tests/testData/inspection/duplicateExpressions/Collections.java b/java/java-tests/testData/inspection/duplicateExpressions/Collections.java new file mode 100644 index 000000000000..3b3d8b0190a6 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/Collections.java @@ -0,0 +1,28 @@ +import java.util.*; + +class C { + void unmodifiableList(List list) { + List u1 = Collections.unmodifiableList(list); + List u2 = Collections.unmodifiableList(list); + } + + void unmodifiableMap(Map map) { + Map u1 = Collections.unmodifiableMap(map); + Map u2 = Collections.unmodifiableMap(map); + } + + void min(List list) { + String s1 = Collections.min(list); + String s2 = Collections.min(list); + } + + void max(List list) { + String s1 = Collections.max(list); + String s2 = Collections.max(list); + } + + void synchronizedList(List list) { + List s1 = Collections.synchronizedList(list); + List s2 = Collections.synchronizedList(list); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateExpressions/DeepNestedClass.java b/java/java-tests/testData/inspection/duplicateExpressions/DeepNestedClass.java new file mode 100644 index 000000000000..294b29c328db --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/DeepNestedClass.java @@ -0,0 +1,26 @@ +class A { + static final int a = 0; + + static class B { + static final int b = 1; + + static class C { + static final int c = 2; + } + } + + void test1() { + int i = A.a; + int j = A.a; + } + + void test2() { + int i = A.B.b; + int j = A.B.b; + } + + void test3() { + int i = A.B.C.c; + int j = A.B.C.c; + } +} diff --git a/java/java-tests/testData/inspection/duplicateExpressions/Qualifier.java b/java/java-tests/testData/inspection/duplicateExpressions/Qualifier.java new file mode 100644 index 000000000000..0a366532e196 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/Qualifier.java @@ -0,0 +1,15 @@ +class A { + int k; + + void testThis() { + int i = this.k; + int j = this.k; + } + + static class B extends A { + void testSuper() { + int i = super.k; + int j = super.k; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt index e7df4ffbee23..38feaf400f28 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt @@ -29,12 +29,15 @@ class DuplicateExpressionsTest : LightCodeInsightFixtureTestCase() { fun testVariableNotModified() = doTest(50) fun testCompositeQualifier() = doTest(40) fun testMethodCallWithSideEffect() = doTest(70) - fun testMathSin() = doTest(40) - fun testMathMax() = doTest(60) + fun testMathSin() = doTest(25) + fun testMathMax() = doTest(45) fun testMathRandom() = doTest(1) fun testVariable() = doTest(1) fun testLambda() = doTest(20) fun testFile() = doTest(1) + fun testCollections() = doTest(1) + fun testDeepNestedClass() = doTest(7) + fun testQualifier() = doTest(1) private fun doTest(threshold: Int = 50) { val oldThreshold = inspection.complexityThreshold