From ed69cc6401557de3eb3cc2802809c28d9773608a Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 10 Nov 2021 12:22:12 +0700 Subject: [PATCH] [java-inspections] IDEA-282262 Add 'Find the cause' action to 'Redundant operation on empty container' inspection GitOrigin-RevId: 79f33120532750b5e93ffa3d7ce40c2345d17e39 --- .../messages/JavaAnalysisBundle.properties | 1 + .../dataFlow/TrackingRunner.java | 29 ++++++++++++++++--- .../tracker/EmptyCollectionSimple.java | 15 ++++++++++ .../DataFlowInspectionTrackerTest.java | 6 ++++ ...ntOperationOnEmptyContainerInspection.java | 19 ++++++++++-- 5 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/EmptyCollectionSimple.java diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index 3ae0602e8c29..e72d9109a23d 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -491,6 +491,7 @@ dfa.find.cause.call.always.fails=call always fails dfa.find.cause.one.of.the.following.happens=one of the following happens: dfa.find.cause.an.execution.might.exist.where=an execution might exist where: dfa.find.cause.value.is.always.the.same=value is always {0} +dfa.find.cause.size.is.always.zero=size is always zero dfa.find.cause.value.x.is.always.the.same=value ''{0}'' is always ''{1}'' dfa.find.cause.compile.time.constant=it''s compile-time constant which evaluates to ''{0}'' dfa.find.cause.equality.established.from.condition=''{0}'' was established from condition diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java index dabd5704b68f..d7aff2e970c7 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java @@ -533,6 +533,27 @@ public final class TrackingRunner extends StandardDataFlowRunner { } } + public static class ZeroSizeDfaProblemType extends DfaProblemType { + final SpecialField myField; + + public ZeroSizeDfaProblemType(@NotNull SpecialField field) { + myField = field; + } + + @Override + public CauseItem[] findCauses(TrackingRunner runner, PsiExpression expression, MemoryStateChange history) { + DfaValue topOfStack = history.myTopOfStack; + DfaValue value = myField.createValue(runner.getFactory(), topOfStack); + MemoryStateChange change = MemoryStateChange.create(history, new JvmPushInstruction(value, null), Map.of(), value); + return runner.findConstantValueCause(expression, change, 0); + } + + @Override + public String toString() { + return JavaAnalysisBundle.message("dfa.find.cause.size.is.always.zero"); + } + } + static class CustomDfaProblemType extends DfaProblemType { private final @Nls String myMessage; @@ -546,7 +567,7 @@ public final class TrackingRunner extends StandardDataFlowRunner { } } - private CauseItem @NotNull [] findConstantValueCause(PsiExpression expression, MemoryStateChange history, Object expectedValue) { + private CauseItem @NotNull [] findConstantValueCause(@Nullable PsiExpression expression, MemoryStateChange history, Object expectedValue) { if (expression instanceof PsiLiteralExpression) return new CauseItem[0]; Object constantExpressionValue = ExpressionUtils.computeConstantExpression(expression); DfaValue value = history.myTopOfStack; @@ -554,7 +575,7 @@ public final class TrackingRunner extends StandardDataFlowRunner { return new CauseItem[]{new CauseItem(JavaAnalysisBundle.message("dfa.find.cause.compile.time.constant", value), expression)}; } Boolean boolConst = value.getDfType().getConstantOfType(Boolean.class); - if (boolConst != null && boolConst.equals(expectedValue)) { + if (boolConst != null && expression != null && boolConst.equals(expectedValue)) { return findBooleanResultCauses(expression, history, boolConst); } if (value instanceof DfaVariableValue) { @@ -618,8 +639,8 @@ public final class TrackingRunner extends StandardDataFlowRunner { return new CauseItem(message, anchor); } - private CauseItem[] findBooleanResultCauses(PsiExpression expression, - MemoryStateChange history, + private CauseItem[] findBooleanResultCauses(@NotNull PsiExpression expression, + @NotNull MemoryStateChange history, boolean value) { if (BoolUtils.isNegation(expression)) { PsiExpression negated = BoolUtils.getNegated(expression); diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/EmptyCollectionSimple.java b/java/java-tests/testData/inspection/dataFlow/tracker/EmptyCollectionSimple.java new file mode 100644 index 000000000000..95c91df79371 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/EmptyCollectionSimple.java @@ -0,0 +1,15 @@ +/* +Size is always zero (map.entrySet(); line#10) + 'map.size == 0' was established from condition (map.isEmpty(); line#9) + */ +import java.util.*; + +public class EmptyCollectionSimple { + void test(Map map) { + if (map.isEmpty()) { + for(var e : map.entrySet()) { + + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java index e434e5e49654..f435932518a8 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -4,6 +4,7 @@ package com.intellij.java.codeInspection; import com.intellij.JavaTestUtil; import com.intellij.codeInspection.dataFlow.CommonDataflow; import com.intellij.codeInspection.dataFlow.TrackingRunner; +import com.intellij.codeInspection.dataFlow.jvm.SpecialField; import com.intellij.openapi.editor.SelectionModel; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; @@ -93,6 +94,10 @@ public class DataFlowInspectionTrackerTest extends LightJavaCodeInsightFixtureTe return new TrackingRunner.CastDfaProblemType(); } PsiElement parent = expression.getParent(); + if (parent instanceof PsiForeachStatement) { + return new TrackingRunner.ZeroSizeDfaProblemType( + expression.getType() instanceof PsiArrayType ? SpecialField.ARRAY_LENGTH : SpecialField.COLLECTION_SIZE); + } if (parent instanceof PsiReferenceExpression) { // Test possible NPE in qualifiers only return new TrackingRunner.NullableDfaProblemType(); @@ -191,4 +196,5 @@ public class DataFlowInspectionTrackerTest extends LightJavaCodeInsightFixtureTe public void testSubStringEqualsIgnoreCase() { doTest(); } public void testSubStringEqualsIgnoreCase2() { doTest(); } public void testArrayBlockingQueueContract() { doTest(); } + public void testEmptyCollectionSimple() { doTest(); } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantOperationOnEmptyContainerInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantOperationOnEmptyContainerInspection.java index 73eab642a689..5cb4a7d3b685 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantOperationOnEmptyContainerInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/redundancy/RedundantOperationOnEmptyContainerInspection.java @@ -6,6 +6,8 @@ import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.codeInspection.dataFlow.CommonDataflow; +import com.intellij.codeInspection.dataFlow.TrackingRunner; +import com.intellij.codeInspection.dataFlow.fix.FindDfaProblemCauseFix; import com.intellij.codeInspection.dataFlow.jvm.SpecialField; import com.intellij.codeInspection.dataFlow.types.DfType; import com.intellij.codeInspection.util.InspectionMessage; @@ -70,7 +72,7 @@ public class RedundantOperationOnEmptyContainerInspection extends AbstractBaseJa if (ExpressionUtils.isVoidContext(call)) { fix = new DeleteElementFix(call, InspectionGadgetsBundle.message("remove.call.fix.family.name")); } - holder.registerProblem(container, msg, fix); + holder.registerProblem(container, msg, fix, getFindCauseFix(container)); } } @@ -80,7 +82,20 @@ public class RedundantOperationOnEmptyContainerInspection extends AbstractBaseJa if (value == null) return; String msg = getProblemMessage(value); if (msg == null) return; - holder.registerProblem(value, msg, new DeleteElementFix(statement, InspectionGadgetsBundle.message("remove.loop.fix.family.name"))); + holder.registerProblem(value, msg, + new DeleteElementFix(statement, InspectionGadgetsBundle.message("remove.loop.fix.family.name")), + getFindCauseFix(value)); + } + + private @NotNull LocalQuickFix getFindCauseFix(@NotNull PsiExpression value) { + PsiType type = value.getType(); + SpecialField field; + if (type instanceof PsiArrayType) { + field = SpecialField.ARRAY_LENGTH; + } else { + field = SpecialField.COLLECTION_SIZE; + } + return new FindDfaProblemCauseFix(false, value, new TrackingRunner.ZeroSizeDfaProblemType(field)); } @Nullable