From d7e1bdf5b84b41febfe940a0cc8465e09a6786f5 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 23 Aug 2017 17:09:29 +0700 Subject: [PATCH] CommonDataflow: annotate method calls; IDEA-177798 Simplifiable equals expression: support non-constant argument --- .../dataFlow/CommonDataflow.java | 21 +++++++++++ .../siyeh/InspectionGadgetsBundle.properties | 1 + ...implifiableEqualsExpressionInspection.java | 37 +++++++++++++------ .../SimplifiableEqualsExpression.html | 6 ++- .../SimplifiableEqualsExpression.java | 26 +++++++++++++ ...ifiableEqualsExpressionInspectionTest.java | 8 ++++ 6 files changed, 87 insertions(+), 12 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java index 2db6fe5abd30..3dc630827bc4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java @@ -15,13 +15,16 @@ */ package com.intellij.codeInspection.dataFlow; +import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction; import com.intellij.codeInspection.dataFlow.instructions.PushInstruction; +import com.intellij.codeInspection.dataFlow.value.DfaConstValue; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.psi.*; import com.intellij.psi.util.CachedValueProvider; import com.intellij.psi.util.CachedValuesManager; import com.intellij.psi.util.PsiModificationTracker; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ObjectUtils; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.Nullable; @@ -46,6 +49,7 @@ public class CommonDataflow { private static DataflowResult runDFA(@Nullable PsiElement block) { if (block == null) return null; DataFlowRunner runner = new DataFlowRunner(false, true); + DfaConstValue fail = runner.getFactory().getConstFactory().getContractFail(); DataflowResult dfr = new DataflowResult(); StandardInstructionVisitor visitor = new StandardInstructionVisitor() { @Override @@ -59,6 +63,23 @@ public class CommonDataflow { } return states; } + + @Override + public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, + DataFlowRunner runner, + DfaMemoryState memState) { + DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState); + PsiExpression context = ObjectUtils.tryCast(instruction.getContext(), PsiExpression.class); + if (context != null) { + for (DfaInstructionState state : states) { + DfaValue value = state.getMemoryState().peek(); + if(value != fail) { + dfr.add(context, (DfaMemoryStateImpl)state.getMemoryState()); + } + } + } + return states; + } }; RunnerResult result = runner.analyzeMethod(block, visitor); return result == RunnerResult.OK ? dfr : null; diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index 09f77533730d..316f4bc5e186 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1908,6 +1908,7 @@ empty.directory.display.name=Empty directory empty.directories.problem.descriptor=Empty directory {0} empty.directories.only.under.source.roots.option=Only report empty directories located under a source folder empty.directories.delete.quickfix=Delete empty directory ''{0}'' +simplifiable.equals.expression.option.non.constant=Report equals with non-constant not-null argument simplifiable.equals.expression.display.name=Unnecessary 'null' check before 'equals()' call simplifiable.equals.expression.problem.descriptor=Unnecessary ''null'' check before ''{0}()'' call #loc simplifiable.equals.expression.quickfix=Flip ''.{0}()'' and remove unnecessary ''null'' check diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspection.java index 45bddf71a939..12fd9f86eccb 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspection.java @@ -17,6 +17,9 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.CleanupLocalInspectionTool; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.dataFlow.CommonDataflow; +import com.intellij.codeInspection.dataFlow.DfaFactType; +import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; @@ -29,11 +32,25 @@ import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.SideEffectChecker; +import com.siyeh.ig.psiutils.VariableAccessUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; public class SimplifiableEqualsExpressionInspection extends BaseInspection implements CleanupLocalInspectionTool { + public boolean REPORT_NON_CONSTANT = true; + + @Nullable + @Override + public JComponent createOptionsPanel() { + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("simplifiable.equals.expression.option.non.constant"), this, + "REPORT_NON_CONSTANT"); + } + @Nls @NotNull @Override @@ -154,7 +171,7 @@ public class SimplifiableEqualsExpressionInspection extends BaseInspection imple return new SimplifiableEqualsExpressionVisitor(); } - private static class SimplifiableEqualsExpressionVisitor extends BaseInspectionVisitor { + private class SimplifiableEqualsExpressionVisitor extends BaseInspectionVisitor { @Override public void visitPolyadicExpression(PsiPolyadicExpression expression) { @@ -202,12 +219,12 @@ public class SimplifiableEqualsExpressionInspection extends BaseInspection imple } } - private static String getMethodName(PsiMethodCallExpression methodCallExpression) { + private String getMethodName(PsiMethodCallExpression methodCallExpression) { final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); return methodExpression.getReferenceName(); } - private static boolean isEqualsConstant(PsiExpression expression, PsiVariable variable) { + private boolean isEqualsConstant(PsiExpression expression, PsiVariable variable) { if (!(expression instanceof PsiMethodCallExpression)) { return false; } @@ -217,13 +234,7 @@ public class SimplifiableEqualsExpressionInspection extends BaseInspection imple if (!HardcodedMethodConstants.EQUALS.equals(methodName) && !HardcodedMethodConstants.EQUALS_IGNORE_CASE.equals(methodName)) { return false; } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (!(qualifier instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; - final PsiElement target = referenceExpression.resolve(); - if (!variable.equals(target)) { + if (!ExpressionUtils.isReferenceTo(methodExpression.getQualifierExpression(), variable)) { return false; } final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); @@ -232,7 +243,11 @@ public class SimplifiableEqualsExpressionInspection extends BaseInspection imple return false; } final PsiExpression argument = arguments[0]; - return PsiUtil.isConstantExpression(argument); + if (PsiUtil.isConstantExpression(argument)) return true; + return REPORT_NON_CONSTANT && + !VariableAccessUtils.variableIsUsed(variable, argument) && + !SideEffectChecker.mayHaveSideEffects(argument) && + Boolean.FALSE.equals(CommonDataflow.getExpressionFact(argument, DfaFactType.CAN_BE_NULL)); } } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/SimplifiableEqualsExpression.html b/plugins/InspectionGadgets/src/inspectionDescriptions/SimplifiableEqualsExpression.html index 271ab045724d..fb6ed6e9d857 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/SimplifiableEqualsExpression.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/SimplifiableEqualsExpression.html @@ -11,7 +11,11 @@ And the quickfix will replace that with: 
     if ("literal".equals(s)) {}
