From 2f0df9072566de319a8d8d511497796a8e1cc3c1 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 14 Dec 2012 12:16:49 +0100 Subject: [PATCH] IDEA-97460 (Java intention 'Replace for-each with iterator 'for' loop' generates red code) --- ...rEachLoopWithIteratorForLoopIntention.java | 60 +++++++------------ .../siyeh/ipp/forloop/iterator/Wildcards.java | 12 ++++ .../ipp/forloop/iterator/Wildcards_after.java | 14 +++++ ...hLoopWithIteratorForLoopIntentionTest.java | 12 ++-- 4 files changed, 52 insertions(+), 46 deletions(-) create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards_after.java diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntention.java index c52bafd03bd6..eab3a56a97db 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntention.java @@ -41,59 +41,43 @@ public class ReplaceForEachLoopWithIteratorForLoopIntention extends Intention { if (statement == null) { return; } - final Project project = statement.getProject(); - final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); final PsiExpression iteratedValue = statement.getIteratedValue(); if (iteratedValue == null) { return; } - @NonNls final StringBuilder newStatement = new StringBuilder(); final PsiType iteratedValueType = iteratedValue.getType(); if (!(iteratedValueType instanceof PsiClassType)) { return; } - final PsiClassType classType = (PsiClassType)iteratedValueType; - final PsiParameter iterationParameter = statement.getIterationParameter(); - final PsiType parameterType = iterationParameter.getType(); - final String iterator = codeStyleManager.suggestUniqueVariableName("iterator", statement, true); - final String typeText = parameterType.getCanonicalText(); - newStatement.append("for(java.util.Iterator"); - if (classType.hasParameters()) { - newStatement.append('<'); - final PsiType[] parameters = classType.getParameters(); - if (parameters.length == 1) { - newStatement.append(parameters[0].getCanonicalText()); - } else { - newStatement.append(typeText); - } - newStatement.append("> "); - } else { - newStatement.append(' '); - } - newStatement.append(iterator); - newStatement.append(" = "); + @NonNls final StringBuilder methodCall = new StringBuilder(); if (ParenthesesUtils.getPrecedence(iteratedValue) > ParenthesesUtils.METHOD_CALL_PRECEDENCE) { - newStatement.append('('); - newStatement.append(iteratedValue.getText()); - newStatement.append(')'); + methodCall.append('(').append(iteratedValue.getText()).append(')'); } else { - newStatement.append(iteratedValue.getText()); + methodCall.append(iteratedValue.getText()); } - newStatement.append(".iterator();"); - newStatement.append(iterator); - newStatement.append(".hasNext();) {"); - final CodeStyleSettings codeStyleSettings = - CodeStyleSettingsManager.getSettings(project); + methodCall.append(".iterator()"); + final Project project = statement.getProject(); + final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory(); + final PsiExpression iteratorCall = factory.createExpressionFromText(methodCall.toString(), iteratedValue); + final PsiType variableType = GenericsUtil.getVariableTypeByExpressionType(iteratorCall.getType()); + if (variableType == null) { + return; + } + @NonNls final StringBuilder newStatement = new StringBuilder(); + newStatement.append("for(").append(variableType.getCanonicalText()).append(' '); + final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); + final String iterator = codeStyleManager.suggestUniqueVariableName("iterator", statement, true); + newStatement.append(iterator).append("=").append(iteratorCall.getText()).append(';'); + newStatement.append(iterator).append(".hasNext();) {"); + final CodeStyleSettings codeStyleSettings = CodeStyleSettingsManager.getSettings(project); if (codeStyleSettings.GENERATE_FINAL_LOCALS) { newStatement.append("final "); } - newStatement.append(typeText); - newStatement.append(' '); - newStatement.append(iterationParameter.getName()); - newStatement.append(" = "); - newStatement.append(iterator); - newStatement.append(".next();"); + final PsiParameter iterationParameter = statement.getIterationParameter(); + final PsiType parameterType = iterationParameter.getType(); + final String typeText = parameterType.getCanonicalText(); + newStatement.append(typeText).append(' ').append(iterationParameter.getName()).append(" = ").append(iterator).append(".next();"); final PsiStatement body = statement.getBody(); if (body == null) { return; diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards.java new file mode 100644 index 000000000000..254e911e0739 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards.java @@ -0,0 +1,12 @@ +package com.siyeh.ipp.forloop.iterator; + +import java.util.List; +import java.util.Map; + +class Wildcards { + + void renames(Map allRenames) { + for (Map.Entry entry : allRenames.entrySet()) { + } + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards_after.java new file mode 100644 index 000000000000..2da7279c15d8 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/forloop/iterator/Wildcards_after.java @@ -0,0 +1,14 @@ +package com.siyeh.ipp.forloop.iterator; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +class Wildcards { + + void renames(Map allRenames) { + for (Iterator> iterator = allRenames.entrySet().iterator(); iterator.hasNext(); ) { + Map.Entry entry = iterator.next(); + } + } +} \ No newline at end of file diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntentionTest.java index 3bd75dc9675d..75d45ed7083b 100644 --- a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntentionTest.java +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/forloop/ReplaceForEachLoopWithIteratorForLoopIntentionTest.java @@ -18,13 +18,15 @@ package com.siyeh.ipp.forloop; import com.siyeh.IntentionPowerPackBundle; import com.siyeh.ipp.IPPTestCase; -import java.util.Collection; - +/** + * @see com.siyeh.ipp.forloop.ReplaceForEachLoopWithIteratorForLoopIntention + */ public class ReplaceForEachLoopWithIteratorForLoopIntentionTest extends IPPTestCase { public void testBareCollectionLoop() { doTest(); } public void testGenericTypes() { doTest(); } public void testBoundedTypes() { doTest(); } public void testPrecedence() { doTest(); } + public void testWildcards() { doTest(); } @Override protected String getIntentionName() { @@ -35,10 +37,4 @@ public class ReplaceForEachLoopWithIteratorForLoopIntentionTest extends IPPTestC protected String getRelativePath() { return "forloop/iterator"; } - - void x(Collection c) { - for (Object n : c) { - - } - } }