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 6d1bf08c8af8..f375f70b4e02 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 @@ -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 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, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index a83abedacb40..36ca6d1620d1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -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 myPossibleVariableValues = MultiMap.createSet(); private final Set myReceiverMutabilityViolation = new HashSet<>(); private final Set myArgumentMutabilityViolation = new HashSet<>(); + private final Map 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 sameValueAssignments() { + return StreamEx.ofKeys(mySameValueAssigned, Boolean::booleanValue); + } + @Override protected void onInstructionProducesCCE(TypeCastInstruction instruction) { myCCEInstructions.add(instruction); diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 16839d62c9c0..52a1a1d31fed 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -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 createNPEFixes(PsiExpression qualifier, PsiExpression expression, boolean onTheFly) { qualifier = PsiUtil.deparenthesizeExpression(qualifier); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NumberComparisonsWhenValueIsKnown.java b/java/java-tests/testData/inspection/dataFlow/fixture/NumberComparisonsWhenValueIsKnown.java index 8609af5bfe4a..ee0011f119f9 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/NumberComparisonsWhenValueIsKnown.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NumberComparisonsWhenValueIsKnown.java @@ -22,7 +22,7 @@ class Test { void forLoop() { int target = 3; for (int current = 0; current < target; ) { - current = 0; + current = 0; } } @@ -40,7 +40,7 @@ class Test { int target = 3; while (current < target) { - current = 0; + current = 0; } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/RedundantAssignment.java b/java/java-tests/testData/inspection/dataFlow/fixture/RedundantAssignment.java new file mode 100644 index 000000000000..82d3a2962619 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/RedundantAssignment.java @@ -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 { + flag = false; + } + } + System.out.println(flag); + } + + void fillArray(int[] arr) { + arr[0] = 1; + arr[1] = 2; + arr[2] = 3; + arr[0] = 1; + } + + void withTest(int x) { + if(x != 0) { + System.out.println(x); + } else { + x = 0; + } + System.out.println("oops"); + } + + void var(Object b) { + Object a = b; + if(b.hashCode() > 10) { + a = null; + } else { + a = b; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java b/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java index b87492380f6d..15cb3dd98651 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java @@ -16,7 +16,7 @@ class Test { } if (!rangeMarkersDisposed) { - foo = "dd"; + foo = "dd"; } } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 9da0613fdf59..05769b22ec0d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -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(); } } diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 56ebec35e574..089fc1286bbf 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -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