Bug 550334 - clean up: convert for loops enhanced results in NPE
- fix ConvertForLoopOperation.validateBody() to invalidate body
if we are dealing with a Collections for loop and an array index
is found for the for index variable
- add new tests for this in CleanUpTest and ConvertForLoopQuickFixTest
Change-Id: I23796d0e88e2b2bf579348dc762b2d2567947524
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 2b1bfcc..78f4ff4 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
@@ -480,6 +480,8 @@
if (nameBinding.equals(fIndexBinding)) {
if (node.getLocationInParent() == ArrayAccess.INDEX_PROPERTY) {
+ if (fIsCollection)
+ throw new InvalidBodyError();
ArrayAccess arrayAccess= (ArrayAccess)node.getParent();
Expression array= arrayAccess.getArray();
if (array instanceof QualifiedName) {
@@ -596,7 +598,7 @@
return super.visit(node);
IBinding binding= getBinding(node.getArray());
- if (fArrayBinding.equals(binding)) {
+ if (!fIsCollection && fArrayBinding.equals(binding)) {
IBinding index= getBinding(node.getIndex());
if (fIndexBinding.equals(index)) {
if (node.getLocationInParent() == VariableDeclarationFragment.INITIALIZER_PROPERTY) {
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 23794897..c3002e1 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
@@ -4197,6 +4197,39 @@
assertRefactoringResultAsExpected(new ICompilationUnit[] {cu1}, new String[] {expected1});
}
+ public void testJava50ForLoopBug550334() 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("public class ForeachTest {\n");
+ buf.append(" void foo(List list) {\n");
+ buf.append(" String[] a= new String[]{\"a\", \"b\", \"c\"});\n");
+ buf.append(" for (int i= 0; i < list.size(); ++i) {\n");
+ buf.append(" list.get(i).append(a[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("public class ForeachTest {\n");
+ buf.append(" void foo(List list) {\n");
+ buf.append(" String[] a= new String[]{\"a\", \"b\", \"c\"});\n");
+ buf.append(" for (int i= 0; i < list.size(); ++i) {\n");
+ buf.append(" list.get(i).append(a[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 1ab234e..d43c199 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 testArrayAccessWithCollections() 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() {\n");
+ buf.append(" int[] array = {1,2,3,4};\n");
+ buf.append(" List<String> list = ArrayList.asList(\"a\", \"b\", \"c\", \"d\");\n");
+ buf.append(" for (int i = 0, i < list.size(); i++){\n");
+ buf.append(" System.out.println(array[i] + list.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 testIndexDoesNotStartFromZero() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf= new StringBuilder();
