Skip to content

Commit

Permalink
Fix "fix return type" quickfix for type param return type
Browse files Browse the repository at this point in the history 
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 eclipse-jdt#1558

Signed-off-by: David Thompson <[email protected]>
  • Loading branch information
@datho7561
datho7561 committed Jul 30, 2024
1 parent 2327a09 commit 9112bda
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 21 deletions.
14 changes: 10 additions & 4 deletions ...tion/common/org/eclipse/jdt/internal/ui/text/correction/TypeMismatchBaseSubProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,19 @@ 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();
}
T p1= createChangeIncompatibleReturnTypeProposal(cu, methodDecl, astRoot, overriddenReturnType, false, IProposalRelevance.CHANGE_RETURN_TYPE);
// propose erasure
T p1= createChangeIncompatibleReturnTypeProposal(cu, methodDecl, astRoot, overriddenReturnType.getErasure(), false, IProposalRelevance.CHANGE_RETURN_TYPE);
if (p1 != null)
proposals.add(p1);

// propose 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);
}
}

ICompilationUnit targetCu= cu;

IMethodBinding overriddenDecl= overridden.getMethodDeclaration();
Expand Down
33 changes: 33 additions & 0 deletions ...g/eclipse/jdt/internal/ui/text/correction/proposals/TypeChangeCorrectionProposalCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,10 +55,13 @@
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;
Expand Down Expand Up @@ -276,6 +280,35 @@ 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 (fNewType.isTypeVariable() && ((List<TypeParameter>)methodDecl.typeParameters()).stream().filter(p -> p.getName().toString().equals(fNewType.getName())).findAny().isEmpty()) {
// add the type variable
ListRewrite typeParameterRewrite = rewrite.getListRewrite(methodDecl, MethodDeclaration.TYPE_PARAMETERS_PROPERTY);
TypeParameter newTypeParameter = ast.newTypeParameter();
SimpleName newTypeParameterName = ast.newSimpleName(fNewType.getName());
newTypeParameter.setName(newTypeParameterName);
// add and import any bounds
Stream.of(fNewType.getTypeBounds()) //
.forEach(bound -> {
imports.addImport(bound, ast, context, fTypeLocation);
// TODO: I think there is something about nested bounds or something
// Maybe it's that they can have type args?
SimpleType boundType = ast.newSimpleType(ast.newSimpleName(bound.getName()));
newTypeParameter.typeBounds().add(boundType);
});
typeParameterRewrite.insertLast(newTypeParameter, null);

// update any raw types in the parameters that only have one type variable to use the type variable
for (int i = 0 ; i < methodDecl.parameters().size(); i++) {
SingleVariableDeclaration svd = (SingleVariableDeclaration)methodDecl.parameters().get(i);
ITypeBinding paramBinding = svd.getType().resolveBinding();
if (paramBinding.isRawType() && paramBinding.getTypeDeclaration().getTypeParameters().length == 1) {
Type origTypeCopy = (Type) rewrite.createCopyTarget(svd.getType());
ParameterizedType newParamWithArg = ast.newParameterizedType(origTypeCopy);
newParamWithArg.typeArguments().add(type);
rewrite.set(svd, SingleVariableDeclaration.TYPE_PROPERTY, newParamWithArg, null);
}
}
}
// add javadoc tag
Javadoc javadoc= methodDecl.getJavadoc();
if (javadoc != null && origReturnType != null && origReturnType.isPrimitiveType()
Expand Down
67 changes: 50 additions & 17 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/TypeMismatchQuickFixTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1493,27 +1493,43 @@ public <T extends Annotation> void getAnnotation(Class<T> annotationClass) {

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> 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 extends Annotation> T getAnnotation(Class<T> annotationClass) {
}
};
}
}
""";
package test1;
import java.lang.annotation.Annotation;
import java.lang.reflect.AccessibleObject;
public class E {
void m() {
new AccessibleObject() {
public <T extends Annotation> Annotation getAnnotation(Class<T> 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 extends Annotation> T getAnnotation(Class<T> annotationClass) {
}
};
}
}
""";

assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 });
}

@Test
Expand All @@ -1536,26 +1552,43 @@ public void getAnnotation(Class annotationClass) {

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> 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) {
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 extends Annotation> T getAnnotation(Class<T> annotationClass) {
}
};
}
}
""";

assertEqualStringsIgnoreOrder(new String[] { preview1, preview2 }, new String[] { expected1, expected2 });
}

@Test
Expand Down

0 comments on commit 9112bda

Please sign in to comment.