From e3fb3c143cc1f7a7a6ec8deca81292e6139456cf Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 6 Mar 2018 11:40:16 +0700 Subject: [PATCH] Bug fixes in DefUseInspection IDEA-187758 Variable initializer is redundant false-negative with chained constructors IDEA-187756 Variable initializer is redundant false-positive when the value is read before write --- .../defUse/DefUseInspectionBase.java | 30 +++++++++++-------- .../FieldInitializerChainedConstructor.java | 15 ++++++++++ ...FieldInitializerUsedInMethodReference.java | 15 ++++++++++ .../java/codeInspection/DefUseTest.java | 10 +++++++ 4 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 java/java-tests/testData/inspection/defUse/FieldInitializerChainedConstructor.java create mode 100644 java/java-tests/testData/inspection/defUse/FieldInitializerUsedInMethodReference.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/defUse/DefUseInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/defUse/DefUseInspectionBase.java index b4556ffd6c93..7456669ee8e8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/defUse/DefUseInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/defUse/DefUseInspectionBase.java @@ -1,11 +1,16 @@ // Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection.defUse; +import com.intellij.codeInsight.ExpressionUtil; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; +import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; import com.intellij.codeInsight.daemon.impl.quickfix.RemoveUnusedVariableUtil; import com.intellij.codeInspection.*; import com.intellij.psi.*; +import com.intellij.psi.controlFlow.AnalysisCanceledException; +import com.intellij.psi.controlFlow.ControlFlow; +import com.intellij.psi.controlFlow.ControlFlowUtil; import com.intellij.psi.controlFlow.DefUseUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.ObjectUtils; @@ -16,10 +21,8 @@ import org.jetbrains.annotations.NotNull; import javax.swing.*; import java.awt.*; -import java.util.ArrayList; -import java.util.Collections; +import java.util.*; import java.util.List; -import java.util.Set; public class DefUseInspectionBase extends AbstractBaseJavaLocalInspectionTool { public boolean REPORT_PREFIX_EXPRESSIONS; @@ -66,15 +69,7 @@ public class DefUseInspectionBase extends AbstractBaseJavaLocalInspectionTool { List unusedDefs = DefUseUtil.getUnusedDefs(body, usedVariables); if (unusedDefs != null && !unusedDefs.isEmpty()) { - Collections.sort(unusedDefs, (o1, o2) -> { - int offset1 = o1.getContext().getTextOffset(); - int offset2 = o2.getContext().getTextOffset(); - - if (offset1 == offset2) return 0; - if (offset1 < offset2) return -1; - - return 1; - }); + unusedDefs.sort(Comparator.comparingInt(o -> o.getContext().getTextOffset())); for (DefUseUtil.Info info : unusedDefs) { PsiElement context = info.getContext(); @@ -179,10 +174,21 @@ public class DefUseInspectionBase extends AbstractBaseJavaLocalInspectionTool { return false; } for (PsiMethod constructor : constructors) { + if (JavaHighlightUtil.getChainedConstructors(constructor) != null) continue; final PsiCodeBlock body = constructor.getBody(); if (body == null || !HighlightControlFlowUtil.variableDefinitelyAssignedIn(field, body)) { return false; } + try { + ControlFlow flow = HighlightControlFlowUtil.getControlFlowNoConstantEvaluate(body); + if (ControlFlowUtil.getReadBeforeWrite(flow).stream() + .anyMatch(read -> ExpressionUtil.isEffectivelyUnqualified(read) && read.isReferenceTo(field))) { + return false; + } + } + catch (AnalysisCanceledException e) { + return false; + } } return true; } diff --git a/java/java-tests/testData/inspection/defUse/FieldInitializerChainedConstructor.java b/java/java-tests/testData/inspection/defUse/FieldInitializerChainedConstructor.java new file mode 100644 index 000000000000..080390cc4db7 --- /dev/null +++ b/java/java-tests/testData/inspection/defUse/FieldInitializerChainedConstructor.java @@ -0,0 +1,15 @@ +class Foo { + String s = "foo"; + + Foo(String _s) { + s = _s; + } + + Foo() { + this("foo"); + } + + public static void main(String[] args) { + new Foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/defUse/FieldInitializerUsedInMethodReference.java b/java/java-tests/testData/inspection/defUse/FieldInitializerUsedInMethodReference.java new file mode 100644 index 000000000000..b3c5de322412 --- /dev/null +++ b/java/java-tests/testData/inspection/defUse/FieldInitializerUsedInMethodReference.java @@ -0,0 +1,15 @@ +import java.util.function.Supplier; + +class Foo { + String s = "foo"; + + Foo() { + Supplier fn = s::trim; + s = "bar"; + System.out.println(fn.get()); + } + + public static void main(String[] args) { + new Foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DefUseTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DefUseTest.java index 678a0411ff1b..bae0a2e029a2 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DefUseTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DefUseTest.java @@ -17,7 +17,9 @@ package com.intellij.java.codeInspection; import com.intellij.JavaTestUtil; import com.intellij.codeInspection.defUse.DefUseInspection; +import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; public class DefUseTest extends LightCodeInsightFixtureTestCase { @Override @@ -63,6 +65,14 @@ public class DefUseTest extends LightCodeInsightFixtureTestCase { myFixture.enableInspections(inspection); myFixture.testHighlighting(getTestName(false) + ".java"); } + public void testFieldInitializerUsedInMethodReference() { doTest(); } + public void testFieldInitializerChainedConstructor() { doTest(); } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_10; + } private void doTest() { myFixture.enableInspections(new DefUseInspection());