diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index c4247499a5c2..b5a6822ab1d8 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1585,9 +1585,12 @@ new.string.buffer.with.char.argument.quickfix=Replace char argument with String suspicious.comparator.compare.display.name=Suspicious 'Comparator.compare()' implementation suspicious.comparator.compare.descriptor.parameter.not.used='compare()' parameter #ref is not used #loc suspicious.comparator.compare.descriptor.non.reflexive=Comparator does not return 0 for equal elements -to.array.call.with.zero.length.array.argument.display.name=Call to 'Collection.toArray()' with zero-length array argument -to.array.call.with.zero.length.array.argument.problem.descriptor=Call to #ref() with zero-length array argument ''{0}'' #loc -to.array.call.with.zero.length.array.argument.quickfix=Replace argument with correctly sized array +to.array.call.style.display.name='Collection.toArray()' call style +to.array.call.style.problem.descriptor.zero=Call to #ref() with empty array argument ''{0}'' #loc +to.array.call.style.problem.descriptor.presized=Call to #ref() with pre-sized array argument ''{0}'' #loc +to.array.call.style.quickfix.family.name=Fix size of the array passed to 'toArray' call +to.array.call.style.quickfix.make.presized=Replace argument with pre-sized array +to.array.call.style.quickfix.make.zero=Replace argument with empty array throwable.instance.never.thrown.runtime.exception.problem.descriptor=Runtime exception instance #ref is not thrown #loc throwable.instance.never.thrown.checked.exception.problem.descriptor=Checked exception instance #ref is not thrown #loc throwable.instance.never.thrown.error.problem.descriptor=Error instance #ref is not thrown #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ToArrayCallWithZeroLengthArrayArgumentInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ToArrayCallWithZeroLengthArrayArgumentInspectionBase.java deleted file mode 100644 index f8f04001924a..000000000000 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/ToArrayCallWithZeroLengthArrayArgumentInspectionBase.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2000-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.siyeh.ig.performance; - -import com.intellij.psi.*; -import com.intellij.psi.util.InheritanceUtil; -import com.siyeh.InspectionGadgetsBundle; -import com.siyeh.ig.BaseInspection; -import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.CollectionUtils; -import com.siyeh.ig.psiutils.ConstructionUtils; -import org.jetbrains.annotations.Nls; -import org.jetbrains.annotations.NonNls; -import org.jetbrains.annotations.NotNull; - -public class ToArrayCallWithZeroLengthArrayArgumentInspectionBase extends BaseInspection { - @Override - @Nls - @NotNull - public String getDisplayName() { - return InspectionGadgetsBundle.message( - "to.array.call.with.zero.length.array.argument.display.name"); - } - - @Override - @NotNull - protected String buildErrorString(Object... infos) { - final PsiExpression argument = (PsiExpression)infos[1]; - return InspectionGadgetsBundle.message( - "to.array.call.with.zero.length.array.argument.problem.descriptor", - argument.getText()); - } - - @Override - public BaseInspectionVisitor buildVisitor() { - return new ToArrayCallWithZeroLengthArrayArgument(); - } - - private static class ToArrayCallWithZeroLengthArrayArgument extends BaseInspectionVisitor { - - @Override - public void visitMethodCallExpression(PsiMethodCallExpression expression) { - super.visitMethodCallExpression(expression); - final PsiReferenceExpression methodExpression = expression.getMethodExpression(); - @NonNls final String methodName = methodExpression.getReferenceName(); - if (!"toArray".equals(methodName)) { - return; - } - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 1) { - return; - } - final PsiExpression argument = arguments[0]; - final PsiType type = argument.getType(); - if (!(type instanceof PsiArrayType)) { - return; - } - if (type.getArrayDimensions() != 1) { - return; - } - if (argument instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)argument; - final PsiElement element = referenceExpression.resolve(); - if (!(element instanceof PsiField)) { - return; - } - final PsiField field = (PsiField)element; - if (!CollectionUtils.isConstantEmptyArray(field)) { - return; - } - } - else if (!ConstructionUtils.isEmptyArrayInitializer(argument)) { - return; - } - final PsiMethod method = expression.resolveMethod(); - if (method == null) { - return; - } - final PsiClass containingClass = method.getContainingClass(); - if (!InheritanceUtil.isInheritor(containingClass, CommonClassNames.JAVA_UTIL_COLLECTION)) { - return; - } - registerMethodCallError(expression, expression, argument); - } - } -} diff --git a/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml b/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml index ee9598d76068..5c3342634ec2 100644 --- a/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml +++ b/plugins/InspectionGadgets/src/META-INF/InspectionGadgets.xml @@ -2033,8 +2033,8 @@ groupBundle="messages.InspectionsBundle" groupKey="group.names.performance.issues" enabledByDefault="true" level="INFORMATION" implementationClass="com.siyeh.ig.performance.TailRecursionInspection"/> myMode = mode); + panel.add(radioButton); + group.add(radioButton); + } + + return panel; + } @Override @Nullable protected InspectionGadgetsFix buildFix(Object... infos) { - return new ToArrayCallWithZeroLengthArrayArgumentFix(); + final PsiExpression argument = (PsiExpression)infos[1]; + return new ToArrayCallWithZeroLengthArrayArgumentFix(myMode.isEmptyPreferred(argument)); + } + + @Override + @Nls + @NotNull + public String getDisplayName() { + return InspectionGadgetsBundle.message("to.array.call.style.display.name"); + } + + @Override + public void readSettings(@NotNull Element node) { + Element element = node.getChild(PREFER_EMPTY_ARRAY_SETTING); + if (element != null) { + myMode = PreferEmptyArray.from(element.getAttributeValue("value")); + } + } + + @Override + public void writeSettings(@NotNull Element node) { + Element element = new Element(PREFER_EMPTY_ARRAY_SETTING); + element.setAttribute("value", myMode.toString()); + node.addContent(element); + } + + @Override + @NotNull + protected String buildErrorString(Object... infos) { + final PsiExpression argument = (PsiExpression)infos[1]; + return myMode.isEmptyPreferred(argument) ? + InspectionGadgetsBundle.message("to.array.call.style.problem.descriptor.presized", argument.getText()) : + InspectionGadgetsBundle.message("to.array.call.style.problem.descriptor.zero", argument.getText()); + } + + @Override + public BaseInspectionVisitor buildVisitor() { + return new BaseInspectionVisitor() { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + @NonNls final String methodName = methodExpression.getReferenceName(); + if (!"toArray".equals(methodName)) return; + final PsiExpressionList argumentList = expression.getArgumentList(); + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length != 1) return; + final PsiExpression argument = arguments[0]; + final PsiType type = argument.getType(); + if (!(type instanceof PsiArrayType)) return; + final PsiMethod method = expression.resolveMethod(); + if (method == null) return; + final PsiClass containingClass = method.getContainingClass(); + if (!InheritanceUtil.isInheritor(containingClass, CommonClassNames.JAVA_UTIL_COLLECTION)) return; + if (type.getArrayDimensions() != 1) return; + + boolean wrongArray = + myMode.isEmptyPreferred(argument) + ? isPresizedArray(argument, methodExpression.getQualifierExpression()) + : isEmptyArray(argument); + if (wrongArray) { + registerMethodCallError(expression, expression, argument); + } + } + }; + } + + private static boolean isEmptyArray(@Nullable PsiExpression argument) { + if (argument instanceof PsiReferenceExpression) { + final PsiElement element = ((PsiReferenceExpression)argument).resolve(); + if (!(element instanceof PsiField)) return false; + return CollectionUtils.isConstantEmptyArray((PsiField)element); + } + return ConstructionUtils.isEmptyArrayInitializer(argument); + } + + @Contract("_, null -> false") + private static boolean isPresizedArray(@Nullable PsiExpression argument, @Nullable PsiExpression qualifier) { + if (qualifier == null) return false; + PsiNewExpression newExpression = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(argument), PsiNewExpression.class); + if (newExpression == null) return false; + PsiExpression[] dimensions = newExpression.getArrayDimensions(); + if (dimensions.length != 1) return false; + PsiMethodCallExpression maybeSizeCall = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(dimensions[0]), PsiMethodCallExpression.class); + if (COLLECTION_SIZE.test(maybeSizeCall)) { + PsiExpression sizeQualifier = maybeSizeCall.getMethodExpression().getQualifierExpression(); + return sizeQualifier != null && PsiEquivalenceUtil.areElementsEquivalent(sizeQualifier, qualifier); + } + return false; } private static class ToArrayCallWithZeroLengthArrayArgumentFix extends InspectionGadgetsFix { + private boolean myEmptyPreferred; + + public ToArrayCallWithZeroLengthArrayArgumentFix(boolean emptyPreferred) { + myEmptyPreferred = emptyPreferred; + } + + @Nls + @NotNull + @Override + public String getName() { + return myEmptyPreferred ? + InspectionGadgetsBundle.message("to.array.call.style.quickfix.make.zero") : + InspectionGadgetsBundle.message("to.array.call.style.quickfix.make.presized"); + } @Override @NotNull public String getFamilyName() { - return InspectionGadgetsBundle.message("to.array.call.with.zero.length.array.argument.quickfix"); + return InspectionGadgetsBundle.message("to.array.call.style.quickfix.family.name"); } @Override @@ -49,44 +210,36 @@ public class ToArrayCallWithZeroLengthArrayArgumentInspection extends ToArrayCal final PsiElement element = descriptor.getPsiElement(); final PsiElement parent = element.getParent(); final PsiElement grandParent = parent.getParent(); - if (!(grandParent instanceof PsiMethodCallExpression)) { - return; - } + if (!(grandParent instanceof PsiMethodCallExpression)) return; final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)grandParent; final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); final PsiExpression qualifier = methodExpression.getQualifierExpression(); final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 1) { - return; - } + if (arguments.length != 1) return; final PsiExpression argument = arguments[0]; - if (qualifier == null) { - return; - } + if (qualifier == null) return; + final String collectionText = qualifier.getText(); final PsiType type = argument.getType(); - if (type == null) { - return; - } + if (type == null) return; final PsiType componentType = type.getDeepComponentType(); final String typeText = componentType.getCanonicalText(); - if (!(qualifier instanceof PsiMethodCallExpression)) { - @NonNls final String replacementText = "new " + typeText + '[' + collectionText + ".size()]"; - final String newExpressionText = PsiReplacementUtil.getElementText(methodCallExpression, argument, replacementText); - PsiReplacementUtil.replaceExpression(methodCallExpression, newExpressionText); + + + if (myEmptyPreferred || ExpressionUtils.isSimpleExpression(qualifier)) { + CommentTracker ct = new CommentTracker(); + String sizeClause = myEmptyPreferred ? "0" : collectionText + ".size()"; + @NonNls final String replacementText = "new " + typeText + '[' + sizeClause + "]"; + ct.replaceAndRestoreComments(argument, replacementText); return; } // need to introduce a variable to prevent calling a method twice PsiStatement statement = PsiTreeUtil.getParentOfType(methodCallExpression, PsiStatement.class); - if (statement == null) { - return; - } + if (statement == null) return; final PsiType qualifierType = qualifier.getType(); - if (qualifierType == null) { - return; - } + if (qualifierType == null) return; PsiDeclarationStatement declarationStatement = factory.createVariableDeclarationStatement("var", qualifierType, qualifier); PsiElement statementParent = statement.getParent(); while (statementParent instanceof PsiLoopStatement || statementParent instanceof PsiIfStatement) { diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ToArrayCallWithZeroLengthArrayArgument.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ToArrayCallWithZeroLengthArrayArgument.html index 2a30b9744bbd..bb467bd4ffcb 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/ToArrayCallWithZeroLengthArrayArgument.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ToArrayCallWithZeroLengthArrayArgument.html @@ -1,13 +1,22 @@ -Reports any call to toArray() -on an object of type or subtype java.util.Collection -with a zero-length array argument. When passing in an array of too small size, the -toArray() method has to construct a new array of -the right size using reflection. On older JVMs this has worse performance than passing -in an array of at least the size of the collection itself. - +There are two styles to convert collection to array: either using a pre-sized array +(like c.toArray(new String[c.size()])) or using an empty array (like +c.toArray(new String[0]).

