Bug 550672 - Enhanced for each loop conversion creates wrong code
- fix ConvertForLoopOperation.validateBody() to invalidate the
body if there is a get() reference that is for a variable that
isn't the same one that uses size() in the for statement
- add tests to CleanUpTest and ConvertForLoopQuickFixTest
Change-Id: I8e93463570de5b896f99b5770f36908337a2616d
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/ConvertForLoopOperation.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/ConvertForLoopOperation.java
index 78f4ff4..4b4b076 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/ConvertForLoopOperation.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/ConvertForLoopOperation.java
@@ -24,6 +24,7 @@
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.dom.AST;
+import org.eclipse.jdt.core.dom.ASTMatcher;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.ArrayAccess;
import org.eclipse.jdt.core.dom.Assignment;
@@ -540,7 +541,9 @@
ITypeBinding[] parms= methodBinding.getParameterTypes();
if (!fIsCollection || !GET_QUERY.equals(method.getName().getFullyQualifiedName()) ||
parms.length != 1 || !parms[0].getName().equals("int") || //$NON-NLS-1$
- !fSizeMethodBinding.getDeclaringClass().equals(methodBinding.getDeclaringClass()))
+ !fSizeMethodBinding.getDeclaringClass().equals(methodBinding.getDeclaringClass()) ||
+ fSizeMethodAccess.getExpression() == null ||
+ !fSizeMethodAccess.getExpression().subtreeMatch(new ASTMatcher(), method.getExpression()))
throw new InvalidBodyError();
fGetMethodBinding= methodBinding;
} else {
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
index c3002e1..3fcf087 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
@@ -4230,6 +4230,43 @@
assertRefactoringResultAsExpected(new ICompilationUnit[] {cu1}, new String[] {expected1});
}
+ public void testJava50ForLoopBug550672() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuilder buf= new StringBuilder();
+ buf.append("package test1;\n");
+ buf.append("import java.util.List;\n");
+ buf.append("import java.util.ArrayList;\n");
+ buf.append("public class ForeachTest {\n");
+ buf.append(" void foo(List list) {\n");
+ buf.append(" List<File> a = new ArrayList<>();\n");
+ buf.append(" List<File> b = new ArrayList<>();\n");
+ buf.append(" for (int i = 0; i < a.size(); i++) {\n");
+ buf.append(" System.out.print(a.get(i) + \" \" + b.get(i));\n");
+ buf.append(" }\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu1= pack1.createCompilationUnit("E1.java", buf.toString(), false, null);
+
+ enable(CleanUpConstants.CONTROL_STATMENTS_CONVERT_FOR_LOOP_TO_ENHANCED);
+
+ buf= new StringBuilder();
+ buf.append("package test1;\n");
+ buf.append("import java.util.List;\n");
+ buf.append("import java.util.ArrayList;\n");
+ buf.append("public class ForeachTest {\n");
+ buf.append(" void foo(List list) {\n");
+ buf.append(" List<File> a = new ArrayList<>();\n");
+ buf.append(" List<File> b = new ArrayList<>();\n");
+ buf.append(" for (int i = 0; i < a.size(); i++) {\n");
+ buf.append(" System.out.print(a.get(i) + \" \" + b.get(i));\n");
+ buf.append(" }\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ String expected1= buf.toString();
+
+ assertRefactoringResultAsExpected(new ICompilationUnit[] {cu1}, new String[] {expected1});
+ }
+
public void testJava50ForLoopBug154939() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf= new StringBuilder();
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertForLoopQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertForLoopQuickFixTest.java
index d43c199..a37a20d 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertForLoopQuickFixTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertForLoopQuickFixTest.java
@@ -1080,6 +1080,30 @@
assertCorrectLabels(proposals);
}
+ public void testMultipleCollectionsNotAccepted() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuilder buf= new StringBuilder();
+ buf.append("package test1;\n");
+ buf.append("import java.util.List;\n");
+ buf.append("import java.util.ArrayList;\n");
+ buf.append("public class A {\n");
+ buf.append(" public void foo(Map<String, String> map) {\n");
+ buf.append(" List<File> a = new ArrayList<>();\n");
+ buf.append(" List<File> b = new ArrayList<>();\n");
+ buf.append(" for (int i = 0; i < a.size(); i++) {\n");
+ buf.append(" System.out.print(a.get(i) + \" \" + b.get(i));\n");
+ buf.append(" }\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("A.java", buf.toString(), false, null);
+
+ List<IJavaCompletionProposal> proposals= fetchConvertingProposal(buf, cu);
+
+ assertNull(fConvertLoopProposal);
+
+ assertCorrectLabels(proposals);
+ }
+
public void testArrayAccessWithCollections() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf= new StringBuilder();
