diff --git a/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java b/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java index 20de7f0bac64..a7a46b4d81bf 100644 --- a/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java @@ -6,14 +6,13 @@ import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.codeInspection.util.LambdaGenerationUtil; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.TextRange; import com.intellij.pom.java.JavaFeature; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.controlFlow.DefUseUtil; +import com.intellij.psi.controlFlow.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; @@ -23,6 +22,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.function.Predicate; @@ -61,9 +61,8 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect PsiExpression rValue = assignment.getRExpression(); if (rValue == null) return; if (!isChangedInLoop(loop, rValue)) { - if (!ControlFlowUtils.isInLoop(statement) && - isDefaultValueAssigned(assignment, rValue) && - isFilledWithDefaultValues(container.getQualifier(), statement)) { + Object defaultValue = PsiTypesUtil.getDefaultValue(assignment.getType()); + if (isDefaultValue(rValue, defaultValue) && isFilledWithDefaultValues(container.getQualifier(), statement, defaultValue)) { holder.registerProblem(statement, getRange(statement, ProblemHighlightType.WARNING), InspectionsBundle.message("inspection.explicit.array.filling.redundant.loop.description"), QuickFixFactory.getInstance().createDeleteFix(statement)); @@ -88,88 +87,98 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect .anyMatch(call -> !ClassUtils.isImmutable(call.getType()) && !ConstructionUtils.isEmptyArrayInitializer(call)); } - private boolean isDefaultValueAssigned(@NotNull PsiAssignmentExpression assignment, @NotNull PsiExpression rhs) { - Object defaultValue = PsiTypesUtil.getDefaultValue(assignment.getType()); - if (ExpressionUtils.isNullLiteral(rhs) && defaultValue == null) return true; - Object constantValue = ExpressionUtils.computeConstantExpression(rhs); + private boolean isDefaultValue(@NotNull PsiExpression expression, @Nullable Object defaultValue) { + if (ExpressionUtils.isNullLiteral(expression) && defaultValue == null) return true; + Object constantValue = ExpressionUtils.computeConstantExpression(expression); return constantValue != null && constantValue.equals(defaultValue); } - private boolean isFilledWithDefaultValues(@NotNull PsiExpression expression, @NotNull PsiForStatement forStatement) { + private boolean isFilledWithDefaultValues(@NotNull PsiExpression expression, + @NotNull PsiForStatement statement, + @Nullable Object defaultValue) { PsiReferenceExpression arrayRef = tryCast(PsiUtil.skipParenthesizedExprDown(expression), PsiReferenceExpression.class); if (arrayRef == null) return false; PsiVariable arrayVar = tryCast(arrayRef.resolve(), PsiVariable.class); if (arrayVar == null) return false; PsiCodeBlock block = tryCast(PsiUtil.getVariableCodeBlock(arrayVar, null), PsiCodeBlock.class); if (block == null) return false; - Set defs = getDefsStatements(DefUseUtil.getDefs(block, arrayVar, arrayRef)); + ControlFlow flow = createControlFlow(block); + if (flow == null) return false; + int statementStart = flow.getStartOffset(statement); + if (statementStart == -1) return false; + int statementEnd = flow.getEndOffset(statement); + if (statementEnd == -1) return false; + PsiElement[] defs = getDefs(block, arrayVar, arrayRef, defaultValue); if (defs == null) return false; - return !isUsedBetween(forStatement, defs, arrayVar, block); - } - - private boolean isUsedBetween(@NotNull PsiElement ref, @NotNull Set defs, - @NotNull PsiVariable arrayVar, @NotNull PsiCodeBlock block) { - Ref isUsed = Ref.create(false); - - block.accept(new JavaRecursiveElementWalkingVisitor() { - - boolean inContext; - - @Override - public void visitStatement(PsiStatement statement) { - if (defs.contains(statement)) { - inContext = true; - } - else { - if (statement == ref) { - inContext = false; - stopWalking(); - } - super.visitStatement(statement); - } - } - - @Override - public void visitReferenceElement(PsiJavaCodeReferenceElement reference) { - if (inContext && reference.isReferenceTo(arrayVar)) { - isUsed.set(true); - stopWalking(); - } - super.visitReferenceElement(reference); - } - }); - return isUsed.get(); + Set exclude = getDefsOffsets(flow, defs); + if (exclude == null) return false; + for (int i = statementStart; i < statementEnd; i++) { + exclude.add(i); + } + return Arrays.stream(defs) + .map(def -> flow.getEndOffset(def)) + .noneMatch(offset -> ControlFlowUtils.isVariableReferencedBeforeStatementEntry(flow, offset + 1, statement, arrayVar, exclude)); } @Nullable - private Set getDefsStatements(@NotNull PsiElement[] defs) { - Set statements = new HashSet<>(); - for (PsiElement def : defs) { + private ControlFlow createControlFlow(@NotNull PsiCodeBlock block) { + try { + return ControlFlowFactory.getInstance(block.getProject()) + .getControlFlow(block, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + } + catch (AnalysisCanceledException ignored) { + return null; + } + } + + @Nullable + private PsiElement[] getDefs(@NotNull PsiCodeBlock block, + @NotNull PsiVariable arrayVar, + @NotNull PsiReferenceExpression arrayRef, + @Nullable Object defaultValue) { + PsiElement[] defs = DefUseUtil.getDefs(block, arrayVar, arrayRef); + PsiExpression[] expressions = new PsiExpression[defs.length]; + for (int i = 0; i < defs.length; i++) { + PsiElement def = defs[i]; PsiVariable variable = tryCast(def, PsiVariable.class); if (variable != null) { PsiExpression initializer = variable.getInitializer(); - if (initializer == null || !isNewArrayCreation(initializer)) return null; - PsiDeclarationStatement declaration = PsiTreeUtil.getParentOfType(initializer, PsiDeclarationStatement.class); - if (declaration == null) return null; - statements.add(declaration); + if (!isNewArrayCreation(initializer, defaultValue)) return null; + expressions[i] = initializer; continue; } PsiAssignmentExpression assignment = PsiTreeUtil.getParentOfType(def, PsiAssignmentExpression.class); if (assignment != null) { - if (!isNewArrayCreation(assignment.getRExpression())) return null; - PsiExpressionStatement expressionStatement = PsiTreeUtil.getParentOfType(assignment, PsiExpressionStatement.class); - if (expressionStatement == null) return null; - statements.add(expressionStatement); + if (!isNewArrayCreation(assignment.getRExpression(), defaultValue)) return null; + expressions[i] = assignment; continue; } return null; } - return statements; + return expressions; } - private boolean isNewArrayCreation(@Nullable PsiExpression expression) { - expression = PsiUtil.skipParenthesizedExprDown(expression); - return expression == null || expression instanceof PsiNewExpression; + private boolean isNewArrayCreation(@Nullable PsiExpression expression, @Nullable Object defaultValue) { + PsiNewExpression newExpression = tryCast(PsiUtil.skipParenthesizedExprDown(expression), PsiNewExpression.class); + if (newExpression == null) return false; + PsiArrayInitializerExpression initializer = newExpression.getArrayInitializer(); + if (initializer == null) return true; + return Arrays.stream(initializer.getInitializers()).allMatch(init -> isDefaultValue(init, defaultValue)); + } + + @Nullable + private Set getDefsOffsets(@NotNull ControlFlow flow, @NotNull PsiElement[] defs) { + Set set = new HashSet<>(); + for (PsiElement def : defs) { + int start = flow.getStartOffset(def); + if (start == -1) return null; + int end = flow.getEndOffset(def); + if (end == -1) return null; + for (int i = start; i < end; i++) { + set.add(i); + } + } + return set; } private void registerProblem(@NotNull PsiForStatement statement, boolean isSetAll) { diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayInitWithNonDefaultValues.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayInitWithNonDefaultValues.java new file mode 100644 index 000000000000..206b262764c0 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayInitWithNonDefaultValues.java @@ -0,0 +1,15 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +import java.util.Arrays; + +public class Test { + + public static int[] init(boolean b) { + int[] arr = new int[10]; + if (b) { + arr = new int[]{1, 2, 3, 4, 5}; + } + Arrays.fill(arr, 0); + return arr; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java index 44f54faaa3a8..d1d813c3267e 100644 --- a/java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java @@ -1,6 +1,4 @@ -// "Replace loop with 'Arrays.fill()' method call" "true" - -import java.util.Arrays; +// "Delete element" "true" public class Test { @@ -8,9 +6,7 @@ public class Test { int[] data = new int[n]; int i = 0; while (i < 3) { - Arrays.fill(data, 0); - if (i < 2) data[n - 1] = 6; - i++; + i++; } return data; } diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java index 472d1cbb7605..165b9632d382 100644 --- a/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java @@ -2,8 +2,11 @@ public class Test { - public static int[] init(int[] arr) { + public static int[] init(int[] arr, boolean b) { arr = new int[10]; + if (b) { + arr = new int[20]; + } return arr; } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayInitWithNonDefaultValues.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayInitWithNonDefaultValues.java new file mode 100644 index 000000000000..93ea55894d9b --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayInitWithNonDefaultValues.java @@ -0,0 +1,15 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +public class Test { + + public static int[] init(boolean b) { + int[] arr = new int[10]; + if (b) { + arr = new int[]{1, 2, 3, 4, 5}; + } + for (int i = 0; i < arr.length; i++) { + arr[i] = 0; + } + return arr; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java index bae338170110..67c61f65ad71 100644 --- a/java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java @@ -1,4 +1,4 @@ -// "Replace loop with 'Arrays.fill()' method call" "true" +// "Delete element" "true" public class Test { @@ -9,7 +9,6 @@ public class Test { for (int j = 0; j < data.length; j++) { data[j] = 0; } - if (i < 2) data[n - 1] = 6; i++; } return data; diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java index 7a744df74a81..edcd2413bc93 100644 --- a/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java @@ -2,8 +2,11 @@ public class Test { - public static int[] init(int[] arr) { + public static int[] init(int[] arr, boolean b) { arr = new int[10]; + if (b) { + arr = new int[20]; + } for (int i = 0; i < arr.length; i++) { arr[i] = 0; } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java index 8630df913fa6..49231f8c0e21 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java @@ -33,6 +33,7 @@ import org.jetbrains.annotations.Nullable; import java.util.BitSet; import java.util.Collections; import java.util.List; +import java.util.Set; import static com.siyeh.ig.psiutils.ControlFlowUtils.InitializerUsageStatus.*; @@ -773,16 +774,18 @@ public class ControlFlowUtils { * Back-edges are also considered, so the actual place where it referenced might be outside of * (start, loop entry) interval. * - * @param flow ControlFlow to analyze - * @param start start point + * @param flow ControlFlow to analyze + * @param start start point * @param statement loop to check - * @param variable variable to analyze + * @param variable variable to analyze + * @param excluded instructions to exclude * @return true if variable can be referenced between start point and statement entry */ - private static boolean isVariableReferencedBeforeStatementEntry(@NotNull ControlFlow flow, - final int start, - final PsiStatement statement, - @NotNull PsiVariable variable) { + public static boolean isVariableReferencedBeforeStatementEntry(@NotNull ControlFlow flow, + final int start, + final PsiStatement statement, + @NotNull PsiVariable variable, + @NotNull Set excluded) { final int statementStart = flow.getStartOffset(statement); final int statementEnd = flow.getEndOffset(statement); @@ -800,19 +803,19 @@ public class ControlFlowUtils { int to = edge.myTo; if(referenced.get(from)) { // jump to the loop start from within the loop is not considered as loop entry - if(to == statementStart && (from < statementStart || from >= statementEnd)) { + if (to == statementStart && (from < statementStart || from >= statementEnd)) { return true; } - if(!referenced.get(to)) { + if (!referenced.get(to)) { referenced.set(to); changed = true; } continue; } - if(ControlFlowUtil.isVariableAccess(flow, from, variable)) { + if (ControlFlowUtil.isVariableAccess(flow, from, variable) && !excluded.contains(from)) { referenced.set(from); referenced.set(to); - if(to == statementStart) return true; + if (to == statementStart) return true; changed = true; } } @@ -845,7 +848,7 @@ public class ControlFlowUtils { } int start = controlFlow.getEndOffset(var.getInitializer())+1; int stop = controlFlow.getStartOffset(statement); - if(isVariableReferencedBeforeStatementEntry(controlFlow, start, statement, var)) return UNKNOWN; + if (isVariableReferencedBeforeStatementEntry(controlFlow, start, statement, var, Collections.emptySet())) return UNKNOWN; if (!ControlFlowUtil.isValueUsedWithoutVisitingStop(controlFlow, start, stop, var)) return AT_WANTED_PLACE_ONLY; return var.hasModifierProperty(PsiModifier.FINAL) ? UNKNOWN : AT_WANTED_PLACE; }