IDEA-184299 Warn if variable is assigned to the value which it already has on all control paths

This commit is contained in:
Tagir Valeev
2017-12-28 17:12:54 +07:00
parent 90d4f8ab2e
commit 3d5f2d8508
8 changed files with 126 additions and 7 deletions
@@ -25,6 +25,7 @@ import com.intellij.openapi.util.WriteExternalException;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.ArrayUtil;
@@ -218,6 +219,10 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
return null;
}
protected LocalQuickFix createRemoveAssignmentFix(PsiAssignmentExpression assignment) {
return null;
}
protected LocalQuickFix createReplaceWithTrivialLambdaFix(Object value) {
return null;
}
@@ -286,6 +291,34 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
InspectionsBundle.message("dataflow.message.immutable.modified"));
reportMutabilityViolations(holder, reportedAnchors, visitor.getMutabilityViolations(false),
InspectionsBundle.message("dataflow.message.immutable.passed"));
reportDuplicateAssignments(holder, reportedAnchors, visitor);
}
private void reportDuplicateAssignments(ProblemsHolder holder,
HashSet<PsiElement> reportedAnchors,
DataFlowInstructionVisitor visitor) {
visitor.sameValueAssignments().forEach(expr -> {
if(!reportedAnchors.add(expr)) return;
PsiAssignmentExpression assignment = PsiTreeUtil.getParentOfType(expr, PsiAssignmentExpression.class);
PsiElement context = PsiTreeUtil.getParentOfType(expr, PsiForStatement.class, PsiClassInitializer.class);
if (context instanceof PsiForStatement && PsiTreeUtil.isAncestor(((PsiForStatement)context).getInitialization(), expr, true)) {
return;
}
if (context instanceof PsiClassInitializer && expr instanceof PsiReferenceExpression) {
if (assignment != null) {
Object constValue = ExpressionUtils.computeConstantExpression(assignment.getRExpression());
if (constValue == PsiTypesUtil.getDefaultValue(expr.getType())) {
PsiElement target = ((PsiReferenceExpression)expr).resolve();
if (target instanceof PsiField &&
((PsiField)target).getContainingClass() == ((PsiClassInitializer)context).getContainingClass()) {
return;
}
}
}
}
holder.registerProblem(expr, InspectionsBundle.message("dataflow.message.redundant.assignment"), createRemoveAssignmentFix(assignment));
});
}
private static void reportMutabilityViolations(ProblemsHolder holder,
@@ -2,10 +2,7 @@
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
@@ -36,8 +33,44 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
private final MultiMap<PushInstruction, Object> myPossibleVariableValues = MultiMap.createSet();
private final Set<PsiElement> myReceiverMutabilityViolation = new HashSet<>();
private final Set<PsiElement> myArgumentMutabilityViolation = new HashSet<>();
private final Map<PsiExpression, Boolean> mySameValueAssigned = new HashMap<>();
private boolean myAlwaysReturnsNotNull = true;
@Override
public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiExpression left = instruction.getLExpression();
if (left != null && !Boolean.FALSE.equals(mySameValueAssigned.get(left))) {
DfaValue dest = memState.peek();
// Reporting of floating zero is skipped, because this produces false-positives on the code like
// if(x == -0.0) x = 0.0;
if (dest instanceof DfaVariableValue || (dest instanceof DfaConstValue && !isFloatingZero(((DfaConstValue)dest).getValue()))) {
DfaMemoryState copy = memState.createCopy();
copy.pop();
DfaValue src = copy.peek();
boolean sameValue = !copy.applyCondition(runner.getFactory().createCondition(dest, DfaRelationValue.RelationType.NE, src));
mySameValueAssigned.merge(left, sameValue, Boolean::logicalAnd);
}
else {
mySameValueAssigned.put(left, Boolean.FALSE);
}
}
return super.visitAssign(instruction, runner, memState);
}
private static boolean isFloatingZero(Object value) {
if (value instanceof Double) {
return ((Double)value).doubleValue() == 0.0;
}
if (value instanceof Float) {
return ((Float)value).floatValue() == 0.0f;
}
return false;
}
StreamEx<PsiExpression> sameValueAssignments() {
return StreamEx.ofKeys(mySameValueAssigned, Boolean::booleanValue);
}
@Override
protected void onInstructionProducesCCE(TypeCastInstruction instruction) {
myCCEInstructions.add(instruction);
@@ -16,6 +16,7 @@
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.NullableNotNullDialog;
import com.intellij.codeInsight.daemon.impl.quickfix.DeleteSideEffectsAwareFix;
import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix;
import com.intellij.codeInspection.*;
import com.intellij.codeInspection.dataFlow.fix.SurroundWithRequireNonNullFix;
@@ -101,6 +102,14 @@ public class DataFlowInspection extends DataFlowInspectionBase {
return fixes;
}
@Override
protected LocalQuickFix createRemoveAssignmentFix(PsiAssignmentExpression assignment) {
if (assignment == null || assignment.getRExpression() == null || !(assignment.getParent() instanceof PsiExpressionStatement)) {
return null;
}
return new DeleteSideEffectsAwareFix((PsiStatement)assignment.getParent(), assignment.getRExpression());
}
@NotNull
protected List<LocalQuickFix> createNPEFixes(PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
qualifier = PsiUtil.deparenthesizeExpression(qualifier);
@@ -22,7 +22,7 @@ class Test {
void forLoop() {
int target = 3;
for (int current = 0; <warning descr="Condition 'current < target' is always 'true'">current < target</warning>; ) {
current = 0;
<warning descr="Value assigned to the variable is already assigned to it">current</warning> = 0;
}
}
@@ -40,7 +40,7 @@ class Test {
int target = 3;
while (<warning descr="Condition 'current < target' is always 'true'">current < target</warning>)
{
current = 0;
<warning descr="Value assigned to the variable is already assigned to it">current</warning> = 0;
}
}
@@ -0,0 +1,42 @@
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class RedundantAssignment {
void test(int x) {
boolean flag = false;
if(x > 2) {
if(x % 3 == 1) {
flag = true;
} else {
<warning descr="Value assigned to the variable is already assigned to it">flag</warning> = false;
}
}
System.out.println(flag);
}
void fillArray(int[] arr) {
arr[0] = 1;
arr[1] = 2;
arr[2] = 3;
<warning descr="Value assigned to the variable is already assigned to it">arr[0]</warning> = 1;
}
void withTest(int x) {
if(x != 0) {
System.out.println(x);
} else {
<warning descr="Value assigned to the variable is already assigned to it">x</warning> = 0;
}
System.out.println("oops");
}
void var(Object b) {
Object a = b;
if(b.hashCode() > 10) {
a = null;
} else {
<warning descr="Value assigned to the variable is already assigned to it">a</warning> = b;
}
}
}
@@ -16,7 +16,7 @@ class Test {
}
if (!rangeMarkersDisposed) {
foo = "dd";
<warning descr="Value assigned to the variable is already assigned to it">foo</warning> = "dd";
}
}
}
@@ -577,4 +577,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testNullableReturn() { doTest(); }
public void testManyBooleans() { doTest(); }
public void testPureNoArgMethodAsVariable() { doTest(); }
public void testRedundantAssignment() { doTest(); }
}
@@ -94,6 +94,7 @@ dataflow.message.constant.method.reference=Method reference result is always ''{
dataflow.message.array.index.out.of.bounds=Array index is out of bounds
dataflow.message.immutable.modified=Immutable object is modified
dataflow.message.immutable.passed=Immutable object is passed where mutable is expected
dataflow.message.redundant.assignment=Value assigned to the variable is already assigned to it
#deprecated
inspection.deprecated.display.name=Deprecated API usage