CommonDataflow: annotate method calls; IDEA-177798 Simplifiable equals expression: support non-constant argument

This commit is contained in:
Tagir Valeev
2017-09-06 15:24:44 +07:00
parent 6ec240f606
commit d7e1bdf5b8
6 changed files with 87 additions and 12 deletions
@@ -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;
@@ -1908,6 +1908,7 @@ empty.directory.display.name=Empty directory
empty.directories.problem.descriptor=Empty directory <code>{0}</code>
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
@@ -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));
}
}
}
@@ -11,7 +11,11 @@ And the quickfix will replace that with:
<code><pre>
<b>if</b> ("literal".equals(s)) {}
</pre></code>
<!-- tooltip end -->
</p>
<p>
When checkbox is checked, 'equals()' with non-constant argument may also be reported if 'equals()' argument
is proven to be not-null.
</p>
<!-- tooltip end -->
</body>
</html>
@@ -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<Long> getOptional() {
return Optional.of(1L);
}
// IDEA-177798 Simplifiable equals expression: support non-constant argument
public boolean foo(Long previousGroupHead) {
Long aLong = getOptional().get();
return <warning descr="Unnecessary 'null' check before 'equals()' call">previousGroupHead != null</warning> && previousGroupHead.equals(aLong);
}
void trimTest(String s1, String s2) {
if(<warning descr="Unnecessary 'null' check before 'equals()' call">s1 == null</warning> || !s1.equals(s2.trim())) {
System.out.println("...");
}
}
void test(List<String> list) {
String s = list.get(0);
if(s != null && s.equals(list.get(1))) {
System.out.println("???");
}
}
}
@@ -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() {