From 47b823ffda19a4b3aab0ad912f5001595e3dd96c Mon Sep 17 00:00:00 2001 From: Artemiy Sartakov Date: Mon, 16 Sep 2019 20:19:33 +0700 Subject: [PATCH] ExplicitArrayFillingInspection: check array content before suggesting to remove explicit filling loop (IDEA-222533) GitOrigin-RevId: 8e5cfd1973a0152893fac37d7d50837fc92ca53a --- .../ExplicitArrayFillingInspection.java | 103 +++++++++++++++++- .../ExplicitArrayFilling.html | 2 +- .../afterArrayCopyIsUpdated.java | 18 +++ .../afterArrayIsMethodParam.java | 14 +++ .../explicitArrayFilling/afterInnerLoop.java | 17 +++ .../afterNewlyCreatedArray.java | 9 ++ .../afterUsedAfterLoop.java | 10 ++ .../beforeArrayCopyIsUpdated.java | 18 +++ .../beforeArrayIsMethodParam.java | 14 +++ .../explicitArrayFilling/beforeInnerLoop.java | 17 +++ .../beforeNewlyCreatedArray.java | 12 ++ .../beforeUsedAfterLoop.java | 13 +++ .../src/messages/InspectionsBundle.properties | 1 + 13 files changed, 241 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterArrayCopyIsUpdated.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterArrayIsMethodParam.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/afterUsedAfterLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayCopyIsUpdated.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayIsMethodParam.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java create mode 100644 java/java-tests/testData/inspection/explicitArrayFilling/beforeUsedAfterLoop.java diff --git a/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java b/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java index 875257a43a46..20de7f0bac64 100644 --- a/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/ExplicitArrayFillingInspection.java @@ -1,28 +1,34 @@ // Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection; -import com.intellij.codeInsight.daemon.QuickFixBundle; import com.intellij.codeInsight.intention.QuickFixFactory; 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.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; -import com.intellij.util.ObjectUtils; +import com.intellij.psi.util.PsiUtil; import com.siyeh.ig.psiutils.*; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; +import java.util.HashSet; +import java.util.Set; import java.util.function.Predicate; +import static com.intellij.util.ObjectUtils.tryCast; + public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspectionTool { private static final Logger LOG = Logger.getInstance(ExplicitArrayFillingInspection.class); @@ -55,10 +61,11 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect PsiExpression rValue = assignment.getRExpression(); if (rValue == null) return; if (!isChangedInLoop(loop, rValue)) { - Object constValue = ExpressionUtils.computeConstantExpression(rValue); - if (constValue != null && constValue.equals(PsiTypesUtil.getDefaultValue(assignment.getType()))) { + if (!ControlFlowUtils.isInLoop(statement) && + isDefaultValueAssigned(assignment, rValue) && + isFilledWithDefaultValues(container.getQualifier(), statement)) { holder.registerProblem(statement, getRange(statement, ProblemHighlightType.WARNING), - QuickFixBundle.message("delete.element.fix.text"), + InspectionsBundle.message("inspection.explicit.array.filling.redundant.loop.description"), QuickFixFactory.getInstance().createDeleteFix(statement)); return; } @@ -81,6 +88,90 @@ 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); + return constantValue != null && constantValue.equals(defaultValue); + } + + private boolean isFilledWithDefaultValues(@NotNull PsiExpression expression, @NotNull PsiForStatement forStatement) { + 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)); + 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(); + } + + @Nullable + private Set getDefsStatements(@NotNull PsiElement[] defs) { + Set statements = new HashSet<>(); + for (PsiElement def : defs) { + 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); + 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); + continue; + } + return null; + } + return statements; + } + + private boolean isNewArrayCreation(@Nullable PsiExpression expression) { + expression = PsiUtil.skipParenthesizedExprDown(expression); + return expression == null || expression instanceof PsiNewExpression; + } + private void registerProblem(@NotNull PsiForStatement statement, boolean isSetAll) { String message = InspectionsBundle.message("inspection.explicit.array.filling.description", isSetAll ? "setAll" : "fill"); ReplaceWithArraysCallFix fix = new ReplaceWithArraysCallFix(!isSetAll); @@ -138,7 +229,7 @@ public class ExplicitArrayFillingInspection extends AbstractBaseJavaLocalInspect @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiForStatement statement = ObjectUtils.tryCast(descriptor.getStartElement(), PsiForStatement.class); + PsiForStatement statement = tryCast(descriptor.getStartElement(), PsiForStatement.class); if (statement == null) return; CountingLoop loop = CountingLoop.from(statement); if (loop == null) return; diff --git a/java/java-impl/src/inspectionDescriptions/ExplicitArrayFilling.html b/java/java-impl/src/inspectionDescriptions/ExplicitArrayFilling.html index 17a8f2059584..8cae219ce276 100644 --- a/java/java-impl/src/inspectionDescriptions/ExplicitArrayFilling.html +++ b/java/java-impl/src/inspectionDescriptions/ExplicitArrayFilling.html @@ -1,6 +1,6 @@ -Reports loops which could be replaced with the Arrays.setAll() or Arrays.fill() calls. +Reports loops which could be replaced with Arrays.setAll() or Arrays.fill() calls. This inspection suggests replacing loops with Arrays.setAll() if the language level of the project or module is 8 or higher. Replacing loops with Arrays.fill() is possible with any language level.

