Skip to content

Faulty/Undesired CodeCompletion behaviour #2620 #3350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
angelickite wants to merge 5 commits into
base: master
Choose a base branch
from
+249 −19
Open

Faulty/Undesired CodeCompletion behaviour #2620 #3350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2184482
Faulty/Undesired CodeCompletion behaviour #2620
angelickite Nov 25, 2024
aa0c66d
Faulty/Undesired CodeCompletion behaviour #2620
stephan-herrmann Jan 18, 2025
6627f62
Merge branch 'eclipse-jdt:master' into gh2620-2
angelickite Oct 2, 2025
2a43061
rollback L-Paren checks, add argument persistence
angelickite Oct 4, 2025
1cfe27b
add test to observe argument preservation, adjust existing tests
angelickite Oct 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 197 additions & 1 deletion org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ public void testCamelCaseField1() throws JavaModelException {
}
// https://bugs.eclipse.org/bugs/show_bug.cgi?id=102572
public void testCamelCaseLocalVariable1() throws JavaModelException {
String old = getSetCodeAssistProperty(JavaCore.CODEASSIST_VISIBILITY_CHECK, JavaCore.ENABLED);
String old = getSetCodeAssistProperty(JavaCore.CODEASSIST_CAMEL_CASE_MATCH, JavaCore.ENABLED);
try {
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy(
Expand Down Expand Up @@ -26221,4 +26221,200 @@ public void test() {
+ "}",
requestor.getResults());
}
public void testGH2620_1() throws JavaModelException {
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
if (true) { // the if-statement is crucial for this test.
/*x*/local.ref.test();
}
}
public static class Test {
public Test ref;
public static void test() {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.ref.te";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
int relevance = R_DEFAULT + R_RESOLVED + R_INTERESTING + R_CASE + R_NON_RESTRICTED;
assertResults(
"test[METHOD_REF]{test(), LGH2620$Test;, ()V, null, null, test, null, [157, 164], "+relevance+"}",
requestor.getResults()
);
}
public void testGH2620_1a() throws JavaModelException {
// to observe the difference when cursor is right to '('
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
if (true) { // the if-statement is crucial for this test.
/*x*/local.ref.test();
}
}
public static class Test {
public Test ref;
public static void test() {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.ref.test(";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
int relevance = R_DEFAULT + R_RESOLVED + R_INTERESTING + R_EXACT_NAME + R_CASE + R_NON_RESTRICTED + R_UNQUALIFIED;
assertResults(
"test[METHOD_REF]{, LGH2620$Test;, ()V, null, null, test, null, [163, 163], "+relevance+"}",
requestor.getResults()
);
}
public void testGH2620_1b() throws JavaModelException {
// to observe argument proposal
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
int j = 1;
if (true) { // the if-statement is crucial for this test.
/*x*/local.ref.test();
}
}
public static class Test {
public Test ref;
public static void test(int i) {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.ref.test";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
int relevance = R_DEFAULT + R_RESOLVED + R_INTERESTING + R_EXACT_NAME + R_CASE + R_NON_RESTRICTED;
assertResults(
"test[METHOD_REF]{test(), LGH2620$Test;, (I)V, null, null, test, (i), [170, 177], "+relevance+"}",
requestor.getResults()
);
}
public void testGH2620_1c() throws JavaModelException {
// to observe argument preservation
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
int j = 1;
if (true) { // the if-statement is crucial for this test.
/*x*/local.ref.test(1);
}
}
public static class Test {
public Test ref;
public static void test(int i) {}
public static void test2() {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.ref.test";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
// argument preservation can be observed by the replacement range which should only cover the method name:
assertResults(
"""
test2[METHOD_REF]{test2, LGH2620$Test;, ()V, null, null, test2, null, [170, 174], 49}
test[METHOD_REF]{test, LGH2620$Test;, (I)V, null, null, test, (i), [170, 174], 53}
""".strip()
,
requestor.getResults()
);
}
public void testGH2620_2() throws JavaModelException {
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
/*x*/local..test(); // cursor is tested for between the dots: "local.|.test();"
}
public static class Test {
public Test ref;
public static void test() {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
assertResults("""
test[METHOD_REF]{test(), LGH2620$Test;, ()V, null, null, test, null, [92, 92], 49}
clone[METHOD_REF]{clone(), Ljava.lang.Object;, ()Ljava.lang.Object;, null, null, clone, null, [92, 92], 60}
equals[METHOD_REF]{equals(), Ljava.lang.Object;, (Ljava.lang.Object;)Z, null, null, equals, (obj), [92, 92], 60}
finalize[METHOD_REF]{finalize(), Ljava.lang.Object;, ()V, null, null, finalize, null, [92, 92], 60}
getClass[METHOD_REF]{getClass(), Ljava.lang.Object;, ()Ljava.lang.Class;, null, null, getClass, null, [92, 92], 60}
hashCode[METHOD_REF]{hashCode(), Ljava.lang.Object;, ()I, null, null, hashCode, null, [92, 92], 60}
notify[METHOD_REF]{notify(), Ljava.lang.Object;, ()V, null, null, notify, null, [92, 92], 60}
notifyAll[METHOD_REF]{notifyAll(), Ljava.lang.Object;, ()V, null, null, notifyAll, null, [92, 92], 60}
ref[FIELD_REF]{ref, LGH2620$Test;, LGH2620$Test;, null, null, ref, null, [92, 92], 60}
toString[METHOD_REF]{toString(), Ljava.lang.Object;, ()Ljava.lang.String;, null, null, toString, null, [92, 92], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, ()V, null, null, wait, null, [92, 92], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, (J)V, null, null, wait, (millis), [92, 92], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, (JI)V, null, null, wait, (millis, nanos), [92, 92], 60}
""".strip()
,
requestor.getResults()
);
}
public void testGH2620_3() throws JavaModelException {
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy("/Completion/src/GH2620.java", """
public class GH2620 {
public static void main (String[] args) {
Test local;
if (true) { // the if-statement is crucial for this test.
/*x*/local..test(); // cursor is tested for between the dots: "local.|.test();"
}
}
public static class Test {
public Test ref;
public static void test() {}
}
}
""");
CompletionTestsRequestor2 requestor = new CompletionTestsRequestor2(true, true, true, false);
String str = this.workingCopies[0].getSource();
String completeBehind = "/*x*/local.";
int cursorLocation = str.indexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
assertResults("""
test[METHOD_REF]{test(), LGH2620$Test;, ()V, null, null, test, null, [153, 153], 49}
clone[METHOD_REF]{clone(), Ljava.lang.Object;, ()Ljava.lang.Object;, null, null, clone, null, [153, 153], 60}
equals[METHOD_REF]{equals(), Ljava.lang.Object;, (Ljava.lang.Object;)Z, null, null, equals, (obj), [153, 153], 60}
finalize[METHOD_REF]{finalize(), Ljava.lang.Object;, ()V, null, null, finalize, null, [153, 153], 60}
getClass[METHOD_REF]{getClass(), Ljava.lang.Object;, ()Ljava.lang.Class;, null, null, getClass, null, [153, 153], 60}
hashCode[METHOD_REF]{hashCode(), Ljava.lang.Object;, ()I, null, null, hashCode, null, [153, 153], 60}
notify[METHOD_REF]{notify(), Ljava.lang.Object;, ()V, null, null, notify, null, [153, 153], 60}
notifyAll[METHOD_REF]{notifyAll(), Ljava.lang.Object;, ()V, null, null, notifyAll, null, [153, 153], 60}
ref[FIELD_REF]{ref, LGH2620$Test;, LGH2620$Test;, null, null, ref, null, [153, 153], 60}
toString[METHOD_REF]{toString(), Ljava.lang.Object;, ()Ljava.lang.String;, null, null, toString, null, [153, 153], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, ()V, null, null, wait, null, [153, 153], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, (J)V, null, null, wait, (millis), [153, 153], 60}
wait[METHOD_REF]{wait(), Ljava.lang.Object;, (JI)V, null, null, wait, (millis, nanos), [153, 153], 60}
""".strip()
,
requestor.getResults()
);
}
}
30 changes: 15 additions & 15 deletions ...clipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CompletionTests_1_5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9297,7 +9297,7 @@ public void test0295() throws JavaModelException {
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);

assertResults(
"compareTo[METHOD_REF]{compareTo(), Ltest.ComparableTest<*>;, (*)I, compareTo, (t), " + (R_DEFAULT + R_RESOLVED + R_INTERESTING + R_CASE + R_NON_STATIC + R_NON_RESTRICTED) + "}",
"compareTo[METHOD_REF]{compareTo, Ltest.ComparableTest<*>;, (*)I, compareTo, (t), " + (R_DEFAULT + R_RESOLVED + R_INTERESTING + R_CASE + R_NON_STATIC + R_NON_RESTRICTED) + "}",
requestor.getResults());
}
//https://bugs.eclipse.org/bugs/show_bug.cgi?id=99928
Expand Down Expand Up @@ -14726,20 +14726,20 @@ public void testBug573279() throws Exception {
int cursorLocation = str.lastIndexOf(completeBehind) + completeBehind.length();
this.workingCopies[0].codeComplete(cursorLocation, requestor, this.wcOwner);
int relevance = R_DEFAULT + R_INTERESTING + R_RESOLVED + R_CASE + R_NON_STATIC + R_NON_RESTRICTED;
assertEquals("add[METHOD_REF]{add(), LMyArrayList<Ljava.lang.String;>;, (Ljava.lang.String;)Z, null, null, add, (t), replace[289, 318], token[289, 292], "+relevance+"}\n" +
"clone[METHOD_REF]{clone(), Ljava.lang.Object;, ()Ljava.lang.Object;, null, null, clone, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"equals[METHOD_REF]{equals(), Ljava.lang.Object;, (Ljava.lang.Object;)Z, null, null, equals, (obj), replace[289, 318], token[289, 292], "+relevance+"}\n" +
"finalize[METHOD_REF]{finalize(), Ljava.lang.Object;, ()V, null, null, finalize, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"getClass[METHOD_REF]{getClass(), Ljava.lang.Object;, ()Ljava.lang.Class<+Ljava.lang.Object;>;, null, null, getClass, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"hashCode[METHOD_REF]{hashCode(), Ljava.lang.Object;, ()I, null, null, hashCode, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"notify[METHOD_REF]{notify(), Ljava.lang.Object;, ()V, null, null, notify, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"notifyAll[METHOD_REF]{notifyAll(), Ljava.lang.Object;, ()V, null, null, notifyAll, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"remove[METHOD_REF]{remove(), LMyArrayList<Ljava.lang.String;>;, (Ljava.lang.Object;)Z, null, null, remove, (o), replace[289, 318], token[289, 292], "+relevance+"}\n" +
"size[METHOD_REF]{size(), LMyArrayList<Ljava.lang.String;>;, ()I, null, null, size, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"toString[METHOD_REF]{toString(), Ljava.lang.Object;, ()Ljava.lang.String;, null, null, toString, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait(), Ljava.lang.Object;, ()V, null, null, wait, null, replace[289, 318], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait(), Ljava.lang.Object;, (J)V, null, null, wait, (millis), replace[289, 318], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait(), Ljava.lang.Object;, (JI)V, null, null, wait, (millis, nanos), replace[289, 318], token[289, 292], "+relevance+"}",
assertEquals("add[METHOD_REF]{add, LMyArrayList<Ljava.lang.String;>;, (Ljava.lang.String;)Z, null, null, add, (t), replace[289, 292], token[289, 292], "+relevance+"}\n" +
"clone[METHOD_REF]{clone, Ljava.lang.Object;, ()Ljava.lang.Object;, null, null, clone, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"equals[METHOD_REF]{equals, Ljava.lang.Object;, (Ljava.lang.Object;)Z, null, null, equals, (obj), replace[289, 292], token[289, 292], "+relevance+"}\n" +
"finalize[METHOD_REF]{finalize, Ljava.lang.Object;, ()V, null, null, finalize, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"getClass[METHOD_REF]{getClass, Ljava.lang.Object;, ()Ljava.lang.Class<+Ljava.lang.Object;>;, null, null, getClass, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"hashCode[METHOD_REF]{hashCode, Ljava.lang.Object;, ()I, null, null, hashCode, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"notify[METHOD_REF]{notify, Ljava.lang.Object;, ()V, null, null, notify, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"notifyAll[METHOD_REF]{notifyAll, Ljava.lang.Object;, ()V, null, null, notifyAll, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"remove[METHOD_REF]{remove, LMyArrayList<Ljava.lang.String;>;, (Ljava.lang.Object;)Z, null, null, remove, (o), replace[289, 292], token[289, 292], "+relevance+"}\n" +
"size[METHOD_REF]{size, LMyArrayList<Ljava.lang.String;>;, ()I, null, null, size, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"toString[METHOD_REF]{toString, Ljava.lang.Object;, ()Ljava.lang.String;, null, null, toString, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait, Ljava.lang.Object;, ()V, null, null, wait, null, replace[289, 292], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait, Ljava.lang.Object;, (J)V, null, null, wait, (millis), replace[289, 292], token[289, 292], "+relevance+"}\n" +
"wait[METHOD_REF]{wait, Ljava.lang.Object;, (JI)V, null, null, wait, (millis, nanos), replace[289, 292], token[289, 292], "+relevance+"}",
requestor.getResults());
}
public void testGH969_completeOnFirstArgumentPosition_noToken() throws Exception {
Expand Down
40 changes: 37 additions & 3 deletions org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3269,8 +3269,16 @@ private void completionOnMessageSendName(ASTNode astNode, Binding qualifiedBindi
CompletionOnMessageSendName messageSend = (CompletionOnMessageSendName) astNode;

setTokenRange(messageSend.sourceStart, messageSend.sourceEnd);
if (messageSend.statementEnd > messageSend.sourceStart)
setSourceRange(messageSend.sourceStart, messageSend.statementEnd);

if (messageSend.statementEnd > messageSend.sourceStart) {
// we ensure any existing arguments are preserved
boolean existingMethodCallHasBeenProvidedArguments = (messageSend.arguments != null);
if (existingMethodCallHasBeenProvidedArguments) {
setSourceRange(messageSend.sourceStart, messageSend.sourceEnd);
} else {
setSourceRange(messageSend.sourceStart, messageSend.statementEnd);
}
}

this.completionToken = messageSend.selector;

Expand Down Expand Up @@ -3586,7 +3594,33 @@ private void completionOnQualifiedNameReference(ASTNode astNode, ASTNode enclosi
private void internalCompletionOnQualifiedReference(ASTNode ref, ASTNode enclosingNode, Binding qualifiedBinding, Scope scope,
boolean insideTypeAnnotation, boolean isInsideAnnotationAttribute, InvocationSite site, char[][] tokens, long[] sourcePositions)
{
long completionPosition = sourcePositions[sourcePositions.length - 1];
// we take the cursor location into consideration regarding segments of a qualified name,
//
// Example:
// void x(TreeNode node) {
// TreeNode other = node.ch|.child.child.parent;
// // ...
// }
//
// in the above example, completionPosition will match against the cursor after 'ch', i.e. sourcePositions[1].
// (sourcePositions[0] points to 'node', sourcePositions[4] to 'parent')
long completionPosition = sourcePositions[sourcePositions.length - 1]; // use last segment as a fallback for resilience
for (long p : sourcePositions) {
int start = (int) (p >>> 32);
int end = (int) (p);
if (start > end) { // in specific cases like "node.|.child" the start might be larger than the end (see GH-2620)
// swap them for sanity / resilience
int tmp = start;
start = end;
end = tmp;
}
boolean cursorWithinBounds = (start <= this.actualCompletionPosition && this.actualCompletionPosition <= end);
if (cursorWithinBounds) {
completionPosition = p;
break;
}
}

if (qualifiedBinding.problemId() == ProblemReasons.NotFound) {
setSourceAndTokenRange((int) (completionPosition >>> 32), (int) completionPosition);
// complete field members with missing fields type
Expand Down