fix: Avoid import duplication on unresolved imports (#3730)

src/main/java/spoon/reflect/visitor/ImportCleaner.java

@@ -38,6 +38,7 @@
 import spoon.support.visitor.ClassTypingContext;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -198,7 +199,7 @@ void onCompilationUnitProcessed(CtCompilationUnit compilationUnit) {
 			ModelList<CtImport> existingImports = compilationUnit.getImports();
 			Set<CtImport> computedImports = new HashSet<>(this.computedImports.values());
 			topfor: for (CtImport oldImport : new ArrayList<>(existingImports)) {
-				if (!computedImports.remove(oldImport)) {
+				if (!removeImport(oldImport, computedImports)) {
 
 					// case: import is required in Javadoc
 					for (CtType type: compilationUnit.getDeclaredTypes()) {
@@ -253,8 +254,25 @@ void onCompilationUnitProcessed(CtCompilationUnit compilationUnit) {
 				existingImports.set(existingImports.stream().sorted(importComparator).collect(Collectors.toList()));
 			}
 		}
+
+		/**
+		 * Remove an import from the given collection based on equality or textual equality.
+		 */
+		private boolean removeImport(CtImport toRemove, Collection<CtImport> imports) {
+			String toRemoveStr = toRemove.toString();
+			Iterator<CtImport> it = imports.iterator();
+			while (it.hasNext()) {
+				CtImport imp = it.next();
+				if (toRemove.equals(imp) || toRemoveStr.equals(imp.toString())) {
+					it.remove();
+					return true;
+				}
+			}
+			return false;
+		}
 	}
 
+
 	/**
 	 * @return fast unique identification of reference. It is not the same like printing of import, because it needs to handle access path.
 	 */

src/test/java/spoon/reflect/visitor/ImportCleanerTest.java

@@ -0,0 +1,43 @@
+package spoon.reflect.visitor;
+
+import org.junit.Test;
+import spoon.Launcher;
+import spoon.reflect.CtModel;
+import spoon.reflect.declaration.CtCompilationUnit;
+import spoon.reflect.declaration.CtImport;
+import spoon.reflect.declaration.CtType;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class ImportCleanerTest {
+
+	@Test
+	public void testDoesNotDuplicateUnresolvedImports() {
+	    // contract: The import cleaner should not duplicate unresolved imports
+
+		// arrange
+		Launcher launcher = new Launcher();
+		launcher.addInputResource("./src/test/resources/unresolved/UnresolvedImport.java");
+		CtModel model = launcher.buildModel();
+		CtType<?> type = model.getAllTypes().stream().findFirst().get();
+		CtCompilationUnit cu = type.getFactory().CompilationUnit().getOrCreate(type);
+		List<String> importsBefore = getTextualImports(cu);
+
+		// act
+		new ImportCleaner().process(cu);
+
+		// assert
+		List<String> importsAfter = getTextualImports(cu);
+		assertThat(importsAfter, equalTo(importsBefore));
+	}
+
+	private static List<String> getTextualImports(CtCompilationUnit cu) {
+		return cu.getImports().stream()
+				.map(CtImport::toString)
+				.collect(Collectors.toList());
+	}
+}

src/test/resources/unresolved/UnresolvedImport.java

@@ -0,0 +1,8 @@
+// this import will be unresolved as the package does not exist
+import non.existing.pkg.SomeClass;
+
+public class UnresolvedImport {
+    public static void main(String[] args) {
+        SomeClass instance = new SomeClass();
+    }
+}