From 8edd7f0dd188ef71f532c0628e899dd2328bd0d3 Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Thu, 26 Jan 2023 13:54:50 +0100 Subject: [PATCH] [java-inspections] IDEA-311273 'Explicit array filling' could process a case when array length is a variable GitOrigin-RevId: cdf0c90b6844a750935198fa1efeb3c8e8af103b --- .../ExplicitArrayFillingInspection.java | 44 ++++++++++++------- .../afterBoundLoopArrayUsedAfterLoop.java | 10 +++++ .../afterBoundLoopFillArray.java | 12 +++++ ...eforeBoundLoopArrayReassignBeforeLoop.java | 13 ++++++ .../beforeBoundLoopArrayUsedAfterLoop.java | 13 ++++++ .../beforeBoundLoopFillArray.java | 12 +++++ .../beforeBoundLoopUseBeforeLoop.java | 13 ++++++ .../siyeh/ig/psiutils/ExpressionUtils.java | 9 ++++ .../siyeh/ig/psiutils/IndexedContainer.java | 42 ++++++++++++++++++ 9 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopArrayUsedAfterLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopFillArray.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayReassignBeforeLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayUsedAfterLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopFillArray.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopUseBeforeLoop.java diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java index b5ab45718905..eff10e3c861d 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java @@ -53,10 +53,10 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect CountingLoop loop = CountingLoop.from(statement); if (loop == null || loop.isIncluding() || loop.isDescending()) return; if (!ExpressionUtils.isZero(loop.getInitializer())) return; - IndexedContainer container = IndexedContainer.fromLengthExpression(loop.getBound()); - if (container == null || !(container.getQualifier().getType() instanceof PsiArrayType)) return; PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(ControlFlowUtils.stripBraces(statement.getBody())); if (assignment == null) return; + IndexedContainer container = getContainer(loop, assignment); + if (container == null || !(container.getQualifier().getType() instanceof PsiArrayType)) return; PsiExpression index = container.extractIndexFromGetExpression(assignment.getLExpression()); if (!ExpressionUtils.isReferenceTo(index, loop.getCounter())) return; PsiExpression rValue = assignment.getRExpression(); @@ -80,7 +80,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect registerProblem(statement, true); } - private boolean isChangedInLoop(@NotNull CountingLoop loop, @NotNull PsiExpression rValue) { + private static boolean isChangedInLoop(@NotNull CountingLoop loop, @NotNull PsiExpression rValue) { if (VariableAccessUtils.collectUsedVariables(rValue).contains(loop.getCounter()) || SideEffectChecker.mayHaveSideEffects(rValue)) { return true; @@ -90,7 +90,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect .anyMatch(call -> !ClassUtils.isImmutable(call.getType()) && !ConstructionUtils.isEmptyArrayInitializer(call)); } - private boolean isDefaultValue(@NotNull PsiExpression expression, @Nullable Object defaultValue, @Nullable PsiType lType) { + private static boolean isDefaultValue(@NotNull PsiExpression expression, @Nullable Object defaultValue, @Nullable PsiType lType) { if (ExpressionUtils.isNullLiteral(expression) && defaultValue == null) return true; Object constantValue = ExpressionUtils.computeConstantExpression(expression); PsiType rType = expression.getType(); @@ -102,9 +102,9 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect return constantValue != null && constantValue.equals(defaultValue); } - private boolean isFilledWithDefaultValues(@NotNull PsiExpression expression, - @NotNull PsiForStatement statement, - @Nullable Object defaultValue) { + private static 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); @@ -129,7 +129,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect } @Nullable - private ControlFlow createControlFlow(@NotNull PsiCodeBlock block) { + private static ControlFlow createControlFlow(@NotNull PsiCodeBlock block) { try { return ControlFlowFactory.getInstance(block.getProject()) .getControlFlow(block, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); @@ -139,10 +139,10 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect } } - private PsiElement @Nullable [] getDefs(@NotNull PsiCodeBlock block, - @NotNull PsiVariable arrayVar, - @NotNull PsiReferenceExpression arrayRef, - @Nullable Object defaultValue) { + private static PsiElement @Nullable [] 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++) { @@ -165,7 +165,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect return expressions; } - private boolean isNewArrayCreation(@Nullable PsiExpression expression, @Nullable Object defaultValue) { + private static boolean isNewArrayCreation(@Nullable PsiExpression expression, @Nullable Object defaultValue) { PsiExpression arrInitExpr = PsiUtil.skipParenthesizedExprDown(expression); PsiNewExpression newExpression = tryCast(arrInitExpr, PsiNewExpression.class); PsiArrayInitializerExpression initializer; @@ -181,7 +181,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect } @Nullable - private Set getDefsOffsets(@NotNull ControlFlow flow, PsiElement @NotNull [] defs) { + private static Set getDefsOffsets(@NotNull ControlFlow flow, PsiElement @NotNull [] defs) { Set set = new HashSet<>(); for (PsiElement def : defs) { int start = flow.getStartOffset(def); @@ -256,10 +256,10 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect if (statement == null) return; CountingLoop loop = CountingLoop.from(statement); if (loop == null) return; - IndexedContainer container = IndexedContainer.fromLengthExpression(loop.getBound()); - if (container == null) return; PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(ControlFlowUtils.stripBraces(statement.getBody())); if (assignment == null) return; + IndexedContainer container = getContainer(loop, assignment); + if (container == null) return; PsiExpression rValue = assignment.getRExpression(); if (rValue == null) return; CommentTracker ct = new CommentTracker(); @@ -288,4 +288,16 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect return TypeConversionUtil.isAssignable(assignTo, rType) ? "" : "(" + elementType.getCanonicalText() + ")"; } } + + @Nullable + private static IndexedContainer getContainer(CountingLoop loop, PsiAssignmentExpression assignment) { + IndexedContainer container = IndexedContainer.fromLengthExpression(loop.getBound()); + if (container == null) { + if (!(assignment.getLExpression() instanceof PsiArrayAccessExpression arrayAccessExpression)) { + return null; + } + container = IndexedContainer.arrayContainerWithBound(arrayAccessExpression, loop.getBound()); + } + return container; + } } diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopArrayUsedAfterLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopArrayUsedAfterLoop.java new file mode 100644 index 000000000000..87e01702692b --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopArrayUsedAfterLoop.java @@ -0,0 +1,10 @@ +// "Remove 'for' statement" "true" + +class Test { + + public static int[] init(int n, boolean b) { + int[] data = new int[n]; + data[n - 1] = 6; + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopFillArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopFillArray.java new file mode 100644 index 000000000000..f78597aa2143 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterBoundLoopFillArray.java @@ -0,0 +1,12 @@ +// "Replace loop with 'Arrays.setAll()' method call" "true" + +import java.util.Arrays; + +class Test { + + public static Object[] init(int n, boolean b) { + Object[] data = new Object[n]; + Arrays.setAll(data, j -> (j / 2 + n == 0) ? "1" : new Object()); + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayReassignBeforeLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayReassignBeforeLoop.java new file mode 100644 index 000000000000..b79a501d459f --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayReassignBeforeLoop.java @@ -0,0 +1,13 @@ +// "Remove 'for' statement" "false" + +class Test { + + public static int[] init(int n, boolean b) { + int[] data = new int[n]; + data = new int[n + 1]; + for (int j = 0; j < n; j++) { + data[j] = 0; + } + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayUsedAfterLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayUsedAfterLoop.java new file mode 100644 index 000000000000..3b6e3f3bfe46 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopArrayUsedAfterLoop.java @@ -0,0 +1,13 @@ +// "Remove 'for' statement" "true" + +class Test { + + public static int[] init(int n, boolean b) { + int[] data = new int[n]; + for (int j = 0; j < n; j++) { + data[j] = 0; + } + data[n - 1] = 6; + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopFillArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopFillArray.java new file mode 100644 index 000000000000..f75fdc8cc3d8 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopFillArray.java @@ -0,0 +1,12 @@ +// "Replace loop with 'Arrays.setAll()' method call" "true" + +class Test { + + public static Object[] init(int n, boolean b) { + Object[] data = new Object[n]; + for (int j = 0; j < n; j++) { + data[j] = (j / 2 + n == 0) ? "1" : new Object(); + } + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopUseBeforeLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopUseBeforeLoop.java new file mode 100644 index 000000000000..4c825f899595 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeBoundLoopUseBeforeLoop.java @@ -0,0 +1,13 @@ +// "Remove 'for' statement" "false" + +class Test { + + public static int[] init(int n, boolean b) { + int[] data = new int[n]; + n = 10; + for (int j = 0; j < n; j++) { + data[j] = 0; + } + return data; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index 4e596099981d..3481f9922496 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -1032,6 +1032,15 @@ public final class ExpressionUtils { return tryCast(referenceExpression.resolve(), PsiLocalVariable.class); } + @Contract("null -> null") + @Nullable + public static PsiVariable resolveVariable(@Nullable PsiExpression expression) { + expression = PsiUtil.skipParenthesizedExprDown(expression); + PsiReferenceExpression referenceExpression = tryCast(expression, PsiReferenceExpression.class); + if(referenceExpression == null) return null; + return tryCast(referenceExpression.resolve(), PsiVariable.class); + } + public static boolean isOctalLiteral(PsiLiteralExpression literal) { final PsiType type = literal.getType(); if (!PsiType.INT.equals(type) && !PsiType.LONG.equals(type)) { diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/IndexedContainer.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/IndexedContainer.java index 8f50df78f2eb..bde70d4cf913 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/IndexedContainer.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/IndexedContainer.java @@ -104,6 +104,48 @@ public abstract class IndexedContainer { return null; } + /** + * Use to create IndexedContainer for the next case: + * 
{@code
+   *  int[] newArray = new int[arrayLength];
+   *  for (int i=0; i < arrayLength; i++) {
+   *    newArray[i] = 0;
+   *  }
+   * }
+ * Additionally, the method that newArray and arrayLength are not reassigned + * + * @param arrayAccessExpression expression to create an IndexedContainer from + * @param bound reference to arrayLength + * @return newly created IndexedContainer or null if it is impossible to resolve it + */ + @Nullable + public static IndexedContainer arrayContainerWithBound(@NotNull PsiArrayAccessExpression arrayAccessExpression, + @NotNull PsiExpression bound) { + PsiExpression arrayExpression = arrayAccessExpression.getArrayExpression(); + if (arrayExpression instanceof PsiReferenceExpression reference && + reference.resolve() instanceof PsiVariable arrayVariable) { + PsiExpression initializer = arrayVariable.getInitializer(); + if (!(initializer instanceof PsiNewExpression newExpression)) { + return null; + } + PsiExpression[] dimensions = newExpression.getArrayDimensions(); + if (dimensions.length != 1) { + return null; + } + PsiExpression dimension = dimensions[0]; + PsiVariable dimensionVariable = ExpressionUtils.resolveVariable(dimension); + PsiVariable boundVariable = ExpressionUtils.resolveVariable(bound); + if (dimensionVariable == null || boundVariable == null || !dimensionVariable.isEquivalentTo(boundVariable)) { + return null; + } + if ((VariableAccessUtils.variableIsAssigned(dimensionVariable)) || + (VariableAccessUtils.variableIsAssigned(arrayVariable))) { + return null; + } + } + return new ArrayIndexedContainer(arrayExpression); + } + static class ArrayIndexedContainer extends IndexedContainer { ArrayIndexedContainer(@NotNull PsiExpression qualifier) { super(qualifier);