Bug 411588: "Convert for loops to enhanced" may mess up program logic when dealing with IterableI20130702-1230

2 files changed, 74 insertions, 8 deletions

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertIterableLoopQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertIterableLoopQuickFixTest.java
index 9ce7ef8d84..a067584253 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertIterableLoopQuickFixTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/ConvertIterableLoopQuickFixTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2011 IBM Corporation and others.
+ * Copyright (c) 2005, 2013 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -915,6 +915,72 @@ public final class ConvertIterableLoopQuickFixTest extends QuickFixTest {
 		assertEqualString(preview, expected);
 	}
 
+	public void testWrongIteratorMethod_bug411588() throws Exception {
+		IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+		StringBuffer buf= new StringBuffer();
+		buf.append("package p;\n");
+		buf.append("import java.util.Iterator;\n");
+		buf.append("\n");
+		buf.append("public class TestSaveActionConvertToEnhancedForLoop {\n");
+		buf.append("    static class Something implements Iterable<Object>{\n");
+		buf.append("        @Override\n");
+		buf.append("        public Iterator<Object> iterator() {\n");
+		buf.append("            return null;\n");
+		buf.append("        }\n");
+		buf.append("        \n");
+		buf.append("        public Iterator<Object> iterator(int filter) {\n");
+		buf.append("            return null;\n");
+		buf.append("        }\n");
+		buf.append("    }\n");
+		buf.append("    \n");
+		buf.append("    public static void main(String[] args) {          \n");
+		buf.append("        Something s = new Something();\n");
+		buf.append("        for (Iterator<Object> it = s.iterator(42) ; it.hasNext(); ) {\n");
+		buf.append("             Object obj = it.next();\n");
+		buf.append("        }\n");
+		buf.append("    }\n");
+		buf.append("}\n");
+
+		ICompilationUnit unit= pack.createCompilationUnit("TestSaveActionConvertToEnhancedForLoop.java", buf.toString(), false, null);
+
+		List proposals= fetchConvertingProposal(buf, unit);
+
+		assertTrue(fConvertLoopProposal.getFixStatus() != null && fConvertLoopProposal.getFixStatus().getCode() == IStatus.WARNING);
+
+		assertCorrectLabels(proposals);
+
+		assertNotNull(fConvertLoopProposal.getStatusMessage());
+
+		String preview= getPreviewContent(fConvertLoopProposal);
+
+		buf= new StringBuffer();
+		buf.append("package p;\n");
+		buf.append("import java.util.Iterator;\n");
+		buf.append("\n");
+		buf.append("public class TestSaveActionConvertToEnhancedForLoop {\n");
+		buf.append("    static class Something implements Iterable<Object>{\n");
+		buf.append("        @Override\n");
+		buf.append("        public Iterator<Object> iterator() {\n");
+		buf.append("            return null;\n");
+		buf.append("        }\n");
+		buf.append("        \n");
+		buf.append("        public Iterator<Object> iterator(int filter) {\n");
+		buf.append("            return null;\n");
+		buf.append("        }\n");
+		buf.append("    }\n");
+		buf.append("    \n");
+		buf.append("    public static void main(String[] args) {          \n");
+		buf.append("        Something s = new Something();\n");
+		buf.append("        for (Object obj : s) {\n");
+		buf.append("        }\n");
+		buf.append("    }\n");
+		buf.append("}\n");
+
+		String expected= buf.toString();
+		assertEqualString(preview, expected);
+
+	}
+
 	public void testCorrectIteratorMethod() throws Exception {
 		IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
 		StringBuffer buf= new StringBuffer();
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/ConvertIterableLoopOperation.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/ConvertIterableLoopOperation.java
index 96a64aa4d5..e0e5d0032c 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/ConvertIterableLoopOperation.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/ConvertIterableLoopOperation.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2012 IBM Corporation and others.
+ * Copyright (c) 2005, 2013 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -451,14 +451,14 @@ public final class ConvertIterableLoopOperation extends ConvertLoopOperation {
 								String name= invocation.getName().getIdentifier();
 								if (name.equals("next") || name.equals("nextElement")) { //$NON-NLS-1$ //$NON-NLS-2$
 									nextInvocationCount[0]++;
-									
+
 									Expression left= null;
 									if (invocation.getLocationInParent() == Assignment.RIGHT_HAND_SIDE_PROPERTY) {
 										left= ((Assignment) invocation.getParent()).getLeftHandSide();
 									} else if (invocation.getLocationInParent() == VariableDeclarationFragment.INITIALIZER_PROPERTY) {
 										left= ((VariableDeclarationFragment) invocation.getParent()).getName();
-									} 
-									
+									}
+
 									return visitElementVariable(left);
 								}
 							}
@@ -466,7 +466,7 @@ public final class ConvertIterableLoopOperation extends ConvertLoopOperation {
 						}
 						return true;
 					}
-					
+
 					private boolean visitElementVariable(final Expression node) {
 						if (node != null) {
 							final ITypeBinding binding= node.resolveTypeBinding();
@@ -594,9 +594,9 @@ public final class ConvertIterableLoopOperation extends ConvertLoopOperation {
 
 		MethodInvocation methodInvocation= (MethodInvocation)initializer;
 		String methodName= methodInvocation.getName().getIdentifier();
-		if (!"iterator".equals(methodName)) //$NON-NLS-1$
+		if (!"iterator".equals(methodName) || methodInvocation.arguments().size() != 0) //$NON-NLS-1$
 			return SEMANTIC_CHANGE_WARNING_STATUS;
-		
+
 		return StatusInfo.OK_STATUS;
 	}