- + In older Java versions using pre-sized array was recommended as a reflection + call which is necessary to create an array of proper size was quite slow. + However since late updates of OpenJDK 6 this call was intrinsified making + the performance of the empty array version the same and sometimes even better, comparing + to the pre-sized version. Also passing pre-sized array is dangerous for concurrent or + synchronized collection as data race is possible between the size and toArray + call which may result in extra nulls at the end of the array if the collection was concurrently + shrinked during the operation. +

+

+ This inspection allows to follow the uniform style: either using an empty array + (which is recommended in modern Java) + or using a pre-sized array (which might be faster in older Java versions or some non-HotSpot based JVMs). +

\ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.after.java new file mode 100644 index 000000000000..1a00a97c84df --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.after.java @@ -0,0 +1,17 @@ +package com.siyeh.igfixes.performance.to_array_call_with_zero_length_array_argument; + +import java.util.List; + +class IntroduceVariable { + + static List someFunc() + { + return null; + } + + public static void main(String... args) + + { + String[] foo = someFunc().toArray(new String[0]); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.java new file mode 100644 index 000000000000..7c9e375e2fde --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/to_array_call_with_zero_length_array_argument/PresizedToZero.java @@ -0,0 +1,17 @@ +package com.siyeh.igfixes.performance.to_array_call_with_zero_length_array_argument; + +import java.util.List; + +class IntroduceVariable { + + static List someFunc() + { + return null; + } + + public static void main(String... args) + + { + String[] foo = someFunc().toArray(new String[someFunc().size()]); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ToArrayCallWithZeroLengthArrayArgumentFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ToArrayCallWithZeroLengthArrayArgumentFixTest.java index 2373c08e17ba..ff0188f927cf 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ToArrayCallWithZeroLengthArrayArgumentFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/ToArrayCallWithZeroLengthArrayArgumentFixTest.java @@ -18,21 +18,28 @@ package com.siyeh.ig.fixes.performance; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.IGQuickFixesTestCase; import com.siyeh.ig.performance.ToArrayCallWithZeroLengthArrayArgumentInspection; +import com.siyeh.ig.performance.ToArrayCallWithZeroLengthArrayArgumentInspection.PreferEmptyArray; public class ToArrayCallWithZeroLengthArrayArgumentFixTest extends IGQuickFixesTestCase { + private ToArrayCallWithZeroLengthArrayArgumentInspection myInspection = new ToArrayCallWithZeroLengthArrayArgumentInspection(); @Override protected void setUp() throws Exception { super.setUp(); - myFixture.enableInspections(new ToArrayCallWithZeroLengthArrayArgumentInspection()); + myFixture.enableInspections(myInspection); } public void testIntroduceVariable() { - doFixTest(); + doFixTest(PreferEmptyArray.NEVER, InspectionGadgetsBundle.message("to.array.call.style.quickfix.make.presized")); } - private void doFixTest() { - doTest(getTestName(false), InspectionGadgetsBundle.message("to.array.call.with.zero.length.array.argument.quickfix")); + public void testPresizedToZero() { + doFixTest(PreferEmptyArray.ALWAYS, InspectionGadgetsBundle.message("to.array.call.style.quickfix.make.zero")); + } + + private void doFixTest(PreferEmptyArray mode, String message) { + myInspection.myMode = mode; + doTest(getTestName(false), message); } @Override