Skip to content

Commit

Permalink
Fix message for quickfixes for "wrong number of args" in generic cons…
Browse files Browse the repository at this point in the history 
…tructor

- Add `parentType` to JavacMethodBinding
  - Currently only used when requesting the method binding through
    `typeBinding.getDeclaredMethods`
- Fix implementation of `JavacTypeBinding.getName` for wildcard types
- In `JavacMethodBinding.getKey()`, prefer using the return type from `methodType` to the return type from `methodSymbol`
- Use `JavacMethodBinding.parentType` in `getKey()`
  - Note that the erasure is intentionally not used.

Signed-off-by: David Thompson <davthomp@redhat.com>
  • Loading branch information
@datho7561
datho7561 committed Aug 12, 2024
1 parent cf28e47 commit e6abd91
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 47 deletions.
40 changes: 20 additions & 20 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacBindingResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public BindingKeyException(String message, Throwable cause) {
super(message, cause);
}
}

public class Bindings {
private Map<String, JavacAnnotationBinding> annotationBindings = new HashMap<>();
public JavacAnnotationBinding getAnnotationBinding(Compound ann, IBinding recipient) {
Expand All @@ -115,7 +115,7 @@ public JavacAnnotationBinding getAnnotationBinding(Compound ann, IBinding recipi
if( k != null ) {
annotationBindings.putIfAbsent(k, newInstance);
return annotationBindings.get(k);
}
}
return null;
}
//
Expand All @@ -131,8 +131,8 @@ public JavacMemberValuePairBinding getMemberValuePairBinding(MethodSymbol key, A
}
//
private Map<String, JavacMethodBinding> methodBindings = new HashMap<>();
public JavacMethodBinding getMethodBinding(MethodType methodType, MethodSymbol methodSymbol) {
JavacMethodBinding newInstance = new JavacMethodBinding(methodType, methodSymbol, JavacBindingResolver.this) { };
public JavacMethodBinding getMethodBinding(MethodType methodType, MethodSymbol methodSymbol, com.sun.tools.javac.code.Type parentType) {
JavacMethodBinding newInstance = new JavacMethodBinding(methodType, methodSymbol, parentType, JavacBindingResolver.this) { };
String k = newInstance.getKey();
if( k != null ) {
methodBindings.putIfAbsent(k, newInstance);
Expand Down Expand Up @@ -283,7 +283,7 @@ public IBinding getBinding(final Symbol owner, final com.sun.tools.javac.code.Ty
} else if (owner instanceof TypeSymbol typeSymbol) {
return getTypeBinding(typeSymbol.type);
} else if (owner instanceof final MethodSymbol other) {
return getMethodBinding(type instanceof com.sun.tools.javac.code.Type.MethodType methodType ? methodType : owner.type.asMethodType(), other);
return getMethodBinding(type instanceof com.sun.tools.javac.code.Type.MethodType methodType ? methodType : owner.type.asMethodType(), other, null);
} else if (owner instanceof final VarSymbol other) {
return getVariableBinding(other);
}
Expand Down Expand Up @@ -584,7 +584,7 @@ IMethodBinding resolveMethod(MethodInvocation method) {
if (type != null &&
type.tsym.members().findFirst(ident.getName(), MethodSymbol.class::isInstance) instanceof MethodSymbol methodSymbol &&
methodSymbol.type instanceof MethodType methodType) {
var res = this.bindings.getMethodBinding(methodType, methodSymbol);
var res = this.bindings.getMethodBinding(methodType, methodSymbol, null);
if (res != null) {
return res;
}
Expand All @@ -594,13 +594,13 @@ IMethodBinding resolveMethod(MethodInvocation method) {
javacElement instanceof JCFieldAccess fieldAccess ? fieldAccess.sym :
null;
if (type instanceof MethodType methodType && sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(methodType, methodSymbol);
return this.bindings.getMethodBinding(methodType, methodSymbol, null);
}
if (type instanceof ErrorType errorType && errorType.getOriginalType() instanceof MethodType methodType) {
if (sym.owner instanceof TypeSymbol typeSymbol) {
Iterator<Symbol> methods = typeSymbol.members().getSymbolsByName(sym.getSimpleName(), m -> m instanceof MethodSymbol && methodType.equals(m.type)).iterator();
if (methods.hasNext()) {
return this.bindings.getMethodBinding(methodType, (MethodSymbol)methods.next());
return this.bindings.getMethodBinding(methodType, (MethodSymbol)methods.next(), null);
}
}
return this.bindings.getErrorMethodBinding(methodType, sym);
Expand All @@ -613,7 +613,7 @@ IMethodBinding resolveMethod(MethodDeclaration method) {
resolve();
JCTree javacElement = this.converter.domToJavac.get(method);
if (javacElement instanceof JCMethodDecl methodDecl && methodDecl.type != null) {
return this.bindings.getMethodBinding(methodDecl.type.asMethodType(), methodDecl.sym);
return this.bindings.getMethodBinding(methodDecl.type.asMethodType(), methodDecl.sym, null);
}
return null;
}
Expand All @@ -636,7 +636,7 @@ IMethodBinding resolveMethod(MethodReference methodReference) {
resolve();
JCTree javacElement = this.converter.domToJavac.get(methodReference);
if (javacElement instanceof JCMemberReference memberRef && memberRef.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(memberRef.referentType.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(memberRef.referentType.asMethodType(), methodSymbol, null);
}
return null;
}
Expand All @@ -646,7 +646,7 @@ IMethodBinding resolveMember(AnnotationTypeMemberDeclaration member) {
resolve();
JCTree javacElement = this.converter.domToJavac.get(member);
if (javacElement instanceof JCMethodDecl methodDecl) {
return this.bindings.getMethodBinding(methodDecl.type.asMethodType(), methodDecl.sym);
return this.bindings.getMethodBinding(methodDecl.type.asMethodType(), methodDecl.sym, null);
}
return null;
}
Expand All @@ -660,7 +660,7 @@ IMethodBinding resolveConstructor(EnumConstantDeclaration enumConstantDeclaratio
}
return javacElement instanceof JCNewClass jcExpr
&& !jcExpr.constructor.type.isErroneous()?
this.bindings.getMethodBinding(jcExpr.constructor.type.asMethodType(), (MethodSymbol)jcExpr.constructor) :
this.bindings.getMethodBinding(jcExpr.constructor.type.asMethodType(), (MethodSymbol)jcExpr.constructor, null) :
null;
}

Expand All @@ -672,10 +672,10 @@ IMethodBinding resolveConstructor(SuperConstructorInvocation expression) {
javacElement = javacMethodInvocation.getMethodSelect();
}
if (javacElement instanceof JCIdent ident && ident.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(ident.type != null ? ident.type.asMethodType() : methodSymbol.asType().asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(ident.type != null ? ident.type.asMethodType() : methodSymbol.asType().asMethodType(), methodSymbol, null);
}
if (javacElement instanceof JCFieldAccess fieldAccess && fieldAccess.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol, null);
}
return null;
}
Expand All @@ -688,11 +688,11 @@ IMethodBinding resolveMethod(SuperMethodInvocation method) {
javacElement = javacMethodInvocation.getMethodSelect();
}
if (javacElement instanceof JCIdent ident && ident.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(ident.type.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(ident.type.asMethodType(), methodSymbol, null);
}
if (javacElement instanceof JCFieldAccess fieldAccess && fieldAccess.sym instanceof MethodSymbol methodSymbol
&& fieldAccess.type != null /* when there are syntax errors */) {
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol, null);
}
return null;
}
Expand Down Expand Up @@ -769,7 +769,7 @@ IBinding resolveNameToJavac(Name name, JCTree tree) {
}
}
}

if (tree instanceof JCIdent ident && ident.sym != null) {
if (ident.type instanceof ErrorType errorType
&& errorType.getOriginalType() instanceof ErrorType) {
Expand Down Expand Up @@ -921,7 +921,7 @@ private IMethodBinding resolveConstructorImpl(ClassInstanceCreation expression)
return this.converter.domToJavac.get(expression) instanceof JCNewClass jcExpr
&& jcExpr.constructor != null
&& !jcExpr.constructor.type.isErroneous()?
this.bindings.getMethodBinding(jcExpr.constructor.type.asMethodType(), (MethodSymbol)jcExpr.constructor) :
this.bindings.getMethodBinding(jcExpr.constructor.type.asMethodType(), (MethodSymbol)jcExpr.constructor, null) :
null;
}

Expand All @@ -937,10 +937,10 @@ private IMethodBinding resolveConstructorImpl(ConstructorInvocation invocation)
javacElement = javacMethodInvocation.getMethodSelect();
}
if (javacElement instanceof JCIdent ident && ident.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(ident.type != null ? ident.type.asMethodType() : methodSymbol.type.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(ident.type != null ? ident.type.asMethodType() : methodSymbol.type.asMethodType(), methodSymbol, null);
}
if (javacElement instanceof JCFieldAccess fieldAccess && fieldAccess.sym instanceof MethodSymbol methodSymbol) {
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol);
return this.bindings.getMethodBinding(fieldAccess.type.asMethodType(), methodSymbol, null);
}
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion ...clipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacErrorMethodBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public abstract class JavacErrorMethodBinding extends JavacMethodBinding {
private Symbol originatingSymbol;

public JavacErrorMethodBinding(Symbol originatingSymbol, MethodType methodType, JavacBindingResolver resolver) {
super(methodType, null, resolver);
super(methodType, null, null, resolver);
this.originatingSymbol = originatingSymbol;
}

Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacLambdaBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
public class JavacLambdaBinding extends JavacMethodBinding {

public JavacLambdaBinding(JavacMethodBinding methodBinding) {
super(methodBinding.methodType, methodBinding.methodSymbol, methodBinding.resolver);
super(methodBinding.methodType, methodBinding.methodSymbol, methodBinding.parentType, methodBinding.resolver);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...se.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacMemberValuePairBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract class JavacMemberValuePairBinding implements IMemberValuePairBin
private final JavacBindingResolver resolver;

public JavacMemberValuePairBinding(MethodSymbol key, Attribute value, JavacBindingResolver resolver) {
this.method = resolver.bindings.getMethodBinding(key.type.asMethodType(), key);
this.method = resolver.bindings.getMethodBinding(key.type.asMethodType(), key, null);
this.value = value;
this.resolver = resolver;
}
Expand Down
44 changes: 31 additions & 13 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacMethodBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,20 @@ public abstract class JavacMethodBinding implements IMethodBinding {

public final MethodSymbol methodSymbol;
final MethodType methodType;
final Type parentType;
final JavacBindingResolver resolver;

public JavacMethodBinding(MethodType methodType, MethodSymbol methodSymbol, JavacBindingResolver resolver) {
/**
*
* @param methodType
* @param methodSymbol
* @param parentType can be null, in which case <code>methodSymbol.owner.type</code> will be used instead
* @param resolver
*/
public JavacMethodBinding(MethodType methodType, MethodSymbol methodSymbol, Type parentType, JavacBindingResolver resolver) {
this.methodType = methodType;
this.methodSymbol = methodSymbol;
this.parentType = parentType;
this.resolver = resolver;
}

Expand Down Expand Up @@ -257,22 +266,26 @@ private String resolveTypeName(com.sun.tools.javac.code.Type type, boolean binar
public String getKey() {
try {
StringBuilder builder = new StringBuilder();
getKey(builder, this.methodSymbol, this.methodType, this.resolver);
getKey(builder, this.methodSymbol, this.methodType, this.parentType, this.resolver);
return builder.toString();
} catch(BindingKeyException bke) {
return null;
}
}

static void getKey(StringBuilder builder, MethodSymbol methodSymbol, MethodType methodType, JavacBindingResolver resolver) throws BindingKeyException {
Symbol ownerSymbol = methodSymbol.owner;
while (ownerSymbol != null && !(ownerSymbol instanceof TypeSymbol)) {
ownerSymbol = ownerSymbol.owner;
}
if (ownerSymbol instanceof TypeSymbol ownerTypeSymbol) {
JavacTypeBinding.getKey(builder, resolver.getTypes().erasure(ownerTypeSymbol.type), false);
static void getKey(StringBuilder builder, MethodSymbol methodSymbol, MethodType methodType, Type parentType, JavacBindingResolver resolver) throws BindingKeyException {
if (parentType != null) {
JavacTypeBinding.getKey(builder, parentType, false);
} else {
throw new BindingKeyException(new IllegalArgumentException("Method has no owning class"));
Symbol ownerSymbol = methodSymbol.owner;
while (ownerSymbol != null && !(ownerSymbol instanceof TypeSymbol)) {
ownerSymbol = ownerSymbol.owner;
}
if (ownerSymbol instanceof TypeSymbol ownerTypeSymbol) {
JavacTypeBinding.getKey(builder, resolver.getTypes().erasure(ownerTypeSymbol.type), false);
} else {
throw new BindingKeyException(new IllegalArgumentException("Method has no owning class"));
}
}
builder.append('.');
if (!methodSymbol.isConstructor()) {
Expand Down Expand Up @@ -303,7 +316,9 @@ static void getKey(StringBuilder builder, MethodSymbol methodSymbol, MethodType
}
}
builder.append(')');
if (!(methodSymbol.getReturnType() instanceof JCNoType)) {
if (methodType != null && !(methodType.getReturnType() instanceof JCNoType)) {
JavacTypeBinding.getKey(builder, methodType.getReturnType(), false);
} else if (!(methodSymbol.getReturnType() instanceof JCNoType)) {
JavacTypeBinding.getKey(builder, methodSymbol.getReturnType(), false);
}
if (
Expand Down Expand Up @@ -355,6 +370,9 @@ public String getName() {

@Override
public ITypeBinding getDeclaringClass() {
if (this.parentType != null) {
return this.resolver.bindings.getTypeBinding(this.parentType);
}
// probably incorrect as it may not return the actual declaring type, see getJavaElement()
Symbol parentSymbol = this.methodSymbol.owner;
do {
Expand All @@ -372,7 +390,7 @@ public IBinding getDeclaringMember() {
return null;
}
if (this.methodSymbol.owner instanceof MethodSymbol methodSymbol) {
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol);
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol, null);
} else if (this.methodSymbol.owner instanceof VarSymbol variableSymbol) {
return this.resolver.bindings.getVariableBinding(variableSymbol);
}
Expand Down Expand Up @@ -498,7 +516,7 @@ public IMethodBinding getMethodDeclaration() {
// This method intentionally converts the type to its generic type,
// i.e. drops the type arguments
// i.e. <code>this.<String>getValue(12);</code> will be converted back to <code><T> T getValue(int i) {</code>
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol);
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol, null);
}

@Override
Expand Down
31 changes: 22 additions & 9 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacTypeBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public IJavaElement getJavaElement() {
return ownerType.getTypeParameter(this.getName());
} else if (this.typeSymbol.owner instanceof MethodSymbol ownerSymbol
&& ownerSymbol.type != null
&& this.resolver.bindings.getMethodBinding(ownerSymbol.type.asMethodType(), ownerSymbol).getJavaElement() instanceof IMethod ownerMethod
&& this.resolver.bindings.getMethodBinding(ownerSymbol.type.asMethodType(), ownerSymbol, null).getJavaElement() instanceof IMethod ownerMethod
&& ownerMethod.getTypeParameter(this.getName()) != null) {
return ownerMethod.getTypeParameter(this.getName());
}
Expand All @@ -182,7 +182,7 @@ public IJavaElement getJavaElement() {
return type.getType("", 1);
}
}

JavaFileObject jfo = classSymbol == null ? null : classSymbol.sourcefile;
ICompilationUnit tmp = jfo == null ? null : getCompilationUnit(jfo.getName().toCharArray(), this.resolver.getWorkingCopyOwner());
if( tmp != null ) {
Expand All @@ -197,9 +197,9 @@ public IJavaElement getJavaElement() {
if( ret == null )
done = true;
}
if( ret != null )
if( ret != null )
return ret;
}
}
try {
IType ret = this.resolver.javaProject.findType(cleanedUpName(this.type), this.resolver.getWorkingCopyOwner(), new NullProgressMonitor());
return ret;
Expand Down Expand Up @@ -446,7 +446,7 @@ public IMethodBinding[] getDeclaredMethods() {
.map(MethodSymbol.class::cast)
.map(sym -> {
Type.MethodType methodType = this.types.memberType(this.type, sym).asMethodType();
return this.resolver.bindings.getMethodBinding(methodType, sym);
return this.resolver.bindings.getMethodBinding(methodType, sym, this.type);
})
.filter(Objects::nonNull)
.toArray(IMethodBinding[]::new);
Expand Down Expand Up @@ -490,10 +490,10 @@ public IMethodBinding getDeclaringMethod() {
do {
if (parentSymbol instanceof final MethodSymbol method) {
if (method.type instanceof Type.MethodType methodType) {
return this.resolver.bindings.getMethodBinding(methodType, method);
return this.resolver.bindings.getMethodBinding(methodType, method, null);
}
if( method.type instanceof Type.ForAll faType && faType.qtype instanceof MethodType mtt) {
IMethodBinding found = this.resolver.bindings.getMethodBinding(mtt, method);
IMethodBinding found = this.resolver.bindings.getMethodBinding(mtt, method, null);
return found;
}
return null;
Expand Down Expand Up @@ -538,7 +538,7 @@ public IMethodBinding getFunctionalInterfaceMethod() {
try {
Symbol symbol = types.findDescriptorSymbol(this.typeSymbol);
if (symbol instanceof MethodSymbol methodSymbol) {
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol);
return this.resolver.bindings.getMethodBinding(methodSymbol.type.asMethodType(), methodSymbol, null);
}
} catch (FunctionDescriptorLookupError ignore) {
}
Expand Down Expand Up @@ -585,6 +585,19 @@ public String getName() {
}
return builder.toString();
}
if (type instanceof WildcardType wt) {
if (wt.type == null || this.resolver.resolveWellKnownType("java.lang.Object").equals(this.resolver.bindings.getTypeBinding(wt.type))) {
return "?";
}
StringBuilder builder = new StringBuilder("? ");
if (wt.isExtendsBound()) {
builder.append("extends ");
} else if (wt.isSuperBound()) {
builder.append("super ");
}
builder.append(this.resolver.bindings.getTypeBinding(wt.type).getName());
return builder.toString();
}
StringBuilder builder = new StringBuilder(this.typeSymbol.getSimpleName().toString());
ITypeBinding[] types = this.getUncheckedTypeArguments(this.type, this.typeSymbol);

Expand Down Expand Up @@ -739,7 +752,7 @@ public IAnnotationBinding[] getTypeAnnotations() {
public ITypeBinding[] getTypeArguments() {
return getTypeArguments(this.type, this.typeSymbol);
}

private ITypeBinding[] getTypeArguments(Type t, TypeSymbol ts) {
if (t == ts.type || t.getTypeArguments().isEmpty() || isTargettingPreGenerics()) {
return NO_TYPE_ARGUMENTS;
Expand Down
4 changes: 2 additions & 2 deletions org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacVariableBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private String getKeyImpl() throws BindingKeyException {
}
return builder.toString();
} else if (this.variableSymbol.owner instanceof MethodSymbol methodSymbol) {
JavacMethodBinding.getKey(builder, methodSymbol, methodSymbol.type instanceof Type.MethodType methodType ? methodType : null, this.resolver);
JavacMethodBinding.getKey(builder, methodSymbol, methodSymbol.type instanceof Type.MethodType methodType ? methodType : null, null, this.resolver);
builder.append('#');
builder.append(this.variableSymbol.name);
// FIXME: is it possible for the javac AST to contain multiple definitions of the same variable?
Expand Down Expand Up @@ -241,7 +241,7 @@ public IMethodBinding getDeclaringMethod() {
if (!(method.type instanceof Type.MethodType methodType)) {
return null;
}
return this.resolver.bindings.getMethodBinding(methodType, method);
return this.resolver.bindings.getMethodBinding(methodType, method, null);
}
parentSymbol = parentSymbol.owner;
} while (parentSymbol != null);
Expand Down

0 comments on commit e6abd91

Please sign in to comment.