Fix Iterator import removal bug for combined loops

org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/core/manipulation/CleanUpContextCore.java

@@ -19,6 +19,8 @@
 import org.eclipse.jdt.core.ICompilationUnit;
 import org.eclipse.jdt.core.dom.CompilationUnit;
 
+import org.eclipse.jdt.internal.corext.refactoring.structure.ImportRemover;
+
 
 /**
  * The context that contains all information required by a clean up to create a fix.
@@ -33,6 +35,8 @@
 
 	private final CompilationUnit fAst;
 
+	private ImportRemover fSharedImportRemover;
+
 	/**
 	 * Creates a new clean up context.
 	 *
@@ -75,4 +79,25 @@
 	public CompilationUnit getAST() {
 		return fAst;
 	}
+
+	/**
+	 * Sets the shared ImportRemover for this cleanup context.
+	 * This allows multiple cleanups to share a single ImportRemover instance.
+	 *
+	 * @param remover the ImportRemover to share across cleanups
+	 * @since 1.13
+	 */
+	public void setSharedImportRemover(ImportRemover remover) {
+		fSharedImportRemover = remover;
+	}
+
+	/**
+	 * Gets the shared ImportRemover for this cleanup context.
+	 *
+	 * @return the shared ImportRemover, or <code>null</code> if none has been set
+	 * @since 1.13
+	 */
+	public ImportRemover getSharedImportRemover() {
+		return fSharedImportRemover;
+	}
 }

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CompilationUnitRewriteOperationsFixCore.java

@@ -39,6 +39,7 @@
 
 import org.eclipse.jdt.internal.corext.codemanipulation.ContextSensitiveImportRewriteContext;
 import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
