From 86bc525eadf5e10f9a065e337e50f71e8c7918ef Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 13 Feb 2019 17:28:41 +0300 Subject: [PATCH] Java: Improve the inspection "Multiple occurrences of the same expression" (IDEA-207247) --- .../ComplexityCalculator.java | 7 ++++--- .../DuplicateExpressionsInspection.java | 5 +++-- .../SideEffectCalculator.java | 2 +- .../duplicateExpressions/Lambda.java | 20 +++++++++++++++++++ .../duplicateExpressions/MathMax.java | 6 ++++++ .../duplicateExpressions/MathRandom.java | 6 ++++++ .../duplicateExpressions/MathSin.java | 6 ++++++ .../duplicateExpressions/Variable.java | 18 +++++++++++++++++ .../DuplicateExpressionsTest.kt | 5 +++++ 9 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/duplicateExpressions/Lambda.java create mode 100644 java/java-tests/testData/inspection/duplicateExpressions/MathMax.java create mode 100644 java/java-tests/testData/inspection/duplicateExpressions/MathRandom.java create mode 100644 java/java-tests/testData/inspection/duplicateExpressions/MathSin.java create mode 100644 java/java-tests/testData/inspection/duplicateExpressions/Variable.java 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 6c6b767a6d6c..9a7db8ab87c7 100644 --- a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java +++ b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/ComplexityCalculator.java @@ -172,12 +172,13 @@ class ComplexityCalculator { /** * Quick check to filter out the obvious things early */ - static boolean isDefinitelySimple(@Nullable PsiExpression expression, int threshold) { + static boolean isDefinitelySimple(@Nullable PsiExpression expression) { if (expression instanceof PsiLiteral) { - return CONSTANT < threshold; + return true; } if (expression instanceof PsiReferenceExpression && ((PsiReferenceExpression)expression).getQualifierExpression() == null) { - return REFERENCE < threshold; + 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/DuplicateExpressionsInspection.java b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/DuplicateExpressionsInspection.java index ca6a40d16c0e..54a6fa0d5bf9 100644 --- a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/DuplicateExpressionsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/DuplicateExpressionsInspection.java @@ -64,8 +64,9 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool { } public void visitExpressionImpl(PsiExpression expression) { - if (ComplexityCalculator.isDefinitelySimple(expression, complexityThreshold) || - SideEffectCalculator.isDefinitelyWithSideEffect(expression)) { + if (ComplexityCalculator.isDefinitelySimple(expression) || + SideEffectCalculator.isDefinitelyWithSideEffect(expression) || + expression instanceof PsiLambdaExpression) { return; } DuplicateExpressionsContext context = DuplicateExpressionsContext.getOrCreateContext(expression, session); 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 c7160ede92b0..0807193889b3 100644 --- a/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java +++ b/java/java-impl/src/com/intellij/codeInspection/duplicateExpressions/SideEffectCalculator.java @@ -92,7 +92,7 @@ class SideEffectCalculator { return true; } PsiElement resolved = ref.resolve(); - if (resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter) { + if (resolved instanceof PsiLocalVariable || resolved instanceof PsiParameter || resolved instanceof PsiClass) { return false; } if (resolved instanceof PsiField) { diff --git a/java/java-tests/testData/inspection/duplicateExpressions/Lambda.java b/java/java-tests/testData/inspection/duplicateExpressions/Lambda.java new file mode 100644 index 000000000000..fea2b165f069 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/Lambda.java @@ -0,0 +1,20 @@ +class C { + void test() { + accept(n -> n * n); + accept(n -> n * n); + + accept(n -> { + int a = n * n; + int b = n * n; + return a + b; + }); + } + + void accept(I i) { + i.f(0); + } + + interface I { + int f(int n); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateExpressions/MathMax.java b/java/java-tests/testData/inspection/duplicateExpressions/MathMax.java new file mode 100644 index 000000000000..c98b3812c59f --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/MathMax.java @@ -0,0 +1,6 @@ +class C { + void test(int i, int j) { + int a = Math.max(i, j); + int b = Math.max(i, j); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateExpressions/MathRandom.java b/java/java-tests/testData/inspection/duplicateExpressions/MathRandom.java new file mode 100644 index 000000000000..57fe22b69d50 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/MathRandom.java @@ -0,0 +1,6 @@ +class C { + void test() { + double x = Math.random(); + double y = Math.random(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateExpressions/MathSin.java b/java/java-tests/testData/inspection/duplicateExpressions/MathSin.java new file mode 100644 index 000000000000..2b117e62466f --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/MathSin.java @@ -0,0 +1,6 @@ +class C { + void test() { + double x = Math.sin(4); + double y = Math.sin(4); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/duplicateExpressions/Variable.java b/java/java-tests/testData/inspection/duplicateExpressions/Variable.java new file mode 100644 index 000000000000..a37e4da23ee9 --- /dev/null +++ b/java/java-tests/testData/inspection/duplicateExpressions/Variable.java @@ -0,0 +1,18 @@ +class C { + final int field; + + C(int i) { + field = i; + } + + void test(int param) { + int a = field; + int b = field; + + int c = param; + int d = param; + + int e = field + param; + int f = field + param; + } +} \ 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 1889af0b2af0..6122b3c937be 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DuplicateExpressionsTest.kt @@ -29,6 +29,11 @@ class DuplicateExpressionsTest : LightCodeInsightFixtureTestCase() { fun testVariableNotModified() = doTest(50) fun testCompositeQualifier() = doTest(40) fun testMethodCallWithSideEffect() = doTest(70) + fun testMathSin() = doTest(40) + fun testMathMax() = doTest(60) + fun testMathRandom() = doTest(1) + fun testVariable() = doTest(1) + fun testLambda() = doTest(20) private fun doTest(threshold: Int = 50) { val oldThreshold = inspection.complexityThreshold