- +

+ When checkbox is checked, 'equals()' with non-constant argument may also be reported if 'equals()' argument + is proven to be not-null. +

+ diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/simplifiable_equals_expression/SimplifiableEqualsExpression.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/simplifiable_equals_expression/SimplifiableEqualsExpression.java index 0a99fcfe7e25..d66fa185b565 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/simplifiable_equals_expression/SimplifiableEqualsExpression.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/simplifiable_equals_expression/SimplifiableEqualsExpression.java @@ -1,5 +1,7 @@ package com.siyeh.igtest.controlflow.simplifiable_equals_expression; +import java.util.*; + public class SimplifiableEqualsExpression { void foo(String namespace) { @@ -31,4 +33,28 @@ public class SimplifiableEqualsExpression { return; } } + + private Optional getOptional() { + return Optional.of(1L); + } + + // IDEA-177798 Simplifiable equals expression: support non-constant argument + public boolean foo(Long previousGroupHead) { + Long aLong = getOptional().get(); + + return previousGroupHead != null && previousGroupHead.equals(aLong); + } + + void trimTest(String s1, String s2) { + if(s1 == null || !s1.equals(s2.trim())) { + System.out.println("..."); + } + } + + void test(List list) { + String s = list.get(0); + if(s != null && s.equals(list.get(1))) { + System.out.println("???"); + } + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspectionTest.java index 4442a53d70e3..42daf165846b 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/SimplifiableEqualsExpressionInspectionTest.java @@ -1,7 +1,9 @@ package com.siyeh.ig.controlflow; import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.testFramework.LightProjectDescriptor; import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public class SimplifiableEqualsExpressionInspectionTest extends LightInspectionTestCase { @@ -10,6 +12,12 @@ public class SimplifiableEqualsExpressionInspectionTest extends LightInspectionT doTest(); } + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } + @Nullable @Override protected InspectionProfileEntry getInspection() {