From ca2de7b7da38bef7eb5b88df9e097e8868d16343 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Tue, 30 Jul 2024 11:58:57 -0400 Subject: [PATCH] Fix "fix return type" quickfix for type param return type The old method would propose the erasure of the type param if the compliance was set pre 1.5. If the compliance Instead, now two quickfixes are always propose: 1. Replace the return type with the erasure 2. Introduce a new type param with the same bounds, importing the bounds and adding the type variable for any raw types Fixes #1558 Signed-off-by: David Thompson --- .../TypeMismatchBaseSubProcessor.java | 19 +- .../TypeChangeCorrectionProposalCore.java | 78 ++++ .../quickfix/TypeMismatchQuickFixTests.java | 405 +++++++++++++++++- 3 files changed, 480 insertions(+), 22 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/TypeMismatchBaseSubProcessor.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/TypeMismatchBaseSubProcessor.java index 05b5ddc292c..ac5341681e3 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/TypeMismatchBaseSubProcessor.java +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/TypeMismatchBaseSubProcessor.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.stream.Stream; import org.eclipse.core.runtime.CoreException; @@ -381,12 +382,20 @@ public void collectIncompatibleReturnTypeProposals(IInvocationContext context, I ICompilationUnit cu= context.getCompilationUnit(); IMethodBinding methodDecl= methodDeclBinding.getMethodDeclaration(); ITypeBinding overriddenReturnType= overridden.getReturnType(); - if (! JavaModelUtil.is50OrHigher(context.getCompilationUnit().getJavaProject())) { - overriddenReturnType= overriddenReturnType.getErasure(); + // propose erasure + if (decl.typeParameters().isEmpty() || overriddenReturnType.getTypeBounds().length == 0 || Stream.of(overriddenReturnType.getTypeBounds()).allMatch(bound -> bound.getTypeArguments().length == 0)) { + T p1= createChangeIncompatibleReturnTypeProposal(cu, methodDecl, astRoot, overriddenReturnType.getErasure(), false, IProposalRelevance.CHANGE_RETURN_TYPE); + if (p1 != null) + proposals.add(p1); + } + + // propose using (and potentially introducing) the type variable + if (overriddenReturnType.isTypeVariable()) { + T p2 = createChangeIncompatibleReturnTypeProposal(cu, methodDecl, astRoot, overriddenReturnType, false, IProposalRelevance.CHANGE_RETURN_TYPE); + if (p2 != null) { + proposals.add(p2); + } } - T p1= createChangeIncompatibleReturnTypeProposal(cu, methodDecl, astRoot, overriddenReturnType, false, IProposalRelevance.CHANGE_RETURN_TYPE); - if (p1 != null) - proposals.add(p1); ICompilationUnit targetCu= cu; diff --git a/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/proposals/TypeChangeCorrectionProposalCore.java b/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/proposals/TypeChangeCorrectionProposalCore.java index 46e0908536c..cb55700f7c7 100644 --- a/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/proposals/TypeChangeCorrectionProposalCore.java +++ b/org.eclipse.jdt.core.manipulation/proposals/org/eclipse/jdt/internal/ui/text/correction/proposals/TypeChangeCorrectionProposalCore.java @@ -23,6 +23,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; import org.eclipse.core.runtime.Assert; import org.eclipse.core.runtime.CoreException; @@ -54,13 +55,17 @@ import org.eclipse.jdt.core.dom.Modifier; import org.eclipse.jdt.core.dom.ParameterizedType; 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.TagElement; import org.eclipse.jdt.core.dom.TextElement; import org.eclipse.jdt.core.dom.Type; +import org.eclipse.jdt.core.dom.TypeParameter; import org.eclipse.jdt.core.dom.VariableDeclarationExpression; import org.eclipse.jdt.core.dom.VariableDeclarationFragment; import org.eclipse.jdt.core.dom.VariableDeclarationStatement; +import org.eclipse.jdt.core.dom.WildcardType; import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jdt.core.dom.rewrite.ImportRewrite; import org.eclipse.jdt.core.dom.rewrite.ImportRewrite.ImportRewriteContext; @@ -276,6 +281,40 @@ protected ASTRewrite getRewrite() throws CoreException { rewrite.set(methodDecl, MethodDeclaration.RETURN_TYPE2_PROPERTY, type, null); DimensionRewrite.removeAllChildren(methodDecl, MethodDeclaration.EXTRA_DIMENSIONS2_PROPERTY, rewrite, null); TypeAnnotationRewrite.removePureTypeAnnotations(methodDecl, MethodDeclaration.MODIFIERS2_PROPERTY, rewrite, null); + + // If the return type is a type variable that doesn't exist yet, + // we'll need to add all the type variables and update the parameter types to use the type variables + if (fNewType.isTypeVariable() && ((List)methodDecl.typeParameters()).stream().filter(p -> p.getName().toString().equals(fNewType.getName())).findAny().isEmpty()) { + IMethodBinding realMethodBinding = fNewType.getDeclaringMethod(); + ITypeBinding[] typeParameters = realMethodBinding.getTypeParameters(); + ListRewrite typeParameterRewrite = rewrite.getListRewrite(methodDecl, MethodDeclaration.TYPE_PARAMETERS_PROPERTY); + + // clean out old method type parameters + ((List)typeParameterRewrite.getOriginalList()).stream().forEach(typeVar -> typeParameterRewrite.remove(typeVar, null)); + + // add type parameters and import any bounds + // notably, if you add the type variables, you must add ALL of them. + // eg. if you have T myMethod(Class clazz) + // you cannot override with T myMethod(Class clazz) + for (ITypeBinding parameter : typeParameters) { + TypeParameter newTypeParameter = ast.newTypeParameter(); + SimpleName newTypeParameterName = ast.newSimpleName(parameter.getName()); + newTypeParameter.setName(newTypeParameterName); + Stream.of(parameter.getTypeBounds()) // + .forEach(bound -> { + newTypeParameter.typeBounds().add(getTypeNodeFromBinding(bound, ast, imports)); + }); + typeParameterRewrite.insertLast(newTypeParameter, null); + } + + // Update the parameter types to match that of the resolved method. + // Some of the existing parameter types may be raw instead of containing the expected type parameters. + // Without inserting the type parameters in these cases, the signature will no longer match the overriden type. + for (int i = 0 ; i < methodDecl.parameters().size(); i++) { + SingleVariableDeclaration svd = (SingleVariableDeclaration)methodDecl.parameters().get(i); + rewrite.set(svd, SingleVariableDeclaration.TYPE_PROPERTY, getTypeNodeFromBinding(realMethodBinding.getParameterTypes()[i], ast, imports), null); + } + } // add javadoc tag Javadoc javadoc= methodDecl.getJavadoc(); if (javadoc != null && origReturnType != null && origReturnType.isPrimitiveType() @@ -526,4 +565,43 @@ private void handledInferredParametrizedType(ASTNode node, ASTNode declaringNode } } + private static Type getTypeNodeFromBinding(ITypeBinding typeBinding, AST ast, ImportRewrite importRewrite) { + + if (typeBinding.isWildcardType()) { + WildcardType wildcardType = ast.newWildcardType(); + ITypeBinding bound = typeBinding.getBound(); + if (bound != null) { + Type boundNode = getTypeNodeFromBinding(bound, ast, importRewrite); + wildcardType.setBound(boundNode); + wildcardType.setUpperBound(typeBinding.isUpperbound()); + } + return wildcardType; + } + + if (typeBinding.isArray()) { + Type elementTypeNode = getTypeNodeFromBinding(typeBinding.getElementType(), ast, importRewrite); + return ast.newArrayType(elementTypeNode, typeBinding.getDimensions()); + } + + if (typeBinding.isTypeVariable()) { + return ast.newSimpleType(ast.newSimpleName(typeBinding.getName())); + } + + // import the simple/parameterized type if needed + importRewrite.addImport(typeBinding); + + // create the simple/parameterized + SimpleName simpleName = ast.newSimpleName(typeBinding.getErasure().getName()); + SimpleType simpleType = ast.newSimpleType(simpleName); + if (typeBinding.isParameterizedType()) { + ParameterizedType parameterizedType = ast.newParameterizedType(simpleType); + for (ITypeBinding argument : typeBinding.getTypeArguments()) { + Type typeArgument = getTypeNodeFromBinding(argument, ast, importRewrite); + parameterizedType.typeArguments().add(typeArgument); + } + return parameterizedType; + } + return simpleType; + } + } \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/TypeMismatchQuickFixTests.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/TypeMismatchQuickFixTests.java index 9f6d4760a60..fcf5d3bfec6 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/TypeMismatchQuickFixTests.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/TypeMismatchQuickFixTests.java @@ -1493,27 +1493,43 @@ public void getAnnotation(Class annotationClass) { CompilationUnit astRoot= getASTRoot(cu); ArrayList proposals= collectCorrections(cu, astRoot); - assertNumberOfProposals(proposals, 1); + assertNumberOfProposals(proposals, 2); assertCorrectLabels(proposals); CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); String preview1= getPreviewContent(proposal); String expected1= """ - package test1; - import java.lang.annotation.Annotation; - import java.lang.reflect.AccessibleObject; - public class E { - void m() { - new AccessibleObject() { - public T getAnnotation(Class annotationClass) { - } - }; - } - } - """; + package test1; + import java.lang.annotation.Annotation; + import java.lang.reflect.AccessibleObject; + public class E { + void m() { + new AccessibleObject() { + public Annotation getAnnotation(Class annotationClass) { + } + }; + } + } + """; - assertEqualStringsIgnoreOrder(new String[] { preview1 }, new String[] { expected1 }); + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + String expected2= """ + package test1; + import java.lang.annotation.Annotation; + import java.lang.reflect.AccessibleObject; + public class E { + void m() { + new AccessibleObject() { + public T getAnnotation(Class annotationClass) { + } + }; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 }); } @Test @@ -1536,7 +1552,7 @@ public void getAnnotation(Class annotationClass) { CompilationUnit astRoot= getASTRoot(cu); ArrayList proposals= collectCorrections(cu, astRoot); - assertNumberOfProposals(proposals, 1); + assertNumberOfProposals(proposals, 2); assertCorrectLabels(proposals); CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); @@ -1544,18 +1560,373 @@ public void getAnnotation(Class annotationClass) { String expected1= """ package test1; + import java.lang.annotation.Annotation; import java.lang.reflect.AccessibleObject; public class E { void m() { new AccessibleObject() { - public T getAnnotation(Class annotationClass) { + public Annotation getAnnotation(Class annotationClass) { } }; } } """; - assertEqualStringsIgnoreOrder(new String[] { preview1 }, new String[] { expected1 }); + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + String expected2= """ + package test1; + import java.lang.annotation.Annotation; + import java.lang.reflect.AccessibleObject; + public class E { + void m() { + new AccessibleObject() { + public T getAnnotation(Class annotationClass) { + } + }; + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 }); + } + + @Test + public void testMismatchingReturnTypeOnGenericMethodNestedTypeArgs() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + + String str= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract >> T myMethod(Class asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Class asdf) { + } + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 3); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract >> T myMethod(Class asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + List myMethod(Class asdf) { + } + } + } + """; + + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + String expected2= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract >> T myMethod(Class asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + >> T myMethod(Class asdf) { + } + } + } + """; + + String preview3= getPreviewContent((CUCorrectionProposal)proposals.get(2)); + + String expected3= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract >> void myMethod(Class asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Class asdf) { + } + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3 }, new String[] { expected1, expected2 , expected3}); + } + + @Test + public void testMismatchingReturnTypeOnGenericMethodNestedTypeArgs2() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + + String str= """ + package test1; + import java.util.List; + + public class E { + static interface Interfacable { + void itsMethod(); + } + private abstract static class MyClass { + abstract , ? super Comparable>>> T myMethod(Interfacable, T> asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Interfacable asdf) { + } + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 3); + assertCorrectLabels(proposals); + + CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0); + String preview1= getPreviewContent(proposal); + + String expected1= """ + package test1; + import java.util.List; + + public class E { + static interface Interfacable { + void itsMethod(); + } + private abstract static class MyClass { + abstract , ? super Comparable>>> T myMethod(Interfacable, T> asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + List myMethod(Interfacable asdf) { + } + } + } + """; + + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + String expected2= """ + package test1; + import java.util.List; + + import test1.E.Interfacable; + + public class E { + static interface Interfacable { + void itsMethod(); + } + private abstract static class MyClass { + abstract , ? super Comparable>>> T myMethod(Interfacable, T> asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + , ? super Comparable>>> T myMethod(Interfacable, T> asdf) { + } + } + } + """; + + String preview3= getPreviewContent((CUCorrectionProposal)proposals.get(2)); + + String expected3= """ + package test1; + import java.util.List; + + public class E { + static interface Interfacable { + void itsMethod(); + } + private abstract static class MyClass { + abstract , ? super Comparable>>> void myMethod(Interfacable, T> asdf); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Interfacable asdf) { + } + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3 }, new String[] { expected1, expected2 , expected3 }); + } + + @Test + public void testMismatchingReturnTypeOnGenericMethodNestedTypeArgs3() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + + String str= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract > T myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + > void myMethod(Class clazz) { + + } + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 2); + assertCorrectLabels(proposals); + + String preview1= getPreviewContent((CUCorrectionProposal)proposals.get(0)); + + String expected1= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract > T myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + > T myMethod(Class clazz) { + + } + } + } + """; + + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + String expected2= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract > void myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + > void myMethod(Class clazz) { + + } + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 }); + } + + @Test + public void testMismatchingReturnTypeOnGenericMethodNestedTypeArgs4() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + + String str= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract T myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Class clazz) { + + } + } + } + """; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null); + + CompilationUnit astRoot= getASTRoot(cu); + ArrayList proposals= collectCorrections(cu, astRoot); + assertNumberOfProposals(proposals, 3); + assertCorrectLabels(proposals); + + String preview1= getPreviewContent((CUCorrectionProposal)proposals.get(0)); + + String expected1= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract T myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + Object myMethod(Class clazz) { + + } + } + } + """; + + String preview2= getPreviewContent((CUCorrectionProposal)proposals.get(1)); + + // FIXME: ideally we should use the existing type names + String expected2= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract T myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + T myMethod(Class clazz) { + + } + } + } + """; + + String preview3= getPreviewContent((CUCorrectionProposal)proposals.get(2)); + + String expected3= """ + package test1; + import java.util.List; + + public class E { + private abstract static class MyClass { + abstract void myMethod(Class clazz); + } + private static class MyExtensionClass extends MyClass { + @Override + void myMethod(Class clazz) { + + } + } + } + """; + + assertEqualStringsIgnoreOrder(new String[] { preview1, preview2, preview3 }, new String[] { expected1, expected2, expected3 }); } @Test