+import org.eclipse.jdt.internal.corext.refactoring.structure.ImportRemover;
 
 public class CompilationUnitRewriteOperationsFixCore extends AbstractFixCore {
 
@@ -87,6 +88,7 @@ public String getAdditionalInfo() {
 	private final CompilationUnitRewriteOperation[] fOperations;
 	private final CompilationUnit fCompilationUnit;
 	protected LinkedProposalModelCore fLinkedProposalModel;
+	private ImportRemover fSharedImportRemover;
 
 	public CompilationUnitRewriteOperationsFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation operation) {
 		this(name, compilationUnit, new CompilationUnitRewriteOperation[] { operation });
@@ -114,10 +116,26 @@ public LinkedProposalModelCore getLinkedPositions() {
 		return fLinkedProposalModel;
 	}
 
+	/**
+	 * Sets the shared ImportRemover to be used by this fix.
+	 * This allows multiple fixes to share a single ImportRemover instance.
+	 *
+	 * @param remover the ImportRemover to share
+	 * @since 1.13
+	 */
+	public void setSharedImportRemover(ImportRemover remover) {
+		fSharedImportRemover = remover;
+	}
+
 	@Override
 	public CompilationUnitChange createChange(IProgressMonitor progressMonitor) throws CoreException {
 		CompilationUnitRewrite cuRewrite= new CompilationUnitRewrite((ICompilationUnit)fCompilationUnit.getJavaElement(), fCompilationUnit);
 
+		// Use shared ImportRemover if available
+		if (fSharedImportRemover != null) {
+			cuRewrite.setImportRemover(fSharedImportRemover);
+		}
+
 		fLinkedProposalModel.clear();
 		for (CompilationUnitRewriteOperation operation : fOperations) {
 			operation.rewriteAST(cuRewrite, fLinkedProposalModel);

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/helper/WhileToForEach.java

@@ -564,7 +564,6 @@ public void rewrite(UseIteratorToForLoopFixCore upp, final WhileLoopToChangeHit
 		newEnhancedForStatement.setBody(ASTNodes.createMoveTarget(rewrite, hit.whileStatement.getBody()));
 		ASTNodes.replaceButKeepComment(rewrite, hit.whileStatement, newEnhancedForStatement, group);
 		remover.registerRemovedNode(hit.whileStatement.getExpression());
-		remover.applyRemoves(importRewrite);
 	}
 
 	private boolean isLocalOrMemberType(ITypeBinding binding, WhileStatement whileStatement) {

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/structure/CompilationUnitRewrite.java

@@ -392,6 +392,17 @@ public ImportRemover getImportRemover() {
 		return fImportRemover;
 	}
 
+	/**
+	 * Sets an external ImportRemover to be used by this CompilationUnitRewrite.
+	 * This allows multiple CompilationUnitRewrite instances to share the same ImportRemover.
+	 *
+	 * @param remover the ImportRemover to use
+	 * @since 1.13
+	 */
+	public void setImportRemover(ImportRemover remover) {
+		fImportRemover = remover;
+	}
+
 	private void clearGroupDescriptionEdits() {
 		for (TextEditGroup group : fTextEditGroups) {
 			group.clearTextEdits();

org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest1d8.java

@@ -5607,7 +5607,6 @@ private static void dumpIMethod(String next) {
 		String expected= """
 			package test;
 			import java.io.File;
-			import java.util.Iterator;
 			import java.util.List;
 
 			public class Test {
@@ -7367,6 +7366,61 @@ void test1() {
 
 	}
 
+	/**
+	 * https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/121
+	 * Test that unused Iterator import is removed when both for-loop and while-loop are converted
+	 *
+	 * @throws CoreException on failure
+	 */
+	@Test
+	public void testIssue121_CombinedForAndWhileLoopIteratorImportRemoval() throws CoreException {
+		IPackageFragment pack= fSourceFolder.createPackageFragment("test", false, null);
+		String sample= """
+			package test;
+
+			import java.util.Date;
+			import java.util.Iterator;
+			import java.util.List;
+			import java.util.Map;
+
+			public class Test {
+			    private void method(List<String> list, Map<Date, List<String>> map) {
+			        Iterator<String> removed = list.iterator();
+			        while (removed.hasNext()) {
+			            System.out.println(removed.next());
+			        }
+			        for (Iterator<List<String>> value = map.values().iterator(); value.hasNext();) {
+			            System.out.println(value.next());
+			        }
+			    }
+			}
+			""";
+		ICompilationUnit cu= pack.createCompilationUnit("Test.java", sample, false, null);
+
+		enable(CleanUpConstants.CONTROL_STATEMENTS_CONVERT_FOR_LOOP_TO_ENHANCED);
+
+		String expected= """
+			package test;
+
+			import java.util.Date;
+			import java.util.List;
+			import java.util.Map;
+
+			public class Test {
+			    private void method(List<String> list, Map<Date, List<String>> map) {
+			        for (String element : list) {
+			            System.out.println(element);
+			        }
+			        for (List<String> list2 : map.values()) {
+			            System.out.println(list2);
+			        }
+			    }
+			}
+			""";
+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected },
+				new HashSet<>(Arrays.asList(FixMessages.Java50Fix_ConvertToEnhancedForLoop_description)));
+	}
+
 	@Test
 	public void testRemoveSuppressWarnings() throws Exception {
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);

org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpRefactoring.java

@@ -75,6 +75,7 @@
 import org.eclipse.jdt.internal.corext.refactoring.Checks;
 import org.eclipse.jdt.internal.corext.refactoring.changes.DynamicValidationStateChange;
 import org.eclipse.jdt.internal.corext.refactoring.changes.MultiStateCompilationUnitChange;
+import org.eclipse.jdt.internal.corext.refactoring.structure.ImportRemover;
 import org.eclipse.jdt.internal.corext.refactoring.util.RefactoringASTParser;
 import org.eclipse.jdt.internal.corext.refactoring.util.TextEditUtil;
 import org.eclipse.jdt.internal.corext.util.Messages;
@@ -782,6 +783,14 @@ public static CleanUpChange calculateChange(CleanUpContext context, ICleanUp[] c
 		if (cleanUps.length == 0)
 			return null;
 
+		// Create shared ImportRemover for all cleanups on this compilation unit
+		CompilationUnit ast = context.getAST();
+		if (ast != null) {
+			IJavaProject project = context.getCompilationUnit().getJavaProject();
+			ImportRemover sharedRemover = new ImportRemover(project, ast);
+			context.setSharedImportRemover(sharedRemover);
+		}
+
 		CleanUpChange solution= null;
 		int i= 0;
 		do {
@@ -796,6 +805,10 @@ public static CleanUpChange calculateChange(CleanUpContext context, ICleanUp[] c
 				fix= cleanUp.createFix(context);
 			}
 			if (fix != null) {
+				// Pass shared ImportRemover to fix if it supports it
+				if (fix instanceof CompilationUnitRewriteOperationsFixCore && context.getSharedImportRemover() != null) {
+					((CompilationUnitRewriteOperationsFixCore) fix).setSharedImportRemover(context.getSharedImportRemover());
+				}
 				CompilationUnitChange current= fix.createChange(null);
 				TextEdit currentEdit= current.getEdit();