From eaec99dccee9b0ab752abfa249a090b270a6c1ce Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 27 Jul 2017 19:06:34 +0700 Subject: [PATCH] DataFlowInspection: npe fixes fixed Fixes duplicating NPE expression are disabled when expression has side-effect (not when it's a method call) Parentheses added to generated code when necessary --- .../codeInspection/ReplaceWithTernaryOperatorFix.java | 3 ++- .../dataFlow/DataFlowInspectionBase.java | 3 ++- .../com/intellij/codeInspection/SurroundWithIfFix.java | 3 ++- .../replaceWithTernaryOperator/afterTernary.java | 8 ++++++++ .../replaceWithTernaryOperator/beforeTernary.java | 8 ++++++++ .../quickFix/surroundWithIf/afterTernary.java | 10 ++++++++++ .../quickFix/surroundWithIf/beforeSideEffect.java | 8 ++++++++ .../quickFix/surroundWithIf/beforeTernary.java | 8 ++++++++ .../quickFix/ReplaceWithTernaryOperatorTest.java | 4 +++- .../daemon/quickFix/SurroundWithIfFixTest.java | 4 +++- 10 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeSideEffect.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/ReplaceWithTernaryOperatorFix.java b/java/java-analysis-impl/src/com/intellij/codeInspection/ReplaceWithTernaryOperatorFix.java index 071268b1e720..75bc6aeef3d0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/ReplaceWithTernaryOperatorFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/ReplaceWithTernaryOperatorFix.java @@ -22,6 +22,7 @@ import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; +import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.NotNull; /** @@ -37,7 +38,7 @@ public class ReplaceWithTernaryOperatorFix implements LocalQuickFix { } public ReplaceWithTernaryOperatorFix(@NotNull PsiExpression expressionToAssert) { - myText = expressionToAssert.getText(); + myText = ParenthesesUtils.getText(expressionToAssert, ParenthesesUtils.BINARY_AND_PRECEDENCE); } @NotNull diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index d16599653099..21dcbb5f3137 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -48,6 +48,7 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; import com.siyeh.ig.psiutils.ComparisonUtils; import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.SideEffectChecker; import com.siyeh.ig.psiutils.TypeUtils; import one.util.streamex.StreamEx; import org.jdom.Element; @@ -267,7 +268,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { if (isVolatileFieldReference(qualifier)) { ContainerUtil.addIfNotNull(fixes, createIntroduceVariableFix(qualifier)); } - else if (!isNullLiteral(qualifier) && !(qualifier instanceof PsiMethodCallExpression)) { + else if (!isNullLiteral(qualifier) && !SideEffectChecker.mayHaveSideEffects(qualifier)) { if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4)) { final Project project = qualifier.getProject(); final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory(); diff --git a/java/java-impl/src/com/intellij/codeInspection/SurroundWithIfFix.java b/java/java-impl/src/com/intellij/codeInspection/SurroundWithIfFix.java index 82ecb033158b..3d2c1378fc8c 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SurroundWithIfFix.java +++ b/java/java-impl/src/com/intellij/codeInspection/SurroundWithIfFix.java @@ -27,6 +27,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtilBase; import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.IncorrectOperationException; +import com.siyeh.ig.psiutils.ParenthesesUtils; import com.siyeh.ipp.trivialif.MergeIfAndIntention; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -45,7 +46,7 @@ public class SurroundWithIfFix implements LocalQuickFix { } public SurroundWithIfFix(@NotNull PsiExpression expressionToAssert) { - myText = expressionToAssert.getText(); + myText = ParenthesesUtils.getText(expressionToAssert, ParenthesesUtils.BINARY_AND_PRECEDENCE); } @Override diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java new file mode 100644 index 000000000000..e2f7e3e2b49a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java @@ -0,0 +1,8 @@ +// "Replace with '(b ? null : "foo") != null ?:'" "true" +class A { + void bar(String s) {} + + void foo(boolean b){ + bar((b ? null : "foo") != null ? b ? null : "foo" : null); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java new file mode 100644 index 000000000000..413b3d1b3f95 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java @@ -0,0 +1,8 @@ +// "Replace with '(b ? null : "foo") != null ?:'" "true" +class A { + void bar(String s) {} + + void foo(boolean b){ + bar(b ? null : "foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java new file mode 100644 index 000000000000..862486a4b834 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java @@ -0,0 +1,10 @@ +// "Surround with 'if ((b ? null : "foo") != null)'" "true" +class A { + void bar(String s) {} + + void foo(boolean b){ + if ((b ? null : "foo") != null) { + bar(b ? null : "foo"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeSideEffect.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeSideEffect.java new file mode 100644 index 000000000000..5c0f7390a211 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeSideEffect.java @@ -0,0 +1,8 @@ +// "Surround with 'if ((Math.random() > 0.5 ? null : "foo") != null)'" "false" +class A { + void bar(String s) {} + + void foo(boolean b){ + bar(Math.random() > 0.5 ? null : "foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java new file mode 100644 index 000000000000..6e7bc5706bf5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java @@ -0,0 +1,8 @@ +// "Surround with 'if ((b ? null : "foo") != null)'" "true" +class A { + void bar(String s) {} + + void foo(boolean b){ + bar(b ? null : "foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/ReplaceWithTernaryOperatorTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/ReplaceWithTernaryOperatorTest.java index 1b1f25fa3658..5083b69884b8 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/ReplaceWithTernaryOperatorTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/ReplaceWithTernaryOperatorTest.java @@ -26,7 +26,9 @@ public class ReplaceWithTernaryOperatorTest extends LightQuickFixParameterizedTe @NotNull @Override protected LocalInspectionTool[] configureLocalInspectionTools() { - return new LocalInspectionTool[]{new DataFlowInspection(), new NullableStuffInspection()}; + DataFlowInspection dataFlowInspection = new DataFlowInspection(); + dataFlowInspection.SUGGEST_NULLABLE_ANNOTATIONS = true; + return new LocalInspectionTool[]{dataFlowInspection, new NullableStuffInspection()}; } public void test() throws Exception { diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/SurroundWithIfFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/SurroundWithIfFixTest.java index dc1954bf982c..4cc2f5eff237 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/SurroundWithIfFixTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/SurroundWithIfFixTest.java @@ -25,7 +25,9 @@ public class SurroundWithIfFixTest extends LightQuickFixParameterizedTestCase { @NotNull @Override protected LocalInspectionTool[] configureLocalInspectionTools() { - return new LocalInspectionTool[]{new DataFlowInspection()}; + DataFlowInspection inspection = new DataFlowInspection(); + inspection.SUGGEST_NULLABLE_ANNOTATIONS = true; + return new LocalInspectionTool[]{inspection}; } public void test() throws Exception {