Bug 99673 - [generalize type] does not remove unused imports

Change-Id: Ide290f2ce34434ee9a5b45b0168b2e7b78cbdcef
Signed-off-by: Kalyan Prasad Tatavarthi <kalyan_prasad@in.ibm.com>

8 files changed, 66 insertions, 13 deletions

diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testLocalVarDecl_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testLocalVarDecl_out.java
index 1d5d5d362b..48f7c76e79 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testLocalVarDecl_out.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testLocalVarDecl_out.java
@@ -1,5 +1,4 @@
 import java.util.HashMap;
-import java.util.Hashtable;
 import java.util.Map;
 import java.util.Properties;
 
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithNonParametricSuperType_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithNonParametricSuperType_out.java
index c3a107d5c9..d76f31b7bb 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithNonParametricSuperType_out.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithNonParametricSuperType_out.java
@@ -1,5 +1,3 @@
-import java.util.List;
-
 public class A_testParametricTypeWithNonParametricSuperType_in {
 	void foo(){
 		Object x = null;
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_in.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_in.java
new file mode 100644
index 0000000000..e74eb34521
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_in.java
@@ -0,0 +1,8 @@
+import java.util.List;
+
+public class A_testParametricTypeWithParametricSuperType2_in {
+	void foo(){
+		List<String> x = null;
+		List<String> y = null;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_out.java
new file mode 100644
index 0000000000..2d34f3e49d
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType2_out.java
@@ -0,0 +1,9 @@
+import java.util.Collection;
+import java.util.List;
+
+public class A_testParametricTypeWithParametricSuperType2_in {
+	void foo(){
+		Collection<String> x = null;
+		List<String> y = null;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType_out.java
index 3b78cfe5bf..18a1d0ee82 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType_out.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testParametricTypeWithParametricSuperType_out.java
@@ -1,6 +1,4 @@
 import java.util.Collection;
-import java.util.List;
-
 public class A_testParametricTypeWithParametricSuperType_in {
 	void foo(){
 		Collection<String> x = null;
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testUnboundTypeParameter_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testUnboundTypeParameter_out.java
index 0bd678dd87..a2e2afa5f1 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testUnboundTypeParameter_out.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ChangeTypeRefactoring/positive/A_testUnboundTypeParameter_out.java
@@ -1,5 +1,3 @@
-import java.util.List;
-
 public class A_testUnboundTypeParameter_in {
 	public <T> void baz() {
 		Iterable<T> list= null;
diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
index 61c8a5706c..1cc34d1dc1 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ChangeTypeRefactoringTests.java
@@ -341,6 +341,15 @@ public class ChangeTypeRefactoringTests extends RefactoringTest {
 				"java.lang.Iterable<java.lang.String>"  };
 		StringAsserts.assertEqualStringsIgnoreOrder(actual, expected);
 	}
+	public void testParametricTypeWithParametricSuperType2() throws Exception {
+		Collection<String> types= helper1(5, 22, 5, 22, "java.util.Collection<java.lang.String>").getValidTypeNames();
+		String[] actual= types.toArray(new String[types.size()]);
+		String[] expected= {
+				"java.util.Collection<java.lang.String>",
+				"java.lang.Object",
+				"java.lang.Iterable<java.lang.String>"  };
+		StringAsserts.assertEqualStringsIgnoreOrder(actual, expected);
+	}
 	public void testParametricTypeWithNonParametricSuperType() throws Exception {
 		Collection<String> types= helper1(5, 22, 5, 22, "java.lang.Object").getValidTypeNames();
 		String[] actual= types.toArray(new String[types.size()]);
diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
index b45671c9b0..8d4cf82383 100644
--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeTypeRefactoring.java
@@ -56,6 +56,7 @@ import org.eclipse.jdt.core.dom.IBinding;
 import org.eclipse.jdt.core.dom.IMethodBinding;
 import org.eclipse.jdt.core.dom.ITypeBinding;
 import org.eclipse.jdt.core.dom.IVariableBinding;
+import org.eclipse.jdt.core.dom.ImportDeclaration;
 import org.eclipse.jdt.core.dom.MethodDeclaration;
 import org.eclipse.jdt.core.dom.Modifier;
 import org.eclipse.jdt.core.dom.NodeFinder;
@@ -544,8 +545,10 @@ public class ChangeTypeRefactoring extends Refactoring {
 		MultiTextEdit root= new MultiTextEdit();
 		unitChange.setEdit(root); // Adam sez don't need this, but then unitChange.addGroupDescription() fails an assertion!
 
+		ImportRemover remover= new ImportRemover(fCu.getJavaProject(), unit);
 		String typeName= updateImports(unit, root);
-		updateCu(unit, vars, unitChange, unitRewriter, typeName);
+		updateCu(unit, vars, unitChange, unitRewriter, typeName, remover);
+		removeUnusedImport(remover,unit, root);
 		root.addChild(unitRewriter.rewriteAST());
 	}
 
@@ -557,7 +560,7 @@ public class ChangeTypeRefactoring extends Refactoring {
 	}
 
 	private void updateCu(CompilationUnit unit, Set<ConstraintVariable> vars, CompilationUnitChange unitChange,
-		ASTRewrite unitRewriter, String typeName) throws JavaModelException {
+		ASTRewrite unitRewriter, String typeName, ImportRemover remover) throws JavaModelException {
 
         // use custom SourceRangeComputer to avoid losing comments
 		unitRewriter.setTargetSourceRangeComputer(new SourceRangeComputer());
@@ -566,17 +569,17 @@ public class ChangeTypeRefactoring extends Refactoring {
 			ASTNode decl= findDeclaration(unit, cv);
 			if ((decl instanceof SimpleName || decl instanceof QualifiedName) && cv instanceof ExpressionVariable) {
 				ASTNode gp= decl.getParent().getParent();
-				updateType(unit, getType(gp), unitChange, unitRewriter, typeName);   // local variable or parameter
+				updateType(unit, getType(gp), unitChange, unitRewriter, typeName, remover);   // local variable or parameter
 			} else if (decl instanceof MethodDeclaration || decl instanceof FieldDeclaration) {
-				updateType(unit, getType(decl), unitChange, unitRewriter, typeName); // method return or field type
+				updateType(unit, getType(decl), unitChange, unitRewriter, typeName, remover); // method return or field type
 			} else if (decl instanceof ParameterizedType){
-				updateType(unit, getType(decl), unitChange, unitRewriter, typeName);
+				updateType(unit, getType(decl), unitChange, unitRewriter, typeName, remover);
 			}
 		}
 	}
 
 	private void updateType(CompilationUnit cu, Type oldType, CompilationUnitChange unitChange,
-							ASTRewrite unitRewriter, String typeName) {
+							ASTRewrite unitRewriter, String typeName, ImportRemover remover) {
 
 		String oldName= fSelectionTypeBinding.getName();
 		String[] keys= { BasicElementLabels.getJavaElementName(oldName), BasicElementLabels.getJavaElementName(typeName)};
@@ -602,6 +605,7 @@ public class ChangeTypeRefactoring extends Refactoring {
 		}
 
 		unitRewriter.replace(nodeToReplace, newType, gd);
+		remover.registerRemovedNode(nodeToReplace);
 		unitChange.addTextEditGroup(gd);
 	}
 
@@ -1360,6 +1364,36 @@ public class ChangeTypeRefactoring extends Refactoring {
 		return typeName;
 	}
 
+	private void removeUnusedImport(ImportRemover remover, CompilationUnit astRoot, MultiTextEdit rootEdit) throws CoreException{
+		List<ImportDeclaration> declList= astRoot.imports();
+		ImportDeclaration oldImport= null;
+		String importStr= getImport(fSelectionTypeBinding.getQualifiedName());
+		for (ImportDeclaration decl: declList) {
+			if (decl.getName().getFullyQualifiedName().equals(importStr)) {
+				oldImport= decl;
+				break;
+			}
+		}
+		if (oldImport != null) {
+			ImportRewrite rewrite= StubUtility.createImportRewrite(astRoot, true);
+			remover.applyRemoves(rewrite);
+			rootEdit.addChild(rewrite.rewriteImports(null));
+		}
+	}
+
+	private String getImport(String qualifiedTypeName) {
+		String finalStr= qualifiedTypeName;
+		int angleBracketOffset= qualifiedTypeName.indexOf('<');
+		if (angleBracketOffset != -1) {
+			finalStr= qualifiedTypeName.substring(0, angleBracketOffset);
+		}
+		int bracketOffset= qualifiedTypeName.indexOf('[');
+		if (bracketOffset != -1) {
+			finalStr= qualifiedTypeName.substring(0, bracketOffset);
+		}
+		return finalStr;
+	}
+
 	//	------------------------------------------------------------------------------------------------- //
 	// Miscellaneous helper methods