For example:

diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayCopyIsUpdated.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayCopyIsUpdated.java new file mode 100644 index 000000000000..27e892df5719 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayCopyIsUpdated.java @@ -0,0 +1,18 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +import java.util.Arrays; + +public class Test { + + public static String[] init(int n) { + String[] lines = new String[n]; + String[] copy = getCopy(lines); + copy[0] = "foo"; + Arrays.fill(lines, null); + return lines; + } + + private static String[] getCopy(String[] original) { + return original; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayIsMethodParam.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayIsMethodParam.java new file mode 100644 index 000000000000..d2584947fcc4 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterArrayIsMethodParam.java @@ -0,0 +1,14 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +import java.util.Arrays; + +public class Test { + + public static int[] init(int[] arr, boolean b) { + if (b) { + arr = new int[10]; + } + 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 new file mode 100644 index 000000000000..44f54faaa3a8 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterInnerLoop.java @@ -0,0 +1,17 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +import java.util.Arrays; + +public class Test { + + public static int[] init(int n) { + int[] data = new int[n]; + int i = 0; + while (i < 3) { + Arrays.fill(data, 0); + if (i < 2) data[n - 1] = 6; + i++; + } + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java new file mode 100644 index 000000000000..472d1cbb7605 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterNewlyCreatedArray.java @@ -0,0 +1,9 @@ +// "Delete element" "true" + +public class Test { + + public static int[] init(int[] arr) { + arr = new int[10]; + return arr; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/afterUsedAfterLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/afterUsedAfterLoop.java new file mode 100644 index 000000000000..0fbd51a0664b --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/afterUsedAfterLoop.java @@ -0,0 +1,10 @@ +// "Delete element" "true" + +public 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/beforeArrayCopyIsUpdated.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayCopyIsUpdated.java new file mode 100644 index 000000000000..cf2f828d9966 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayCopyIsUpdated.java @@ -0,0 +1,18 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +public class Test { + + public static String[] init(int n) { + String[] lines = new String[n]; + String[] copy = getCopy(lines); + copy[0] = "foo"; + for (int i = 0; i < lines.length; i++) { + lines[i] = null; + } + return lines; + } + + private static String[] getCopy(String[] original) { + return original; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayIsMethodParam.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayIsMethodParam.java new file mode 100644 index 000000000000..72ed955c16a3 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeArrayIsMethodParam.java @@ -0,0 +1,14 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +public class Test { + + public static int[] init(int[] arr, boolean b) { + if (b) { + arr = new int[10]; + } + 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 new file mode 100644 index 000000000000..bae338170110 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeInnerLoop.java @@ -0,0 +1,17 @@ +// "Replace loop with 'Arrays.fill()' method call" "true" + +public class Test { + + public static int[] init(int n) { + int[] data = new int[n]; + int i = 0; + while (i < 3) { + for (int j = 0; j < data.length; j++) { + data[j] = 0; + } + if (i < 2) data[n - 1] = 6; + i++; + } + return data; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java new file mode 100644 index 000000000000..7a744df74a81 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeNewlyCreatedArray.java @@ -0,0 +1,12 @@ +// "Delete element" "true" + +public class Test { + + public static int[] init(int[] arr) { + arr = new int[10]; + 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/beforeUsedAfterLoop.java b/java/java-tests/testData/inspection/explicitArrayFilling/beforeUsedAfterLoop.java new file mode 100644 index 000000000000..66568afaf709 --- /dev/null +++ b/java/java-tests/testData/inspection/explicitArrayFilling/beforeUsedAfterLoop.java @@ -0,0 +1,13 @@ +// "Delete element" "true" + +public class Test { + + public static int[] init(int n, boolean b) { + int[] data = new int[n]; + for (int j = 0; j < data.length; j++) { + data[j] = 0; + } + data[n - 1] = 6; + return data; + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 66eda5d55877..b751515bb8ea 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -842,6 +842,7 @@ inspection.replace.with.bulk.fix.name=Replace iteration with bulk ''{0}'' call inspection.replace.with.bulk.fix.family.name=Replace with bulk method call inspection.replace.with.bulk.wrap.arrays=Use Arrays.asList() to wrap arrays inspection.explicit.array.filling.fix.family.name=Replace loop with ''Arrays.{0}()'' method call +inspection.explicit.array.filling.redundant.loop.description=Redundant initialization of a newly created array inspection.explicit.array.filling.description=Can be replaced with single ''Arrays.{0}()'' method call inspection.explicit.array.filling.suggest.set.all=Suggest 'Arrays.setAll()' inspection.explicit.array.filling.no.suggestion.for.set.all=Do not suggest to use 'Arrays.setAll()'