From 85fc490df4a0b22b87d2f599749ebe9f40b9bb52 Mon Sep 17 00:00:00 2001 From: Artemiy Sartakov Date: Wed, 20 Feb 2019 16:40:42 +0700 Subject: [PATCH] Unnecessary boxing: added support for static valueOf import, fixed problem with clashing (IDEA-CR-43627) --- .../WrapperTypeMayBePrimitiveInspection.java | 13 +--- .../siyeh/InspectionGadgetsBundle.properties | 1 - .../UnnecessaryBoxingInspection.java | 61 +++++++++++-------- ...oraryOnConversionFromStringInspection.java | 44 ++++--------- .../siyeh/ig/psiutils/JavaPsiBoxingUtils.java | 31 ++++++++++ .../unnecessary_boxing/ShadowImport.java | 13 ++++ .../StaticImport.after.java | 7 +++ .../unnecessary_boxing/StaticImport.java | 7 +++ .../IntegerIntValue.after.java | 2 +- .../migration/UnnecessaryBoxingFixTest.java | 11 +++- 10 files changed, 119 insertions(+), 71 deletions(-) create mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/JavaPsiBoxingUtils.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/ShadowImport.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.java diff --git a/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java b/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java index 09ab1e3470da..2af61728f618 100644 --- a/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/WrapperTypeMayBePrimitiveInspection.java @@ -16,6 +16,7 @@ import com.intellij.util.ArrayUtil; import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.CommentTracker; import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.JavaPsiBoxingUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -28,19 +29,9 @@ public class WrapperTypeMayBePrimitiveInspection extends AbstractBaseJavaLocalIn private static final CallMatcher HASH_CODE = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "hashCode"); private static final CallMatcher VALUE_OF = getValueOfMatcher(); - private static final Map ourReplacementMap = new HashMap<>(); - private static final Set ourAllowedInstanceCalls = new HashSet<>(); static { - ourReplacementMap.put(CommonClassNames.JAVA_LANG_INTEGER, "parseInt"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_LONG, "parseLong"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_FLOAT, "parseFloat"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_BOOLEAN, "parseBoolean"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_DOUBLE, "parseDouble"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_SHORT, "parseShort"); - ourReplacementMap.put(CommonClassNames.JAVA_LANG_BYTE, "parseByte"); - ourAllowedInstanceCalls.add("isInfinite"); ourAllowedInstanceCalls.add("isNaN"); ourAllowedInstanceCalls.add("byteValue"); @@ -340,7 +331,7 @@ public class WrapperTypeMayBePrimitiveInspection extends AbstractBaseJavaLocalIn PsiExpression argument = arguments[0]; if (containingClass == null) return; String containingClassName = containingClass.getQualifiedName(); - String replacementMethodCall = ourReplacementMap.get(containingClassName); + String replacementMethodCall = JavaPsiBoxingUtils.getParseMethod(containingClassName); if (replacementMethodCall == null) return; CommentTracker tracker = new CommentTracker(); String argumentText = tracker.text(argument); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index e9e47ae8c30c..b548b6f3e264 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1136,7 +1136,6 @@ for.loop.with.missing.component.problem.descriptor6=#ref statement for.loop.with.missing.component.problem.descriptor7=#ref statement lacks initializer, condition and update #loc foreach.replace.quickfix=Replace with 'foreach' unnecessary.boxing.remove.quickfix=Remove boxing -unnecessary.boxing.use.parse.quickfix=Replace with ''{0}'' method call unnecessary.unboxing.remove.quickfix=Remove unboxing misordered.assert.equals.arguments.flip.quickfix=Flip compared arguments simplify.junit.assertion.simplify.quickfix=Simplify assertion diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java index 450663bd6fa6..86d2c1255e29 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java @@ -15,6 +15,7 @@ */ package com.siyeh.ig.migration; +import com.intellij.codeInspection.CommonQuickFixBundle; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; @@ -23,7 +24,7 @@ import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; -import com.intellij.util.containers.hash.HashMap; +import com.intellij.util.ObjectUtils; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; @@ -35,23 +36,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; -import java.util.Map; public class UnnecessaryBoxingInspection extends BaseInspection { @SuppressWarnings("PublicField") public boolean onlyReportSuperfluouslyBoxed = false; - private static final Map replacementMap = new HashMap<>(); - - static { - replacementMap.put(PsiType.SHORT, "Short.parseShort"); - replacementMap.put(PsiType.INT, "Integer.parseInt"); - replacementMap.put(PsiType.LONG, "Long.parseLong"); - replacementMap.put(PsiType.DOUBLE, "Double.parseDouble"); - replacementMap.put(PsiType.FLOAT, "Float.parseFloat"); - } - @Override @NotNull public String getDisplayName() { @@ -92,8 +82,8 @@ public class UnnecessaryBoxingInspection extends BaseInspection { this.name = InspectionGadgetsBundle.message("unnecessary.boxing.remove.quickfix"); } - private UnnecessaryBoxingFix(PsiType expectedType) { - this.name = InspectionGadgetsBundle.message("unnecessary.boxing.use.parse.quickfix", replacementMap.get(expectedType)); + private UnnecessaryBoxingFix(PsiType retType) { + this.name = CommonQuickFixBundle.message("fix.replace.with.x", getParseMethod(retType)); } @Override @@ -120,12 +110,12 @@ public class UnnecessaryBoxingInspection extends BaseInspection { } final CommentTracker commentTracker = new CommentTracker(); if (unboxedExpressionType.getCanonicalText().equals("java.lang.String")) { - final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false, true); - final String parseMethodName = replacementMap.get(expectedType); + PsiMethodCallExpression methodCall = (PsiMethodCallExpression)expression; + final String parseMethodName = getParseMethod(methodCall.getType()); if (parseMethodName == null) { return; } - PsiReplacementUtil.replaceExpression(expression, parseMethodName + "(" + unboxedExpression.getText() + ")", commentTracker); + ExpressionUtils.bindCallTo(methodCall, parseMethodName); return; } final Object value = ExpressionUtils.computeConstantExpression(unboxedExpression); @@ -256,24 +246,31 @@ public class UnnecessaryBoxingInspection extends BaseInspection { if (!"valueOf".equals(referenceName)) { return; } + final PsiMethod method = ObjectUtils.tryCast(methodExpression.resolve(), PsiMethod.class); + if (method == null) { + return; + } + final PsiClass aClass = method.getContainingClass(); + if (aClass == null) { + return; + } + final String canonicalText = aClass.getQualifiedName(); + if (PsiTypesUtil.unboxIfPossible(canonicalText) == canonicalText) { + return; + } final PsiType boxedExpressionType = boxedExpression.getType(); if (boxedExpressionType != null && boxedExpressionType.getCanonicalText().equals("java.lang.String")) { final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false, true); - if (replacementMap.containsKey(expectedType)) { - registerError(expression, expectedType); + final PsiType methodReturnType = method.getReturnType(); + if (expectedType instanceof PsiPrimitiveType && getParseMethod(methodReturnType) != null) { + registerError(expression, methodReturnType); } return; } if (!(boxedExpressionType instanceof PsiPrimitiveType)) { return; } - final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); - if (!(qualifierExpression instanceof PsiReferenceExpression)) { - return; - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifierExpression; - final String canonicalText = referenceExpression.getCanonicalText(); - if (PsiTypesUtil.unboxIfPossible(canonicalText) == canonicalText || isBoxingNecessary(expression, boxedExpression)) { + if (isBoxingNecessary(expression, boxedExpression)) { return; } if (onlyReportSuperfluouslyBoxed) { @@ -352,4 +349,16 @@ public class UnnecessaryBoxingInspection extends BaseInspection { return false; } } + + @Nullable + private static String getParseMethod(@Nullable PsiType type) { + if (type == null) { + return null; + } + final String typeText = type.getCanonicalText(); + if (CommonClassNames.JAVA_LANG_BOOLEAN.equals(typeText) || CommonClassNames.JAVA_LANG_BYTE.equals(typeText)) { + return null; + } + return JavaPsiBoxingUtils.getParseMethod(typeText); + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/UnnecessaryTemporaryOnConversionFromStringInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/UnnecessaryTemporaryOnConversionFromStringInspection.java index 0e9a907176f3..219f3170b3e6 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/UnnecessaryTemporaryOnConversionFromStringInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/UnnecessaryTemporaryOnConversionFromStringInspection.java @@ -27,6 +27,7 @@ import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.JavaPsiBoxingUtils; import com.siyeh.ig.psiutils.TypeUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -38,21 +39,6 @@ import java.util.Map; public class UnnecessaryTemporaryOnConversionFromStringInspection extends BaseInspection { - /** - */ - @NonNls private static final Map s_conversionMap = - new HashMap<>(7); - - static { - s_conversionMap.put(CommonClassNames.JAVA_LANG_BOOLEAN, "valueOf"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_BYTE, "parseByte"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_DOUBLE, "parseDouble"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_FLOAT, "parseFloat"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_INTEGER, "parseInt"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_LONG, "parseLong"); - s_conversionMap.put(CommonClassNames.JAVA_LANG_SHORT, "parseShort"); - } - @Override @NotNull public String getDisplayName() { @@ -68,7 +54,7 @@ public class UnnecessaryTemporaryOnConversionFromStringInspection @Override @NotNull public String buildErrorString(Object... infos) { - final String replacementString = calculateReplacementExpression((PsiMethodCallExpression)infos[0], new CommentTracker()); + final String replacementString = calculateReplacementExpression((PsiMethodCallExpression)infos[0], new CommentTracker(), false); return InspectionGadgetsBundle.message( "unnecessary.temporary.on.conversion.from.string.problem.descriptor", replacementString); @@ -76,7 +62,9 @@ public class UnnecessaryTemporaryOnConversionFromStringInspection @Nullable @NonNls - static String calculateReplacementExpression(PsiMethodCallExpression expression, CommentTracker commentTracker) { + static String calculateReplacementExpression(PsiMethodCallExpression expression, + CommentTracker commentTracker, + boolean isFullyQualified) { final PsiReferenceExpression methodExpression = expression.getMethodExpression(); final PsiNewExpression qualifier = ObjectUtils.tryCast(methodExpression.getQualifierExpression(), PsiNewExpression.class); if (qualifier == null) return null; @@ -87,24 +75,18 @@ public class UnnecessaryTemporaryOnConversionFromStringInspection if (type == null) return null; final String qualifierType = type.getPresentableText(); final String canonicalType = type.getCanonicalText(); - final String conversionName = s_conversionMap.get(canonicalType); - if (TypeUtils.typeEquals(CommonClassNames.JAVA_LANG_BOOLEAN, type)) { - if (!PsiUtil.isLanguageLevel5OrHigher(expression)) { - return qualifierType + '.' + conversionName + '(' + commentTracker.text(arg) + ").booleanValue()"; - } - else { - return qualifierType + ".parseBoolean(" + commentTracker.text(arg) + ')'; - } - } - else { - return qualifierType + '.' + conversionName + '(' + commentTracker.text(arg) + ')'; + final String name = isFullyQualified ? canonicalType : qualifierType; + final String conversionName = JavaPsiBoxingUtils.getParseMethod(canonicalType); + if (TypeUtils.typeEquals(CommonClassNames.JAVA_LANG_BOOLEAN, type) && !PsiUtil.isLanguageLevel5OrHigher(expression)) { + return name + '.' + "valueOf" + '(' + commentTracker.text(arg) + ").booleanValue()"; } + return name + '.' + conversionName + '(' + commentTracker.text(arg) + ')'; } @Override @Nullable public InspectionGadgetsFix buildFix(Object... infos) { - final String replacementExpression = calculateReplacementExpression((PsiMethodCallExpression)infos[0], new CommentTracker()); + final String replacementExpression = calculateReplacementExpression((PsiMethodCallExpression)infos[0], new CommentTracker(), false); if (replacementExpression == null) return null; final String name = CommonQuickFixBundle.message("fix.replace.with.x", replacementExpression); return new UnnecessaryTemporaryObjectFix(name); @@ -134,8 +116,8 @@ public class UnnecessaryTemporaryOnConversionFromStringInspection @Override public void doFix(Project project, ProblemDescriptor descriptor) { final PsiMethodCallExpression expression = (PsiMethodCallExpression)descriptor.getPsiElement(); - CommentTracker commentTracker = new CommentTracker(); - final String newExpression = calculateReplacementExpression(expression, commentTracker); + final CommentTracker commentTracker = new CommentTracker(); + final String newExpression = calculateReplacementExpression(expression, commentTracker, true); if (newExpression == null) return; PsiReplacementUtil.replaceExpression(expression, newExpression, commentTracker); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/JavaPsiBoxingUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/JavaPsiBoxingUtils.java new file mode 100644 index 000000000000..5a0df5279b37 --- /dev/null +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/JavaPsiBoxingUtils.java @@ -0,0 +1,31 @@ +// 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.siyeh.ig.psiutils; + +import com.intellij.psi.CommonClassNames; +import com.intellij.util.containers.hash.HashMap; +import org.jetbrains.annotations.Nullable; + +import java.util.Map; + +public class JavaPsiBoxingUtils { + + private static final Map parseMethodsMap = new HashMap<>(); + + static { + parseMethodsMap.put(CommonClassNames.JAVA_LANG_INTEGER, "parseInt"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_LONG, "parseLong"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_FLOAT, "parseFloat"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_BOOLEAN, "parseBoolean"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_DOUBLE, "parseDouble"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_SHORT, "parseShort"); + parseMethodsMap.put(CommonClassNames.JAVA_LANG_BYTE, "parseByte"); + } + + /** + * Get parse method name without qualifier for given full class name. + */ + @Nullable + public static String getParseMethod(@Nullable String className) { + return parseMethodsMap.get(className); + } +} diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/ShadowImport.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/ShadowImport.java new file mode 100644 index 000000000000..9cc0e1926523 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/ShadowImport.java @@ -0,0 +1,13 @@ +package foo.bar; + +class Test { + void test(String str) { + int i = Integer.valueOf(str); + } +} + +class Integer { + static int valueOf(String str) { + return 42; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.after.java new file mode 100644 index 000000000000..5a6c59ed2b40 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.after.java @@ -0,0 +1,7 @@ +import static java.lang.Integer.valueOf; + +class Test { + void test(String str) { + int i = Integer.parseInt(str); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.java new file mode 100644 index 000000000000..5258942ace2b --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/migration/unnecessary_boxing/StaticImport.java @@ -0,0 +1,7 @@ +import static java.lang.Integer.valueOf; + +class Test { + void test(String str) { + int i = valueOf(str); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/unnecessary_temporary_from_string/IntegerIntValue.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/unnecessary_temporary_from_string/IntegerIntValue.after.java index eddf6b740ef3..f86c0de9148e 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/unnecessary_temporary_from_string/IntegerIntValue.after.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/unnecessary_temporary_from_string/IntegerIntValue.after.java @@ -1,6 +1,6 @@ class Test { void test(String s) { /*hello*/ - int x = Integer.parseInt(s +/*double-s*/s); + int x = Integer.parseInt(s +/*double-s*/s); } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/UnnecessaryBoxingFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/UnnecessaryBoxingFixTest.java index 59cb6bb04e94..aabc9366c904 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/UnnecessaryBoxingFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/UnnecessaryBoxingFixTest.java @@ -1,6 +1,7 @@ // Copyright 2000-2018 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.siyeh.ig.fixes.migration; +import com.intellij.codeInspection.CommonQuickFixBundle; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.IGQuickFixesTestCase; import com.siyeh.ig.migration.UnnecessaryBoxingInspection; @@ -89,7 +90,15 @@ public class UnnecessaryBoxingFixTest extends IGQuickFixesTestCase { } public void testParseInt() { - doTest(getTestName(false), InspectionGadgetsBundle.message("unnecessary.boxing.use.parse.quickfix", "Integer.parseInt")); + doTest(CommonQuickFixBundle.message("fix.replace.with.x", "parseInt")); + } + + public void testStaticImport() { + doTest(CommonQuickFixBundle.message("fix.replace.with.x", "parseInt")); + } + + public void testShadowImport() { + assertQuickfixNotAvailable("Fix all 'Unnecessary boxing' problems in file"); } private void doFixTest() {