Skip to content

Commit 2dfd51f

Browse files
datho7561rgrunber
datho7561
authored and
rgrunber
committed
Improve "override method" completion
- Handle qualifying types referenced in the method declaration - eg. do not qualify imported types - eg. do not qualify types from the same package - eg. do not qualify inner classes nor inherited inner classes - Fix parameter names - get them from the `IMethod`, which has more accurate - Handle completion in anonymous classes - Do not suggest overriding a method that's already overriden Signed-off-by: David Thompson <[email protected]>
1 parent 23ffa6a commit 2dfd51f

File tree

3 files changed

+138
-31
lines changed

3 files changed

+138
-31
lines changed

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/DOMCompletionEngine.java‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import org.eclipse.jdt.core.dom.AnonymousClassDeclaration;
7373
import org.eclipse.jdt.core.dom.Assignment;
7474
import org.eclipse.jdt.core.dom.Block;
75+
import org.eclipse.jdt.core.dom.BodyDeclaration;
7576
import org.eclipse.jdt.core.dom.CatchClause;
7677
import org.eclipse.jdt.core.dom.ChildListPropertyDescriptor;
7778
import org.eclipse.jdt.core.dom.ChildPropertyDescriptor;
@@ -790,17 +791,25 @@ public void complete(org.eclipse.jdt.internal.compiler.env.ICompilationUnit sour
790791
}
791792
if (context instanceof SimpleName) {
792793
if (context.getParent() instanceof SimpleType simpleType
793-
&& simpleType.getParent() instanceof FieldDeclaration fieldDeclaration
794-
&& fieldDeclaration.getParent() instanceof AbstractTypeDeclaration typeDecl) {
794+
&& (simpleType.getParent() instanceof FieldDeclaration || (simpleType.getParent() instanceof MethodDeclaration methodDecl && methodDecl.getReturnType2() == simpleType))
795+
&& (simpleType.getParent().getParent() instanceof AbstractTypeDeclaration || simpleType.getParent().getParent() instanceof AnonymousClassDeclaration)) {
795796
// eg.
796797
// public class Foo {
797798
// ba|
798799
// }
799-
ITypeBinding typeDeclBinding = typeDecl.resolveBinding();
800+
BodyDeclaration bodyDeclaration = (BodyDeclaration)simpleType.getParent();
801+
802+
ITypeBinding typeDeclBinding;
803+
if (simpleType.getParent().getParent() instanceof AbstractTypeDeclaration typeDecl) {
804+
typeDeclBinding = typeDecl.resolveBinding();
805+
} else {
806+
typeDeclBinding = ((AnonymousClassDeclaration)simpleType.getParent().getParent()).resolveBinding();
807+
}
808+
800809
findOverridableMethods(typeDeclBinding, this.javaProject, context);
801810
suggestClassDeclarationLikeKeywords();
802811
suggestTypeKeywords(true);
803-
suggestModifierKeywords(fieldDeclaration.getModifiers());
812+
suggestModifierKeywords(bodyDeclaration.getModifiers());
804813
if (!this.requestor.isIgnored(CompletionProposal.TYPE_REF)) {
805814
findTypes(this.prefix, IJavaSearchConstants.TYPE, null)
806815
// don't care about annotations
@@ -3428,18 +3437,29 @@ private void completeConstructor(ITypeBinding typeBinding, ASTNode referencedFro
34283437

34293438
private void findOverridableMethods(ITypeBinding typeBinding, IJavaProject javaProject, ASTNode toReplace) {
34303439
String originalPackageKey = typeBinding.getPackage().getKey();
3440+
3441+
String currentPackageName = this.unit.getPackage() != null ? this.unit.getPackage().getName().toString() : "";
3442+
List<ITypeBinding> importedTypes = this.unit.imports().stream() //
3443+
.map(importDecl -> ((ImportDeclaration)importDecl).resolveBinding()) //
3444+
.filter(ITypeBinding.class::isInstance).map(ITypeBinding.class::cast) //
3445+
.toList();
34313446
Set<String> alreadySuggestedMethodKeys = new HashSet<>();
3447+
for (IMethodBinding declaredMethod : typeBinding.getDeclaredMethods()) {
3448+
alreadySuggestedMethodKeys.add(KeyUtils.getMethodKeyWithOwnerTypeAndReturnTypeRemoved(declaredMethod.getKey()));
3449+
}
34323450
if (typeBinding.getSuperclass() != null) {
3433-
findOverridableMethods0(typeBinding.getSuperclass(), alreadySuggestedMethodKeys, javaProject, originalPackageKey, toReplace);
3451+
findOverridableMethods0(typeBinding, typeBinding.getSuperclass(), alreadySuggestedMethodKeys, javaProject, originalPackageKey, currentPackageName, importedTypes, toReplace);
34343452
}
34353453
for (ITypeBinding superInterface : typeBinding.getInterfaces()) {
3436-
findOverridableMethods0(superInterface, alreadySuggestedMethodKeys, javaProject, originalPackageKey, toReplace);
3454+
findOverridableMethods0(typeBinding, superInterface, alreadySuggestedMethodKeys, javaProject, originalPackageKey, currentPackageName, importedTypes, toReplace);
34373455
}
34383456
}
34393457

3440-
private void findOverridableMethods0(ITypeBinding typeBinding, Set<String> alreadySuggestedKeys, IJavaProject javaProject, String originalPackageKey, ASTNode toReplace) {
3458+
private void findOverridableMethods0(ITypeBinding currentType, ITypeBinding typeBinding, Set<String> alreadySuggestedKeys, IJavaProject javaProject, String originalPackageKey,
3459+
String currentPackageName, List<ITypeBinding> importedTypes, ASTNode toReplace) {
34413460
next : for (IMethodBinding method : typeBinding.getDeclaredMethods()) {
3442-
if (alreadySuggestedKeys.contains(method.getKey())) {
3461+
String cleanedMethodKey = KeyUtils.getMethodKeyWithOwnerTypeAndReturnTypeRemoved(method.getKey());
3462+
if (alreadySuggestedKeys.contains(cleanedMethodKey)) {
34433463
continue next;
34443464
}
34453465
if (method.isSynthetic() || method.isConstructor()
@@ -3449,7 +3469,7 @@ private void findOverridableMethods0(ITypeBinding typeBinding, Set<String> alrea
34493469
|| ((method.getModifiers() & (Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED)) == 0) && !typeBinding.getPackage().getKey().equals(originalPackageKey)) {
34503470
continue next;
34513471
}
3452-
alreadySuggestedKeys.add(method.getKey());
3472+
alreadySuggestedKeys.add(cleanedMethodKey);
34533473
if (Modifier.isFinal(method.getModifiers())) {
34543474
continue next;
34553475
}
@@ -3469,26 +3489,32 @@ private void findOverridableMethods0(ITypeBinding typeBinding, Set<String> alrea
34693489
proposal.setDeclarationSignature(SignatureUtils.getSignatureChar(method.getDeclaringClass()));
34703490
proposal.setKey(method.getKey().toCharArray());
34713491
proposal.setSignature(SignatureUtils.getSignatureChar(method));
3472-
proposal.setParameterNames(Stream.of(method.getParameterNames()).map(name -> name.toCharArray()).toArray(char[][]::new));
3492+
3493+
try {
3494+
proposal.setParameterNames(Stream.of(((IMethod)method.getJavaElement()).getParameterNames()).map(name -> name.toCharArray()).toArray(char[][]::new));
3495+
} catch (JavaModelException e) {
3496+
proposal.setParameterNames(Stream.of(method.getParameterNames()).map(name -> name.toCharArray()).toArray(char[][]::new));
3497+
}
34733498

34743499
int relevance = RelevanceConstants.R_DEFAULT
34753500
+ RelevanceConstants.R_RESOLVED
34763501
+ RelevanceConstants.R_INTERESTING
34773502
+ RelevanceConstants.R_METHOD_OVERIDE
34783503
+ (Modifier.isAbstract(method.getModifiers()) ? RelevanceConstants.R_ABSTRACT_METHOD : 0)
3479-
+ RelevanceConstants.R_NON_RESTRICTED;
3504+
+ RelevanceConstants.R_NON_RESTRICTED
3505+
+ RelevanceUtils.computeRelevanceForCaseMatching(this.prefix.toCharArray(), method.getName().toCharArray(), this.assistOptions);
34803506
proposal.setRelevance(relevance);
34813507

34823508
StringBuilder completion = new StringBuilder();
3483-
DOMCompletionEngineBuilder.createMethod(method, completion);
3509+
DOMCompletionEngineBuilder.createMethod(method, completion, currentType, importedTypes, currentPackageName);
34843510
proposal.setCompletion(completion.toString().toCharArray());
34853511
this.requestor.accept(proposal);
34863512
}
34873513
if (typeBinding.getSuperclass() != null) {
3488-
findOverridableMethods0(typeBinding.getSuperclass(), alreadySuggestedKeys, javaProject, originalPackageKey, toReplace);
3514+
findOverridableMethods0(currentType, typeBinding.getSuperclass(), alreadySuggestedKeys, javaProject, originalPackageKey, currentPackageName, importedTypes, toReplace);
34893515
}
34903516
for (ITypeBinding superInterface : typeBinding.getInterfaces()) {
3491-
findOverridableMethods0(superInterface, alreadySuggestedKeys, javaProject, originalPackageKey, toReplace);
3517+
findOverridableMethods0(currentType, superInterface, alreadySuggestedKeys, javaProject, originalPackageKey, currentPackageName, importedTypes, toReplace);
34923518
}
34933519
}
34943520

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/DOMCompletionEngineBuilder.java‎

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
*******************************************************************************/
1414
package org.eclipse.jdt.internal.codeassist;
1515

16+
import java.util.List;
17+
18+
import org.eclipse.jdt.core.Flags;
19+
import org.eclipse.jdt.core.IMethod;
20+
import org.eclipse.jdt.core.JavaModelException;
1621
import org.eclipse.jdt.core.dom.IMethodBinding;
1722
import org.eclipse.jdt.core.dom.ITypeBinding;
1823
import org.eclipse.jdt.internal.compiler.ast.ASTNode;
@@ -26,8 +31,10 @@ class DOMCompletionEngineBuilder {
2631
private static final String EXTENDS = "extends"; //$NON-NLS-1$
2732
private static final String THROWS = "throws"; //$NON-NLS-1$
2833
private static final String SUPER = "super"; //$NON-NLS-1$
34+
35+
private static final String JAVA_LANG_PKG = "java.lang.";
2936

30-
static void createMethod(IMethodBinding methodBinding, StringBuilder completion) {
37+
static void createMethod(IMethodBinding methodBinding, StringBuilder completion, ITypeBinding currentType, List<ITypeBinding> importedTypes, String currentPackage) {
3138

3239
// Modifiers
3340
// flush uninteresting modifiers
@@ -47,14 +54,14 @@ static void createMethod(IMethodBinding methodBinding, StringBuilder completion)
4754
completion.append(',');
4855
completion.append(' ');
4956
}
50-
createTypeVariable(typeVariableBindings[i], completion);
57+
createTypeVariable(typeVariableBindings[i], completion, currentType, importedTypes, currentPackage);
5158
}
5259
completion.append('>');
5360
completion.append(' ');
5461
}
5562

5663
// Return type
57-
createType(methodBinding.getReturnType(), completion);
64+
createType(methodBinding.getReturnType(), completion, currentType, importedTypes, currentPackage);
5865
completion.append(' ');
5966

6067
// Selector (name)
@@ -64,14 +71,19 @@ static void createMethod(IMethodBinding methodBinding, StringBuilder completion)
6471

6572
// Parameters
6673
ITypeBinding[] parameterTypes = methodBinding.getParameterTypes();
67-
String[] parameterNames = methodBinding.getParameterNames();
74+
String[] parameterNames;
75+
try {
76+
parameterNames = ((IMethod)methodBinding.getJavaElement()).getParameterNames();
77+
} catch (JavaModelException e) {
78+
parameterNames = methodBinding.getParameterNames();
79+
}
6880
int length = parameterTypes.length;
6981
for (int i = 0; i < length; i++) {
7082
if (i != 0) {
7183
completion.append(',');
7284
completion.append(' ');
7385
}
74-
createType(parameterTypes[i], completion);
86+
createType(parameterTypes[i], completion, currentType, importedTypes, currentPackage);
7587
completion.append(' ');
7688
if (parameterNames != null) {
7789
completion.append(parameterNames[i]);
@@ -94,42 +106,42 @@ static void createMethod(IMethodBinding methodBinding, StringBuilder completion)
94106
completion.append(' ');
95107
completion.append(',');
96108
}
97-
createType(exceptions[i], completion);
109+
createType(exceptions[i], completion, currentType, importedTypes, currentPackage);
98110
}
99111
}
100112
}
101113

102-
static void createType(ITypeBinding type, StringBuilder completion) {
114+
static void createType(ITypeBinding type, StringBuilder completion, ITypeBinding currentType, List<ITypeBinding> importedTypes, String currentPackage) {
103115
if (type.isWildcardType() || type.isIntersectionType()) {
104116
completion.append('?');
105117
if (type.isUpperbound() && type.getBound() != null) {
106118
completion.append(' ');
107119
completion.append(EXTENDS);
108120
completion.append(' ');
109-
createType(type.getBound(), completion);
121+
createType(type.getBound(), completion, currentType, importedTypes, currentPackage);
110122
if (type.getTypeBounds() != null) {
111123
for (ITypeBinding bound : type.getTypeBounds()) {
112124
completion.append(' ');
113125
completion.append('&');
114126
completion.append(' ');
115-
createType(bound, completion);
127+
createType(bound, completion, currentType, importedTypes, currentPackage);
116128
}
117129
}
118130
} else if (type.getBound() != null) {
119131
completion.append(' ');
120132
completion.append(SUPER);
121133
completion.append(' ');
122-
createType(type.getBound(), completion);
134+
createType(type.getBound(), completion, currentType, importedTypes, currentPackage);
123135
}
124136
} else if (type.isArray()) {
125-
createType(type.getElementType(), completion);
137+
createType(type.getElementType(), completion, currentType, importedTypes, currentPackage);
126138
int dim = type.getDimensions();
127139
for (int i = 0; i < dim; i++) {
128140
completion.append("[]"); //$NON-NLS-1$
129141
}
130142
} else if (type.isParameterizedType()) {
131143
if (type.isMember()) {
132-
createType(type.getDeclaringClass(), completion);
144+
createType(type.getDeclaringClass(), completion, currentType, importedTypes, currentPackage);
133145
completion.append('.');
134146
completion.append(type.getName());
135147
} else {
@@ -141,24 +153,46 @@ static void createType(ITypeBinding type, StringBuilder completion) {
141153
for (int i = 0, length = typeArguments.length; i < length; i++) {
142154
if (i != 0)
143155
completion.append(',');
144-
createType(typeArguments[i], completion);
156+
createType(typeArguments[i], completion, currentType, importedTypes, currentPackage);
145157
}
146158
completion.append('>');
147159
}
148160
} else {
149-
completion.append(type.getQualifiedName());
161+
boolean appended = false;
162+
for (ITypeBinding importedType : importedTypes) {
163+
if (importedType.getKey().equals(type.getErasure().getKey())) {
164+
completion.append(type.getName());
165+
appended = true;
166+
break;
167+
}
168+
}
169+
ITypeBinding firstMatchingInheritedMemberType = getFirstMatchingInheritedMemberType(currentType, type.getName());
170+
if (firstMatchingInheritedMemberType != null && firstMatchingInheritedMemberType.getKey().equals(type.getKey())) {
171+
completion.append(type.getName());
172+
appended = true;
173+
}
174+
if (!appended) {
175+
String qualifiedName = type.getQualifiedName();
176+
String packageName = type.getPackage() != null ? type.getPackage().getName() : "";
177+
if (qualifiedName.startsWith(JAVA_LANG_PKG)) {
178+
qualifiedName = qualifiedName.substring(JAVA_LANG_PKG.length());
179+
} else if (!packageName.isEmpty() && currentPackage.startsWith(packageName)) {
180+
qualifiedName = qualifiedName.substring(packageName.length() + 1);
181+
}
182+
completion.append(qualifiedName);
183+
}
150184
}
151185
}
152186

153-
static void createTypeVariable(ITypeBinding typeVariable, StringBuilder completion) {
187+
static void createTypeVariable(ITypeBinding typeVariable, StringBuilder completion, ITypeBinding currentType, List<ITypeBinding> importedTypes, String currentPackage) {
154188
completion.append(typeVariable.getName());
155189

156190
if (typeVariable.getSuperclass() != null
157191
&& typeVariable.getTypeBounds()[0].getKey().equals(typeVariable.getSuperclass().getKey())) {
158192
completion.append(' ');
159193
completion.append(EXTENDS);
160194
completion.append(' ');
161-
createType(typeVariable.getSuperclass(), completion);
195+
createType(typeVariable.getSuperclass(), completion, currentType, importedTypes, currentPackage);
162196
}
163197
if (typeVariable.getInterfaces() != null) {
164198
if (!typeVariable.getTypeBounds()[0].getKey().equals(typeVariable.getSuperclass().getKey())) {
@@ -172,9 +206,25 @@ static void createTypeVariable(ITypeBinding typeVariable, StringBuilder completi
172206
completion.append(EXTENDS);
173207
completion.append(' ');
174208
}
175-
createType(typeVariable.getInterfaces()[i], completion);
209+
createType(typeVariable.getInterfaces()[i], completion, currentType, importedTypes, currentPackage);
176210
}
177211
}
178212
}
179213

214+
static ITypeBinding getFirstMatchingInheritedMemberType(ITypeBinding typeBinding, String typeName) {
215+
return getFirstMatchingInheritedMemberType(typeBinding, typeName, true);
216+
}
217+
218+
static ITypeBinding getFirstMatchingInheritedMemberType(ITypeBinding typeBinding, String typeName, boolean canUsePrivate) {
219+
for (ITypeBinding memberType : typeBinding.getDeclaredTypes()) {
220+
if (typeName.equals(memberType.getName()) && (canUsePrivate || !Flags.isPrivate(memberType.getModifiers()))) {
221+
return memberType;
222+
}
223+
}
224+
if (typeBinding.getSuperclass() != null) {
225+
return getFirstMatchingInheritedMemberType(typeBinding.getSuperclass(), typeName, false);
226+
}
227+
return null;
228+
}
229+
180230
}

‎org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/KeyUtils.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Red Hat, Inc. and others.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Red Hat Inc. - initial API and implementation
13+
*******************************************************************************/
14+
package org.eclipse.jdt.internal.codeassist;
15+
16+
/**
17+
* Utility methods for manipulating binding keys.
18+
*/
19+
public class KeyUtils {
20+
21+
/**
22+
* Returns the given method key with the owner type and return type removed.
23+
*
24+
* @param methodKey the method key to remove the owner type and return type from
25+
* @return the given method key with the owner type and return type removed
26+
*/
27+
public static final String getMethodKeyWithOwnerTypeAndReturnTypeRemoved(String methodKey) {
28+
return methodKey.substring(methodKey.indexOf('.'), methodKey.lastIndexOf(')') + 1);
29+
}
30+
31+
}

0 commit comments

Comments
 (0)