Attachment 237236 Details for Bug 241696 – Patch containing suggested improvements

[patch] Patch containing suggested improvements

Fixed-Bug-241696--quick-fix-quickfix-to-iterate-over.patch (text/plain), 32.19 KB, created by Lukas Hanke

on 2013-11-06 10:40:37 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Lukas Hanke

Created: 2013-11-06 10:40:37 EST

Size: 32.19 KB

patch

obsolete

>diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java
>index 40a4d85..b37cf2a 100644
>--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java
>+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java
>@@ -8514,8 +8514,7 @@
> 			buf.append("import java.util.Iterator;\n");
> 			buf.append("public class E {\n");
> 			buf.append("    void foo(Collection<String> collection) {\n");
>-			buf.append("        for (Iterator<String> iterator = collection.iterator(); iterator\n");
>-			buf.append("                .hasNext();) {\n");
>+			buf.append("        for (Iterator<String> iterator = collection.iterator(); iterator.hasNext();) {\n");
> 			buf.append("            String string = iterator.next();\n");
> 			buf.append("            \n");
> 			buf.append("        }\n");
>@@ -8651,6 +8650,67 @@
> 			fJProject1.setOptions(saveOptions);
> 		}
> 	}
>+	
>+	public void testGenerateForGenerics() throws Exception {
>+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test1;\n");
>+		buf.append("import java.util.Collection;\n");
>+		buf.append("import java.util.Date;\n");
>+		buf.append("public class E {\n");
>+		buf.append("    void <T extends Date> foo(Collection<T> collection) {\n");
>+		buf.append("        collection\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
>+
>+		Map saveOptions= fJProject1.getOptions(false);
>+		Map newOptions= new HashMap(saveOptions);
>+		newOptions.put(DefaultCodeFormatterConstants.FORMATTER_PUT_EMPTY_STATEMENT_ON_NEW_LINE, "true");
>+		try {
>+			fJProject1.setOptions(newOptions);
>+			String selection= "collection";
>+			AssistContext context= getCorrectionContext(cu, buf.toString().lastIndexOf(selection) + selection.length(), 0);
>+			List proposals= collectAssists(context, false);
>+
>+			assertNumberOfProposals(proposals, 2);
>+			assertCorrectLabels(proposals);
>+
>+			String[] expected= new String[2];
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("import java.util.Collection;\n");
>+			buf.append("import java.util.Date;\n");
>+			buf.append("public class E {\n");
>+			buf.append("    void <T extends Date> foo(Collection<T> collection) {\n");
>+			buf.append("        for (T t : collection) {\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[0]= buf.toString();
>+
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("import java.util.Collection;\n");
>+			buf.append("import java.util.Date;\n");
>+			buf.append("import java.util.Iterator;\n");
>+			buf.append("public class E {\n");
>+			buf.append("    void <T extends Date> foo(Collection<T> collection) {\n");
>+			buf.append("        for (Iterator<T> iterator = collection.iterator(); iterator\n");
>+			buf.append("                .hasNext();) {\n");
>+			buf.append("            T t = iterator.next();\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[1]= buf.toString();
>+
>+			assertExpectedExistInProposals(proposals, expected);
>+		} finally {
>+			fJProject1.setOptions(saveOptions);
>+		}
>+	}
> 
> 	public void testGenerateForMissingParametrization() throws Exception {
> 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
>@@ -8695,7 +8755,7 @@
> 			buf.append("import java.util.Iterator;\n");
> 			buf.append("public class E {\n");
> 			buf.append("    void foo(Collection collection) {\n");
>-			buf.append("        for (Iterator<Object> iterator = collection.iterator(); iterator\n");
>+			buf.append("        for (Iterator iterator = collection.iterator(); iterator\n");
> 			buf.append("                .hasNext();) {\n");
> 			buf.append("            Object object = iterator.next();\n");
> 			buf.append("            \n");
>@@ -8746,7 +8806,8 @@
> 			buf.append("import java.util.Iterator;\n");
> 			buf.append("public class E {\n");
> 			buf.append("    void foo(Collection collection) {\n");
>-			buf.append("        for (Iterator iterator = collection.iterator(); iterator.hasNext();) {\n");
>+			buf.append("        for (Iterator iterator = collection.iterator(); iterator\n");
>+			buf.append("                .hasNext();) {\n");
> 			buf.append("            Object object = iterator.next();\n");
> 			buf.append("            \n");
> 			buf.append("        }\n");
>@@ -8770,7 +8831,7 @@
> 		buf.append("    }\n");
> 		buf.append("}\n");
> 		ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
>-
>+		
> 		String selection= "array";
> 		AssistContext context= getCorrectionContext(cu, buf.toString().lastIndexOf(selection) + selection.length(), 0);
> 		List proposals= collectAssists(context, false);
>@@ -8778,7 +8839,7 @@
> 		assertNumberOfProposals(proposals, 2);
> 		assertCorrectLabels(proposals);
> 
>-		String[] expected= new String[1];
>+		String[] expected= new String[2];
> 		buf= new StringBuffer();
> 		buf.append("package test1;\n");
> 		buf.append("public class E {\n");
>@@ -8799,7 +8860,224 @@
> 		buf.append("        }\n");
> 		buf.append("    }\n");
> 		buf.append("}\n");
>+		expected[1]= buf.toString();
> 
> 		assertExpectedExistInProposals(proposals, expected);
> 	}
>+	
>+	public void testGenerateForImportsAndFormat1() throws Exception {
>+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test1;\n");
>+		buf.append("public class A {\n");
>+		buf.append("    class Iterator {}\n");	
>+		buf.append("    void foo() {\n");
>+		buf.append("        B.get( /*important: empty*/ );\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+
>+		StringBuffer buf2= new StringBuffer();
>+		buf2.append("package test1;\n");
>+		buf2.append("import java.util.ArrayList;\n");
>+		buf2.append("import java.util.Date;\n");
>+		buf2.append("import java.util.Set;\n");
>+		buf2.append("public class B {\n");
>+		buf2.append("    static ArrayList<Date> get() {\n");
>+		buf2.append("        return new ArrayList<Date>();\n");
>+		buf2.append("    }\n");
>+		buf2.append("    static Set raw(int i) {\n");
>+		buf2.append("        return java.util.Collections.emptySet();\n");
>+		buf2.append("    }\n");
>+		buf2.append("}");
>+		
>+		ICompilationUnit cu= pack1.createCompilationUnit("A.java", buf.toString(), false, null);
>+		pack1.createCompilationUnit("B.java", buf2.toString(), false, null);
>+
>+		Map saveOptions= fJProject1.getOptions(false);
>+		Map newOptions= new HashMap();
>+		JavaCore.setComplianceOptions(JavaCore.VERSION_1_5, newOptions);
>+		newOptions.put(DefaultCodeFormatterConstants.FORMATTER_PUT_EMPTY_STATEMENT_ON_NEW_LINE, "true");
>+		try {
>+			fJProject1.setOptions(newOptions);
>+		
>+			String selection= "B.get( /*important: empty*/ );";
>+			AssistContext context= getCorrectionContext(cu, buf.toString().lastIndexOf(selection) + selection.length(), 0);
>+			List proposals= collectAssists(context, false);
>+	
>+			assertNumberOfProposals(proposals, 4);
>+			assertCorrectLabels(proposals);
>+	
>+			String[] expected= new String[2];
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("\n");
>+			buf.append("import java.util.Date;\n");
>+			buf.append("\n");
>+			buf.append("public class A {\n");
>+			buf.append("    class Iterator {}\n");	
>+			buf.append("    void foo() {\n");
>+			buf.append("        for (Date date : B.get( /*important: empty*/ )) {\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[0]= buf.toString();
>+	
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("\n");
>+			buf.append("import java.util.Date;\n");
>+			buf.append("\n");
>+			buf.append("public class A {\n");
>+			buf.append("    class Iterator {}\n");	
>+			buf.append("    void foo() {\n");
>+			buf.append("        for (java.util.Iterator<Date> iterator = B.get( /*important: empty*/ ).iterator(); iterator\n");
>+			buf.append("                .hasNext();) {\n");
>+			buf.append("            Date date = iterator.next();\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[1]= buf.toString();
>+	
>+			assertExpectedExistInProposals(proposals, expected);
>+		} finally {
>+			fJProject1.setOptions(saveOptions);
>+		}
>+	}
>+	
>+	public void testGenerateForImportsAndFormat2() throws Exception {
>+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test1;\n");
>+		buf.append("public class A {\n");
>+		buf.append("    class Object {}\n");
>+		buf.append("    class Iterator {}\n");	
>+		buf.append("    void foo() {\n");
>+		buf.append("        B.raw(1+ 2);\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+
>+		StringBuffer buf2= new StringBuffer();
>+		buf2.append("package test1;\n");
>+		buf2.append("import java.util.ArrayList;\n");
>+		buf2.append("import java.util.Date;\n");
>+		buf2.append("import java.util.Set;\n");
>+		buf2.append("public class B {\n");
>+		buf2.append("    static ArrayList<Date> get() {\n");
>+		buf2.append("        return new ArrayList<Date>();\n");
>+		buf2.append("    }\n");
>+		buf2.append("    static Set raw(int i) {\n");
>+		buf2.append("        return java.util.Collections.emptySet();\n");
>+		buf2.append("    }\n");
>+		buf2.append("}");
>+		
>+		ICompilationUnit cu= pack1.createCompilationUnit("A.java", buf.toString(), false, null);
>+		pack1.createCompilationUnit("B.java", buf2.toString(), false, null);
>+
>+		Map saveOptions= fJProject1.getOptions(false);
>+		Map newOptions= new HashMap();
>+		JavaCore.setComplianceOptions(JavaCore.VERSION_1_5, newOptions);
>+		newOptions.put(DefaultCodeFormatterConstants.FORMATTER_PUT_EMPTY_STATEMENT_ON_NEW_LINE, "true");
>+		try {
>+			fJProject1.setOptions(newOptions);
>+		
>+			String selection= "B.raw(1+ 2);";
>+			AssistContext context= getCorrectionContext(cu, buf.toString().lastIndexOf(selection) + selection.length(), 0);
>+			List proposals= collectAssists(context, false);
>+	
>+			assertNumberOfProposals(proposals, 4);
>+			assertCorrectLabels(proposals);
>+	
>+			String[] expected= new String[2];
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("public class A {\n");
>+			buf.append("    class Object {}\n");	
>+			buf.append("    class Iterator {}\n");	
>+			buf.append("    void foo() {\n");
>+			buf.append("        for (java.lang.Object object : B.raw(1+ 2)) {\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[0]= buf.toString();
>+	
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("public class A {\n");
>+			buf.append("    class Object {}\n");
>+			buf.append("    class Iterator {}\n");	
>+			buf.append("    void foo() {\n");
>+			buf.append("        for (java.util.Iterator iterator = B.raw(1+ 2).iterator(); iterator\n");
>+			buf.append("                .hasNext();) {\n");
>+			buf.append("            java.lang.Object object = iterator.next();\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[1]= buf.toString();
>+	
>+			assertExpectedExistInProposals(proposals, expected);
>+		} finally {
>+			fJProject1.setOptions(saveOptions);
>+		}
>+	}
>+	
>+	public void testGenerateForImportsArray() throws Exception {
>+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test1;\n");
>+		buf.append("public class A {\n");
>+		buf.append("    class Date {}\n");	
>+		buf.append("    void foo() {\n");
>+		buf.append("        B.get();\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+
>+		StringBuffer buf2= new StringBuffer();
>+		buf2.append("package test1;\n");
>+		buf2.append("import java.util.Date;\n");
>+		buf2.append("public class B {\n");
>+		buf2.append("    static Date[] get() {\n");
>+		buf2.append("        return new Date[1];\n");
>+		buf2.append("    }\n");
>+		buf2.append("}");
>+		
>+		ICompilationUnit cu= pack1.createCompilationUnit("A.java", buf.toString(), false, null);
>+		pack1.createCompilationUnit("B.java", buf2.toString(), false, null);
>+
>+		Map saveOptions= fJProject1.getOptions(false);
>+		Map newOptions= new HashMap();
>+		JavaCore.setComplianceOptions(JavaCore.VERSION_1_5, newOptions);
>+		newOptions.put(DefaultCodeFormatterConstants.FORMATTER_PUT_EMPTY_STATEMENT_ON_NEW_LINE, "true");
>+		try {
>+			fJProject1.setOptions(newOptions);
>+		
>+			String selection= "B.get();";
>+			AssistContext context= getCorrectionContext(cu, buf.toString().lastIndexOf(selection) + selection.length(), 0);
>+			List proposals= collectAssists(context, false);
>+	
>+			assertNumberOfProposals(proposals, 4);
>+			assertCorrectLabels(proposals);
>+	
>+			String[] expected= new String[1];
>+			buf= new StringBuffer();
>+			buf.append("package test1;\n");
>+			buf.append("public class A {\n");
>+			buf.append("    class Date {}\n");
>+			buf.append("    void foo() {\n");
>+			buf.append("        for (java.util.Date date : B.get()) {\n");
>+			buf.append("            \n");
>+			buf.append("        }\n");
>+			buf.append("    }\n");
>+			buf.append("}\n");
>+			expected[0]= buf.toString();
>+	
>+			assertExpectedExistInProposals(proposals, expected);
>+		} finally {
>+			fJProject1.setOptions(saveOptions);
>+		}
>+	}
>+	
> }
>diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties
>index d1fdaaa..8a33e69 100644
>--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties
>+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties
>@@ -345,8 +345,8 @@
> QuickAssistProcessor_convert_local_to_field_description=Convert local variable to field
> QuickAssistProcessor_convert_to_indexed_for_loop=Convert to indexed 'for' loop
> QuickAssistProcessor_convert_to_iterator_for_loop=Convert to Iterator-based 'for' loop
>-QuickAssistProcessor_generate_enhanced_for_loop=Iterate over collection using foreach
>-QuickAssistProcessor_generate_iterator_for_loop=Iterate over collection using iterator
>+QuickAssistProcessor_generate_enhanced_for_loop=Iterate over array or iterable using foreach
>+QuickAssistProcessor_generate_iterator_for_loop=Iterate over iterable using iterator
> QuickAssistProcessor_generate_iterate_array_for_loop=Iterate over array using for
> QuickAssistProcessor_convert_to_message_format=Use 'MessageFormat' for string concatenation
> QuickAssistProcessor_convert_to_multiple_singletype_catch_blocks=Use separate catch blocks
>diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java
>index 45b106e..e0a6dd6 100644
>--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java
>+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java
>@@ -2556,7 +2556,7 @@
> 		return true;
> 	}
> 
>-	private boolean getGenerateForLoopProposals(IInvocationContext context, ASTNode coveringNode, Collection<ICommandAccess> resultingCollections) {
>+	private static boolean getGenerateForLoopProposals(IInvocationContext context, ASTNode coveringNode, Collection<ICommandAccess> resultingCollections) {
> 		Statement statement= ASTResolving.findParentStatement(coveringNode);
> 		if (!(statement instanceof ExpressionStatement)) {
> 			return false;
>@@ -2565,8 +2565,8 @@
> 		Expression expression= ((ExpressionStatement) statement).getExpression();
> 		ICompilationUnit cu= context.getCompilationUnit();
> 		ITypeBinding expressionType= null;
>-		
>-		if (expression instanceof MethodInvocation 
>+
>+		if (expression instanceof MethodInvocation
> 				|| expression instanceof SimpleName
> 				|| expression instanceof FieldAccess) {
> 			expressionType= expression.resolveTypeBinding();
>@@ -2578,7 +2578,7 @@
> 
> 		if (expressionType == null)
> 			return false;
>-		
>+
> 		if (Bindings.findTypeInHierarchy(expressionType, "java.lang.Iterable") != null) { //$NON-NLS-1$
> 			GenerateForLoopAssistProposal iteratorForProposal= new GenerateForLoopAssistProposal(cu, statement, expression, GenerateForLoopAssistProposal.GENERATE_ITERATOR_FOR);
> 			resultingCollections.add(iteratorForProposal);
>diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/proposals/GenerateForLoopAssistProposal.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/proposals/GenerateForLoopAssistProposal.java
>index 4b57444..79deedf 100644
>--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/proposals/GenerateForLoopAssistProposal.java
>+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/proposals/GenerateForLoopAssistProposal.java
>@@ -27,26 +27,23 @@
> import org.eclipse.jdt.core.dom.Expression;
> import org.eclipse.jdt.core.dom.FieldAccess;
> import org.eclipse.jdt.core.dom.ForStatement;
>+import org.eclipse.jdt.core.dom.IMethodBinding;
> import org.eclipse.jdt.core.dom.ITypeBinding;
> import org.eclipse.jdt.core.dom.InfixExpression;
> import org.eclipse.jdt.core.dom.MethodInvocation;
> import org.eclipse.jdt.core.dom.NumberLiteral;
>-import org.eclipse.jdt.core.dom.ParameterizedType;
> import org.eclipse.jdt.core.dom.PostfixExpression;
> import org.eclipse.jdt.core.dom.PrimitiveType;
> import org.eclipse.jdt.core.dom.SimpleName;
>-import org.eclipse.jdt.core.dom.SimpleType;
> import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
> import org.eclipse.jdt.core.dom.Statement;
> import org.eclipse.jdt.core.dom.VariableDeclarationExpression;
> import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
> import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
>-import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
>-
>+import org.eclipse.jdt.internal.corext.codemanipulation.ContextSensitiveImportRewriteContext;
> import org.eclipse.jdt.internal.corext.codemanipulation.StubUtility;
> import org.eclipse.jdt.internal.corext.dom.Bindings;
> import org.eclipse.jdt.internal.corext.dom.ScopeAnalyzer;
>-import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
> 
> import org.eclipse.jdt.internal.ui.JavaPluginImages;
> import org.eclipse.jdt.internal.ui.text.correction.CorrectionMessages;
>@@ -113,6 +110,7 @@
> 	protected ASTRewrite getRewrite() throws CoreException {
> 
> 		AST ast= fCurrentNode.getAST();
>+		createImportRewrite((CompilationUnit) fCurrentExpression.getRoot());
> 
> 		// generate the subexpression which represents the expression to iterate over
> 		if (fCurrentExpression instanceof Assignment) {
>@@ -144,19 +142,17 @@
> 		EnhancedForStatement loopStatement= ast.newEnhancedForStatement();
> 
> 		ASTRewrite rewrite= ASTRewrite.create(ast);
>-		String loopOverTypename= extractElementTypeName(ast);
>-		if (loopOverTypename == null) {
>-			loopOverTypename= Object.class.getSimpleName();
>-		}
>+		ITypeBinding loopOverType= extractElementType(ast);
>+
> 		// generate name proposals and add them to the variable declaration
>-		SimpleName forDeclarationName= resolveLinkedVariableNameWithProposals(ast, rewrite, loopOverTypename, true);
>+		SimpleName forDeclarationName= resolveLinkedVariableNameWithProposals(rewrite, loopOverType.getName(), true);
> 
> 		SingleVariableDeclaration forLoopInitializer= ast.newSingleVariableDeclaration();
>-		forLoopInitializer.setType(ast.newSimpleType(ast.newSimpleName(loopOverTypename)));
>+		forLoopInitializer.setType(getImportRewrite().addImport(loopOverType, ast, new ContextSensitiveImportRewriteContext(fCurrentNode, getImportRewrite())));
> 		forLoopInitializer.setName(forDeclarationName);
> 
> 		loopStatement.setParameter(forLoopInitializer);
>-		loopStatement.setExpression((Expression) ASTNode.copySubtree(ast, fSubExpression));
>+		loopStatement.setExpression((Expression) rewrite.createCopyTarget(fSubExpression));
> 
> 		Block forLoopBody= ast.newBlock();
> 		forLoopBody.statements().add(createBlankLineStatementWithCursorPosition(rewrite));
>@@ -179,31 +175,25 @@
> 		ASTRewrite rewrite= ASTRewrite.create(ast);
> 		ForStatement loopStatement= ast.newForStatement();
> 
>-		String loopOverTypename= extractElementTypeName(ast);
>-		if (loopOverTypename == null && JavaModelUtil.is50OrHigher(getCompilationUnit().getJavaProject())) {
>-			loopOverTypename= Object.class.getSimpleName();
>-		}
>+		ITypeBinding loopOverType= extractElementType(ast);
> 
>-		SimpleName loopVariableName= resolveLinkedVariableNameWithProposals(ast, rewrite, "iterator", true); //$NON-NLS-1$
>-		loopStatement.initializers().add(getIteratorBasedForInitializer(ast, rewrite, loopOverTypename, loopVariableName));
>+		SimpleName loopVariableName= resolveLinkedVariableNameWithProposals(rewrite, "iterator", true); //$NON-NLS-1$
>+		loopStatement.initializers().add(getIteratorBasedForInitializer(rewrite, loopVariableName));
> 
> 		MethodInvocation loopExpression= ast.newMethodInvocation();
> 		loopExpression.setName(ast.newSimpleName("hasNext")); //$NON-NLS-1$
> 		SimpleName expressionName= ast.newSimpleName(loopVariableName.getIdentifier());
> 		addLinkedPosition(rewrite.track(expressionName), false, expressionName.getIdentifier());
> 		loopExpression.setExpression(expressionName);
>-
>+		
> 		loopStatement.setExpression(loopExpression);
> 
> 		Block forLoopBody= ast.newBlock();
>-		Assignment assignResolvedVariable= getIteratorBasedForBodyAssignment(ast, rewrite, loopOverTypename, loopVariableName);
>+		Assignment assignResolvedVariable= getIteratorBasedForBodyAssignment(rewrite, loopOverType, loopVariableName);
> 		forLoopBody.statements().add(ast.newExpressionStatement(assignResolvedVariable));
> 		forLoopBody.statements().add(createBlankLineStatementWithCursorPosition(rewrite));
> 
> 		loopStatement.setBody(forLoopBody);
>-
>-		ImportRewrite fixImports= createImportRewrite((CompilationUnit) fCurrentExpression.getRoot());
>-		fixImports.addImport("java.util.Iterator"); //$NON-NLS-1$
> 
> 		rewrite.replace(fCurrentNode, loopStatement, null);
> 
>@@ -214,31 +204,24 @@
> 	 * Generates the initializer for an iterator based <code>for</code> loop, which declares and
> 	 * initializes the variable to loop over.
> 	 * 
>-	 * @param ast the current {@link AST}
> 	 * @param rewrite the instance of {@link ASTRewrite}
>-	 * @param loopOverTypename the type of the loop variable represented as {@link String}
> 	 * @param loopVariableName the proposed name of the loop variable
> 	 * @return a {@link VariableDeclarationExpression} to use as initializer
> 	 */
>-	private VariableDeclarationExpression getIteratorBasedForInitializer(AST ast, ASTRewrite rewrite, String loopOverTypename, SimpleName loopVariableName) {
>+	private VariableDeclarationExpression getIteratorBasedForInitializer(ASTRewrite rewrite, SimpleName loopVariableName) {
>+		AST ast= rewrite.getAST();
>+		IMethodBinding iteratorMethodBinding= Bindings.findMethodInHierarchy(fCurrentExpression.resolveTypeBinding(), "iterator", new ITypeBinding[] {}); //$NON-NLS-1$
> 		// initializing fragment
> 		VariableDeclarationFragment varDeclarationFragment= ast.newVariableDeclarationFragment();
> 		varDeclarationFragment.setName(loopVariableName);
> 		MethodInvocation iteratorExpression= ast.newMethodInvocation();
>-		iteratorExpression.setName(ast.newSimpleName("iterator")); //$NON-NLS-1$
>-		iteratorExpression.setExpression((Expression) ASTNode.copySubtree(ast, fSubExpression));
>+		iteratorExpression.setName(ast.newSimpleName(iteratorMethodBinding.getName()));
>+		iteratorExpression.setExpression((Expression) rewrite.createCopyTarget(fCurrentExpression));
> 		varDeclarationFragment.setInitializer(iteratorExpression);
> 
> 		// declaration
> 		VariableDeclarationExpression varDeclarationExpression= ast.newVariableDeclarationExpression(varDeclarationFragment);
>-		SimpleType type= ast.newSimpleType(ast.newSimpleName("Iterator")); //$NON-NLS-1$
>-		if (loopOverTypename != null) {
>-			ParameterizedType parameterizedType= ast.newParameterizedType(type);
>-			parameterizedType.typeArguments().add(ast.newSimpleType(ast.newSimpleName(loopOverTypename)));
>-			varDeclarationExpression.setType(parameterizedType);
>-		} else {
>-			varDeclarationExpression.setType(type);
>-		}
>+		varDeclarationExpression.setType(getImportRewrite().addImport(iteratorMethodBinding.getReturnType(), ast, new ContextSensitiveImportRewriteContext(fCurrentNode, getImportRewrite())));
> 
> 		return varDeclarationExpression;
> 	}
>@@ -248,24 +231,22 @@
> 	 * based <code>for</code> loop body, to retrieve the next element of the {@link Iterable}
> 	 * instance.
> 	 * 
>-	 * @param ast the current {@link AST}
> 	 * @param rewrite the current instance of {@link ASTRewrite}
>-	 * @param loopOverTypename the type of the loop variable in string representation
>+	 * @param loopOverType the {@link ITypeBinding} of the loop variable
> 	 * @param loopVariableName the name of the loop variable
> 	 * @return an {@link Assignment}, which retrieves the next element of the {@link Iterable} using
> 	 *         the active {@link Iterator}
> 	 */
>-	private Assignment getIteratorBasedForBodyAssignment(AST ast, ASTRewrite rewrite, String loopOverTypename, SimpleName loopVariableName) {
>+	private Assignment getIteratorBasedForBodyAssignment(ASTRewrite rewrite, ITypeBinding loopOverType, SimpleName loopVariableName) {
>+		AST ast= rewrite.getAST();
> 		Assignment assignResolvedVariable= ast.newAssignment();
> 
>-		// in case no generics were given we get instances of Object.class using iterator.next()
>-		String elementTypename= (loopOverTypename == null ? Object.class.getSimpleName() : loopOverTypename);
> 		// left hand side
>-		SimpleName resolvedVariableName= resolveLinkedVariableNameWithProposals(ast, rewrite, elementTypename, false);
>+		SimpleName resolvedVariableName= resolveLinkedVariableNameWithProposals(rewrite, loopOverType.getName(), false);
> 		VariableDeclarationFragment resolvedVariableDeclarationFragment= ast.newVariableDeclarationFragment();
> 		resolvedVariableDeclarationFragment.setName(resolvedVariableName);
> 		VariableDeclarationExpression resolvedVariableDeclaration= ast.newVariableDeclarationExpression(resolvedVariableDeclarationFragment);
>-		resolvedVariableDeclaration.setType(ast.newSimpleType(ast.newSimpleName(elementTypename)));
>+		resolvedVariableDeclaration.setType(getImportRewrite().addImport(loopOverType, ast, new ContextSensitiveImportRewriteContext(fCurrentNode, getImportRewrite())));
> 		assignResolvedVariable.setLeftHandSide(resolvedVariableDeclaration);
> 
> 		// right hand side
>@@ -291,10 +272,10 @@
> 		ASTRewrite rewrite= ASTRewrite.create(ast);
> 
> 		ForStatement loopStatement= ast.newForStatement();
>-		SimpleName loopVariableName= resolveLinkedVariableNameWithProposals(ast, rewrite, "i", true); //$NON-NLS-1$
>+		SimpleName loopVariableName= resolveLinkedVariableNameWithProposals(rewrite, "i", true); //$NON-NLS-1$
> 		loopStatement.initializers().add(getIndexBasedForInitializer(ast, loopVariableName));
>-		loopStatement.setExpression(getLinkedInfixExpression(ast, rewrite, loopVariableName.getIdentifier()));
>-		loopStatement.updaters().add(getLinkedIncrementExpression(ast, rewrite, loopVariableName.getIdentifier()));
>+		loopStatement.setExpression(getLinkedInfixExpression(rewrite, loopVariableName.getIdentifier()));
>+		loopStatement.updaters().add(getLinkedIncrementExpression(rewrite, loopVariableName.getIdentifier()));
> 
> 		Block forLoopBody= ast.newBlock();
> 		forLoopBody.statements().add(createBlankLineStatementWithCursorPosition(rewrite));
>@@ -307,13 +288,13 @@
> 	/**
> 	 * Creates an {@link InfixExpression} which is linked to the group of the variableToIncrement.
> 	 * 
>-	 * @param ast the current {@link AST} instance
> 	 * @param rewrite the current {@link ASTRewrite} instance
> 	 * @param variableToIncrement the name of the variable to generate the {@link InfixExpression}
> 	 *            for
> 	 * @return a filled, new {@link InfixExpression} instance
> 	 */
>-	private InfixExpression getLinkedInfixExpression(AST ast, ASTRewrite rewrite, String variableToIncrement) {
>+	private InfixExpression getLinkedInfixExpression(ASTRewrite rewrite, String variableToIncrement) {
>+		AST ast= rewrite.getAST();
> 		InfixExpression loopExpression= ast.newInfixExpression();
> 		SimpleName name= ast.newSimpleName(variableToIncrement);
> 		addLinkedPosition(rewrite.track(name), false, name.getIdentifier());
>@@ -321,7 +302,7 @@
> 		loopExpression.setOperator(InfixExpression.Operator.LESS);
> 
> 		FieldAccess getArrayLengthExpression= ast.newFieldAccess();
>-		getArrayLengthExpression.setExpression((Expression) ASTNode.copySubtree(ast, fSubExpression));
>+		getArrayLengthExpression.setExpression((Expression) rewrite.createCopyTarget(fSubExpression));
> 		getArrayLengthExpression.setName(ast.newSimpleName("length")); //$NON-NLS-1$
> 
> 		loopExpression.setRightOperand(getArrayLengthExpression);
>@@ -332,13 +313,13 @@
> 	 * Creates a {@link PostfixExpression} used to increment the loop variable of a <code>for</code>
> 	 * loop to iterate over an array.
> 	 * 
>-	 * @param ast the current {@link AST} instance
> 	 * @param rewrite the current {@link ASTRewrite} instance
> 	 * @param variableToIncrement the name of the variable to increment
> 	 * @return a filled {@link PostfixExpression} realizing an incrementation of the specified
> 	 *         variable
> 	 */
>-	private Expression getLinkedIncrementExpression(AST ast, ASTRewrite rewrite, String variableToIncrement) {
>+	private Expression getLinkedIncrementExpression(ASTRewrite rewrite, String variableToIncrement) {
>+		AST ast= rewrite.getAST();
> 		PostfixExpression incrementLoopVariable= ast.newPostfixExpression();
> 		SimpleName name= ast.newSimpleName(variableToIncrement);
> 		addLinkedPosition(rewrite.track(name), false, name.getIdentifier());
>@@ -348,7 +329,7 @@
> 	}
> 
> 	/**
>-	 * Generates an {@link VariableDeclarationExpression}, which initializes the loop variable to
>+	 * Generates a {@link VariableDeclarationExpression}, which initializes the loop variable to
> 	 * iterate over an array.
> 	 * 
> 	 * @param ast the current {@link AST} instance
>@@ -375,14 +356,14 @@
> 	 * Resolves name proposals by the given basename and adds a {@link LinkedPosition} to the
> 	 * returned {@link SimpleName} expression.
> 	 * 
>-	 * @param ast the current {@link AST}
> 	 * @param rewrite the current instance of an {@link ASTRewrite}
> 	 * @param basename the base string to use for proposal calculation
> 	 * @param firstLinkedProposal true if the generated name is the first {@link LinkedPosition} to
> 	 *            edit in the current {@link CompilationUnit}, false otherwise
> 	 * @return the linked {@link SimpleName} instance based on the name proposals
> 	 */
>-	private SimpleName resolveLinkedVariableNameWithProposals(AST ast, ASTRewrite rewrite, String basename, boolean firstLinkedProposal) {
>+	private SimpleName resolveLinkedVariableNameWithProposals(ASTRewrite rewrite, String basename, boolean firstLinkedProposal) {
>+		AST ast= rewrite.getAST();
> 		String[] nameProposals= getVariableNameProposals(basename);
> 		SimpleName forDeclarationName= (nameProposals.length > 0 ? ast.newSimpleName(nameProposals[0]) : ast.newSimpleName(basename));
> 		for (int i= 0; i < nameProposals.length; i++) {
>@@ -430,19 +411,19 @@
> 	 * to iterate over an array using <code>foreach</code>.
> 	 * 
> 	 * @param ast the current {@link AST} instance
>-	 * @return the string representation of the type's unqualified name
>+	 * @return the {@link ITypeBinding} of the elements to iterate over
> 	 */
>-	private String extractElementTypeName(AST ast) {
>+	private ITypeBinding extractElementType(AST ast) {
> 		ITypeBinding binding= fSubExpression.resolveTypeBinding();
> 		if (binding.isArray()) {
>-			return Bindings.normalizeForDeclarationUse(binding.getElementType(), ast).getName();
>-		}
>-		ITypeBinding[] typeArguments= Bindings.findTypeInHierarchy(binding, "java.lang.Iterable").getTypeArguments(); //$NON-NLS-1$
>-		if (typeArguments != null && typeArguments.length > 0) {
>-			return Bindings.normalizeForDeclarationUse(typeArguments[0], ast).getName();
>+			return Bindings.normalizeForDeclarationUse(binding.getElementType(), ast);
> 		}
> 
>-		return null;
>+		// extract elements type directly out of the bindings
>+		IMethodBinding iteratorMethodBinding= Bindings.findMethodInHierarchy(fCurrentExpression.resolveTypeBinding(), "iterator", new ITypeBinding[] {}); //$NON-NLS-1$
>+		IMethodBinding iteratorNextMethodBinding= Bindings.findMethodInHierarchy(iteratorMethodBinding.getReturnType(), "next", new ITypeBinding[] {}); //$NON-NLS-1$
>+
>+		return iteratorNextMethodBinding.getReturnType();
> 	}
> 
> }

Actions: View | Diff

Attachments on bug 241696: 108083 | 236902 | 237236 | 237461