Fix ComparingOnCriteriaCleanUp to handle wildcard types (#925)

- fixes #174 - add logic to buildField to handle wildcard types - add new scenario to current test in CleanUpTest1d8
eclipse-jdt · Dec 6, 2023 · 3703afa · 3703afa
1 parent 9dbce6b
commit 3703afa
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 2 deletions.
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest1d8.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest1d8.java
@@ -2004,6 +2004,7 @@ public void testComparingOnCriteria() throws Exception {
 				+ "import java.util.Date;\n" //
 				+ "import java.util.List;\n" //
 				+ "import java.util.Locale;\n" //
+				+ "import java.util.TreeSet;\n" //
 				+ "\n" //
 				+ "public class E {\n" //
 				+ "    private Comparator<Date> refactorField = new Comparator<Date>() {\n" //
@@ -2404,6 +2405,13 @@ public void testComparingOnCriteria() throws Exception {
 				+ "\n" //
 				+ "        return listToSort;\n" //
 				+ "    }\n" //
+				+ "\n" //
+				+ "    public class FooBar {\n" //
+				+ "        public String value;\n" //
+				+ "    }\n" //
+				+ "\n" //
+				+ "    private final TreeSet<FooBar> foo = new TreeSet<>((a,b) -> b.value.compareTo(a.value));\n" //
+				+ "\n" //
 				+ "}\n";
 
 		String expected= "" //
@@ -2415,6 +2423,7 @@ public void testComparingOnCriteria() throws Exception {
 				+ "import java.util.Date;\n" //
 				+ "import java.util.List;\n" //
 				+ "import java.util.Locale;\n" //
+				+ "import java.util.TreeSet;\n" //
 				+ "\n" //
 				+ "public class E {\n" //
 				+ "    private Comparator<Date> refactorField = Comparator.comparing(Date::toString);\n" //
@@ -2608,6 +2617,13 @@ public void testComparingOnCriteria() throws Exception {
 				+ "\n" //
 				+ "        return listToSort;\n" //
 				+ "    }\n" //
+				+ "\n" //
+				+ "    public class FooBar {\n" //
+				+ "        public String value;\n" //
+				+ "    }\n" //
+				+ "\n" //
+				+ "    private final TreeSet<FooBar> foo = new TreeSet<>(Comparator.comparing((FooBar a) -> a.value).reversed());\n" //
+				+ "\n" //
 				+ "}\n";
 
 		// When

diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/ComparingOnCriteriaCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/ComparingOnCriteriaCleanUp.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2021 Fabrice TIERCELIN and others.
+ * Copyright (c) 2021, 2023 Fabrice TIERCELIN and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -26,6 +26,8 @@
 import org.eclipse.text.edits.TextEditGroup;
 
 import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.IType;
+import org.eclipse.jdt.core.JavaModelException;
 import org.eclipse.jdt.core.dom.AST;
 import org.eclipse.jdt.core.dom.ASTVisitor;
 import org.eclipse.jdt.core.dom.AnonymousClassDeclaration;
@@ -53,6 +55,7 @@
 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.core.dom.rewrite.ImportRewrite.TypeLocation;
 
 import org.eclipse.jdt.internal.corext.dom.ASTNodeFactory;
 import org.eclipse.jdt.internal.corext.dom.ASTNodes;
@@ -70,6 +73,7 @@
 
 import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
 import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
+
 import org.eclipse.jdt.internal.ui.text.correction.IProblemLocationCore;
 
 /**
@@ -571,11 +575,37 @@ private LambdaExpression buildField(final Expression node, final ITypeBinding ty
 				lambdaExpression.parameters().add(newVariableDeclarationFragment);
 			} else {
 				String comparedClassNameText= importRewrite.addImport(type);
+				String importedName= comparedClassNameText;
 
 				SingleVariableDeclaration newSingleVariableDeclaration= ast.newSingleVariableDeclaration();
-				newSingleVariableDeclaration.setType(ast.newSimpleType(ASTNodeFactory.newName(ast, comparedClassNameText)));
+				if (type.isWildcardType()) {
+					ITypeBinding bound= type.getBound();
+					Type boundType= importRewrite.addImport(bound, ast, importRewrite.getDefaultImportRewriteContext(), TypeLocation.TYPE_BOUND);
+					newSingleVariableDeclaration.setType(boundType);
+					if (bound.isLocal()) {
+						importRewrite.removeImport(bound.getQualifiedName());
+					}
+					importedName= bound.getQualifiedName();
+				} else {
+					newSingleVariableDeclaration.setType(ast.newSimpleType(ASTNodeFactory.newName(ast, comparedClassNameText)));
+				}
+
 				newSingleVariableDeclaration.setName((SimpleName) rewrite.createCopyTarget(name));
 				lambdaExpression.parameters().add(newSingleVariableDeclaration);
+				if (type.isLocal()) {
+					importRewrite.removeImport(importedName);
+				} else {
+					try {
+						IType[] cuTypes= importRewrite.getCompilationUnit().getTypes();
+						for (IType cuType : cuTypes) {
+							if (importedName.startsWith(cuType.getFullyQualifiedName())) {
+								importRewrite.removeImport(importedName);
+							}
+						}
+					} catch (JavaModelException e) {
+						// should not occur
+					}
+				}
 			}
 
 			FieldAccess newFieldAccess= ast.newFieldAccess();