From eb9a0abe703b0e77f24455dfb8ed616942fb6814 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Fri, 11 Apr 2014 18:22:56 +0200 Subject: [PATCH] IDEA-123838 Iteration over 'keySet()' may be replaced with 'entrySet()' iteration quick fix produces wrong code --- ...ySetIterationMayUseEntrySetInspection.java | 115 ++++++------------ .../key_set_with_entry_set/Simple.after.java | 11 ++ .../key_set_with_entry_set/Simple.java | 11 ++ .../KeySetIterationMayUseEntrySetFixTest.java | 32 +++++ 4 files changed, 91 insertions(+), 78 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/KeySetIterationMayUseEntrySetFixTest.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/KeySetIterationMayUseEntrySetInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/KeySetIterationMayUseEntrySetInspection.java index 6979f12550b5..41c1f9516381 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/KeySetIterationMayUseEntrySetInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/performance/KeySetIterationMayUseEntrySetInspection.java @@ -61,8 +61,7 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { return new KeySetIterationMapUseEntrySetFix(); } - private static class KeySetIterationMapUseEntrySetFix - extends InspectionGadgetsFix { + private static class KeySetIterationMapUseEntrySetFix extends InspectionGadgetsFix { @Override @NotNull public String getFamilyName() { @@ -72,13 +71,11 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { @Override @NotNull public String getName() { - return InspectionGadgetsBundle.message( - "key.set.iteration.may.use.entry.set.quickfix"); + return InspectionGadgetsBundle.message("key.set.iteration.may.use.entry.set.quickfix"); } @Override - protected void doFix(Project project, ProblemDescriptor descriptor) - throws IncorrectOperationException { + protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { final PsiElement element = descriptor.getPsiElement(); final PsiElement parent = element.getParent(); if (!(parent instanceof PsiForeachStatement)) { @@ -86,8 +83,7 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { } final PsiElement map; if (element instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)element; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)element; final PsiElement target = referenceExpression.resolve(); if (!(target instanceof PsiVariable)) { return; @@ -97,46 +93,34 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { if (!(initializer instanceof PsiMethodCallExpression)) { return; } - final PsiMethodCallExpression methodCallExpression = - (PsiMethodCallExpression)initializer; - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); - final PsiExpression qualifier = - methodExpression.getQualifierExpression(); + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)initializer; + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); if (!(qualifier instanceof PsiReferenceExpression)) { return; } - final PsiReferenceExpression reference = - (PsiReferenceExpression)qualifier; + final PsiReferenceExpression reference = (PsiReferenceExpression)qualifier; map = reference.resolve(); final String qualifierText = qualifier.getText(); - PsiReplacementUtil.replaceExpression(referenceExpression, - qualifierText + ".entrySet()"); + PsiReplacementUtil.replaceExpression(referenceExpression, qualifierText + ".entrySet()"); } else if (element instanceof PsiMethodCallExpression) { - final PsiMethodCallExpression methodCallExpression = - (PsiMethodCallExpression)element; - final PsiReferenceExpression methodExpression = - methodCallExpression.getMethodExpression(); - final PsiExpression qualifier = - methodExpression.getQualifierExpression(); + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)element; + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + final PsiExpression qualifier = methodExpression.getQualifierExpression(); if (!(qualifier instanceof PsiReferenceExpression)) { return; } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)qualifier; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; map = referenceExpression.resolve(); final String qualifierText = qualifier.getText(); - PsiReplacementUtil.replaceExpression(methodCallExpression, - qualifierText + ".entrySet()"); + PsiReplacementUtil.replaceExpression(methodCallExpression, qualifierText + ".entrySet()"); } else { return; } - final PsiForeachStatement foreachStatement = - (PsiForeachStatement)parent; - final PsiExpression iteratedValue = - foreachStatement.getIteratedValue(); + final PsiForeachStatement foreachStatement = (PsiForeachStatement)parent; + final PsiExpression iteratedValue = foreachStatement.getIteratedValue(); if (iteratedValue == null) { return; } @@ -146,35 +130,24 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { } final PsiClassType classType = (PsiClassType)type; final PsiType[] parameterTypes = classType.getParameters(); - if (parameterTypes.length != 1) { - return; - } - PsiType parameterType = parameterTypes[0]; + PsiType parameterType = parameterTypes.length == 1 ? parameterTypes[0] : null; boolean insertCast = false; if (parameterType == null) { parameterType = TypeUtils.getObjectType(foreachStatement); insertCast = true; } - final PsiParameter parameter = - foreachStatement.getIterationParameter(); - final String variableName = - createNewVariableName(foreachStatement, parameterType); + final PsiParameter parameter = foreachStatement.getIterationParameter(); + final String variableName = createNewVariableName(foreachStatement, parameterType); if (insertCast) { - replaceParameterAccess(parameter, - "((Map.Entry)" + variableName + ')', map, - foreachStatement); + replaceParameterAccess(parameter, "((Map.Entry)" + variableName + ')', map, foreachStatement); } else { - replaceParameterAccess(parameter, variableName, map, - foreachStatement); + replaceParameterAccess(parameter, variableName, map, foreachStatement); } - final PsiElementFactory factory = - JavaPsiFacade.getInstance(project).getElementFactory(); - final PsiParameter newParameter = factory.createParameter( - variableName, parameterType); + final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory(); + final PsiParameter newParameter = factory.createParameter( variableName, parameterType); if (parameter.hasModifierProperty(PsiModifier.FINAL)) { - final PsiModifierList modifierList = - newParameter.getModifierList(); + final PsiModifierList modifierList = newParameter.getModifierList(); if (modifierList != null) { modifierList.setModifierProperty(PsiModifier.FINAL, true); } @@ -186,11 +159,9 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { @NonNls String variableName, PsiElement map, PsiElement context) { - final ParameterAccessCollector collector = - new ParameterAccessCollector(parameter, map); + final ParameterAccessCollector collector = new ParameterAccessCollector(parameter, map); context.accept(collector); - final List accesses = - collector.getParameterAccesses(); + final List accesses = collector.getParameterAccesses(); for (PsiExpression access : accesses) { if (access instanceof PsiMethodCallExpression) { PsiReplacementUtil.replaceExpression(access, variableName + ".getValue()"); @@ -201,15 +172,11 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { } } - private static String createNewVariableName( - @NotNull PsiElement scope, @NotNull PsiType type) { + private static String createNewVariableName(@NotNull PsiElement scope, @NotNull PsiType type) { final Project project = scope.getProject(); - final JavaCodeStyleManager codeStyleManager = - JavaCodeStyleManager.getInstance(project); + final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); @NonNls String baseName; - final SuggestedNameInfo suggestions = - codeStyleManager.suggestVariableName( - VariableKind.LOCAL_VARIABLE, null, null, type); + final SuggestedNameInfo suggestions = codeStyleManager.suggestVariableName(VariableKind.LOCAL_VARIABLE, null, null, type); final String[] names = suggestions.names; if (names != null && names.length > 0) { baseName = names[0]; @@ -220,12 +187,10 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { if (baseName == null || baseName.length() == 0) { baseName = "entry"; } - return codeStyleManager.suggestUniqueVariableName(baseName, scope, - true); + return codeStyleManager.suggestUniqueVariableName(baseName, scope, true); } - private static class ParameterAccessCollector - extends JavaRecursiveElementVisitor { + private static class ParameterAccessCollector extends JavaRecursiveElementVisitor { private final PsiParameter parameter; private final PsiElement map; @@ -233,16 +198,14 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { private final List parameterAccesses = new ArrayList(); - public ParameterAccessCollector( - PsiParameter parameter, PsiElement map) { + public ParameterAccessCollector(PsiParameter parameter, PsiElement map) { this.parameter = parameter; parameterName = parameter.getName(); this.map = map; } @Override - public void visitReferenceExpression( - PsiReferenceExpression expression) { + public void visitReferenceExpression(PsiReferenceExpression expression) { super.visitReferenceExpression(expression); if (expression.getQualifierExpression() != null) { return; @@ -322,17 +285,14 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { } final PsiExpression iteratedExpression; if (iteratedValue instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)iteratedValue; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)iteratedValue; final PsiElement target = referenceExpression.resolve(); if (!(target instanceof PsiLocalVariable)) { return; } final PsiVariable variable = (PsiVariable)target; - final PsiMethod containingMethod = - PsiTreeUtil.getParentOfType(variable, PsiMethod.class); - if (VariableAccessUtils.variableIsAssignedAtPoint(variable, - containingMethod, statement)) { + final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(variable, PsiMethod.class); + if (VariableAccessUtils.variableIsAssignedAtPoint(variable, containingMethod, statement)) { return; } iteratedExpression = variable.getInitializer(); @@ -341,8 +301,7 @@ public class KeySetIterationMayUseEntrySetInspection extends BaseInspection { iteratedExpression = iteratedValue; } final PsiParameter parameter = statement.getIterationParameter(); - if (!isMapKeySetIteration(iteratedExpression, parameter, - statement.getBody())) { + if (!isMapKeySetIteration(iteratedExpression, parameter, statement.getBody())) { return; } registerError(iteratedValue); diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.after.java new file mode 100644 index 000000000000..bf146ff59315 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.after.java @@ -0,0 +1,11 @@ +import java.util.Iterator; +import java.util.Map; + +abstract class B { + { + Map sortMap = null; + for (Object o1 : sortMap.entrySet()) { + Object o = ((Map.Entry) o1).getValue(); + } + } +} diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.java new file mode 100644 index 000000000000..aabab8252af9 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/performance/key_set_with_entry_set/Simple.java @@ -0,0 +1,11 @@ +import java.util.Iterator; +import java.util.Map; + +abstract class B { + { + Map sortMap = null; + for (Object columnIdentifier : sortMap.keySet()) { + Object o = sortMap.get(columnIdentifier); + } + } +} diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/KeySetIterationMayUseEntrySetFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/KeySetIterationMayUseEntrySetFixTest.java new file mode 100644 index 000000000000..cc77256d21db --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/performance/KeySetIterationMayUseEntrySetFixTest.java @@ -0,0 +1,32 @@ +/* + * Copyright 2000-2014 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.fixes.performance; + +import com.siyeh.ig.IGQuickFixesTestCase; +import com.siyeh.ig.performance.KeySetIterationMayUseEntrySetInspection; + +public class KeySetIterationMayUseEntrySetFixTest extends IGQuickFixesTestCase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new KeySetIterationMayUseEntrySetInspection()); + myRelativePath = "performance/key_set_with_entry_set"; + } + + public void testSimple() { doTest("Replace with 'entrySet()' iteration"); } + +} \ No newline at end of file