Skip to content

Commit

Permalink
Fix for issue #287: prefer proposals with compatible types
Browse files Browse the repository at this point in the history 
NOTE: This is limited to assign of property or variable expressions.
  • Loading branch information
@eric-milles
eric-milles committed May 7, 2017
1 parent 9e7e49c commit 6ebcd26
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 74 deletions.
231 changes: 183 additions & 48 deletions ...st.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/RelevanceTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,107 +23,242 @@ import org.junit.Test
*/
final class RelevanceTests extends CompletionTestSuite {

private static String NEWLINE = "\n "
private static String THIS_TO = "this.to"
private static String PU = "pu"

@Test
void testParamThenFieldThenMethodThenDGM() {
String contents = "class Outer {\n def f\n def m(p) {\nnull\n \n}\n}"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, NEWLINE)))
assertProposalOrdering(proposals, "p", "f", "getF", "m", "find")
String contents = '''\
class Outer {
def f
def m(p) {
null
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'null\n')))
assertProposalOrdering(proposals, 'p', 'f', 'getF', 'm', 'find')
}

@Test
void testLocalVarThenFieldThenMethodThenDGM() {
String contents = "class Outer {\n def f\n def m() {\ndef p\n \n}\n}"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, NEWLINE)))
assertProposalOrdering(proposals, "p", "f", "getF", "m", "find")
String contents = '''\
class Outer {
def f
def m() {
def p
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'def p\n')))
assertProposalOrdering(proposals, 'p', 'f', 'getF', 'm', 'find')
}

@Test
void testObjectMethods() {
String contents = "class Outer {\n def toZZZ(p) {\n this.to \n}\n}"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, THIS_TO)))
assertProposalOrdering(proposals, "toZZZ", "toString")
String contents = '''\
class Outer {
def toZZZ(p) {
this.to
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'this.to')))
assertProposalOrdering(proposals, 'toZZZ', 'toString')
}

@Test
void testOverriddenObjectMethods() {
String contents = "class Outer {\n def toZZZ(p) {\n this.to } \n String toString() { \n}\n}"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, THIS_TO)))
assertProposalOrdering(proposals, "toString", "toZZZ")
String contents = '''\
class Outer {
def toZZZ(p) {
this.to
}
String toString() {
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'this.to')))
assertProposalOrdering(proposals, 'toString', 'toZZZ')
}

@Test
void testNewMethodThenModifier() {
String contents = "class Other extends Outer { \n pu\n def x() { } }\n class Outer {\n def pub() { \n}\n}"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, PU)))
assertProposalOrdering(proposals, "pub", "public")
String contents = '''\
class Other extends Outer {
pu
def x() { }
}
class Outer {
def pub() {
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'pu')))
assertProposalOrdering(proposals, 'pub', 'public')
}

// now test that fields and methods of the assigned type are above other methods

@Test // this one should do alphabetical ordering
void testFieldOfAssignedType1() {
String contents = "class Other {\ndef x() { def f = a }\n String az\n int aa }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "aa", "az")
String contents = '''\
class Other {
def x() {
def f = a
}
String az
int aa
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'aa', 'az')
}

@Test // this one should do the string first
void testFieldOfAssignedType2() {
String contents = "class Other {\ndef x() { String f = a }\n String az\n int aa }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
String f = a
}
String az
int aa
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do the int first
void testFieldOfAssignedType3() {
String contents = "class Other {\ndef x() { int f = a }\n String aa\n int az }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
int f = a
}
String aa
int az
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do alphabetical ordering
void testMethodOfAssignedType1() {
String contents = "class Other {\ndef x() { def f = a }\n String az() { }\n int aa() { } }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "aa", "az")
String contents = '''\
class Other {
def x() {
def f = a
}
String az() { }
int aa() { }
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'aa', 'az')
}

@Test // this one should do the string first
void testMethodOfAssignedType2() {
String contents = "class Other {\ndef x() { String f = a }\n String az() { }\n int aa() { } }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
String f = a
}
int aa() { }
String az() { }
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do the string first
void testMethodOfAssignedType2a() {
String contents = '''\
class Other {
private String f
def x() {
this.f = a // property expression
}
int aa() { }
String az() { }
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do the string first
void testMethodOfAssignedType2b() {
String contents = '''\
class Other {
String f
def x() {
this.f = a // property expression
}
int aa() { }
String az() { }
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do the int first
void testMethodOfAssignedType3() {
String contents = "class Other {\ndef x() { int f = a }\n String aa() { }\n int az() { } }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
int f = a
}
String aa() { }
int az() { }
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do alphabetical ordering
void testMethodAndFieldOfAssignedType1() {
String contents = "class Other {\ndef x() { def f = a }\n String az() { }\n int aa }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "aa", "az")
String contents = '''\
class Other {
def x() {
def f = a
}
String az() { }
int aa
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'aa', 'az')
}

@Test // this one should do the string first
void testMethodAndFieldOfAssignedType2() {
String contents = "class Other {\ndef x() { String f = a }\n String az() { }\n int aa }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
String f = a
}
String az() { }
int aa
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}

@Test // this one should do the int first
void testMethodAndFieldOfAssignedType3() {
String contents = "class Other {\ndef x() { int f = a }\n String aa() { }\n int az }"
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, " = a")))
assertProposalOrdering(proposals, "az", "aa")
String contents = '''\
class Other {
def x() {
int f = a
}
String aa() { }
int az
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, ' = a')))
assertProposalOrdering(proposals, 'az', 'aa')
}
}
3 changes: 2 additions & 1 deletion ...mpletion/src/org/codehaus/groovy/eclipse/codeassist/creators/AbstractProposalCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.ModuleNode;
import org.codehaus.groovy.eclipse.codeassist.ProposalUtils;
import org.eclipse.jdt.groovy.core.util.GroovyUtils;
import org.eclipse.jdt.groovy.search.AccessorSupport;
import org.eclipse.jdt.groovy.search.VariableScope;

Expand Down Expand Up @@ -125,7 +126,7 @@ protected void getAllSupers(ClassNode type, Set<ClassNode> set, Set<ClassNode> e
}

protected boolean isInterestingType(ClassNode type) {
return lhsType != null && ClassHelper.getUnwrapper(type).equals(lhsType);
return lhsType != null && GroovyUtils.isAssignable(type, lhsType);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ....completion/src/org/codehaus/groovy/eclipse/codeassist/creators/FieldProposalCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public List<IGroovyProposal> findAllProposals(ClassNode type, Set<ClassNode> cat
for (FieldNode field : allFields) {
// in static context, only allow static fields
if ((!isStatic || field.isStatic()) && ProposalUtils.looselyMatches(prefix, field.getName())) {
float relevanceMultiplier = isInterestingType(field.getType()) ? 101 : 1;
float relevanceMultiplier = isInterestingType(field.getType()) ? 1.1f : 1.0f;
if (field.isStatic()) relevanceMultiplier *= 0.1f;
// de-emphasize 'this' references inside closure
if (!isFirstTime) relevanceMultiplier *= 0.1f;
Expand Down
38 changes: 26 additions & 12 deletions ...completion/src/org/codehaus/groovy/eclipse/codeassist/creators/MethodProposalCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,9 @@ public List<IGroovyProposal> findAllProposals(ClassNode type, Set<ClassNode> cat
for (MethodNode method : allMethods) {
String methodName = method.getName();
if ((!isStatic || method.isStatic() || method.getDeclaringClass() == VariableScope.OBJECT_CLASS_NODE) && checkName(methodName)) {
boolean isInterestingType = false;
if (isStatic && method.isStatic()) {
isInterestingType = true;
} else {
isInterestingType = isInterestingType(method.getReturnType());
}
if (ProposalUtils.looselyMatches(prefix, methodName)) {
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
float relevanceMultiplier = isInterestingType ? 101f : 1f;
relevanceMultiplier *= method.isStatic() ? 0.1f : 1f;
// de-emphasize 'this' references inside closure
relevanceMultiplier *= !alreadySeen.isEmpty() ? 0.1f : 1f;
proposal.setRelevanceMultiplier(relevanceMultiplier);
setRelevanceMultiplier(proposal, firstTime, isStatic);
proposals.add(proposal);
}

Expand All @@ -82,7 +72,7 @@ public List<IGroovyProposal> findAllProposals(ClassNode type, Set<ClassNode> cat
alreadySeenFields.add(mockFieldName);
if (hasNoField(method.getDeclaringClass(), methodName)) {
GroovyFieldProposal proposal = new GroovyFieldProposal(createMockField(method));
proposal.setRelevanceMultiplier(isInterestingType ? 11 : 1);
proposal.setRelevanceMultiplier(/*isInterestingType ? 11 :*/ 1);
proposals.add(proposal);
}
}
Expand Down Expand Up @@ -163,4 +153,28 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
}
}
}

private void setRelevanceMultiplier(GroovyMethodProposal proposal, boolean firstTime, boolean isStatic) {
MethodNode method = proposal.getMethod();

float relevanceMultiplier;
if (isStatic && method.isStatic()) {
relevanceMultiplier = 10.0f;
} else if (!method.isStatic()) {
relevanceMultiplier = 1.00f;
} else {
relevanceMultiplier = 0.77f;
}

// de-emphasize 'this' references inside closure
if (!firstTime) {
relevanceMultiplier *= 0.1f;
}

if (isInterestingType(method.getReturnType())) {
relevanceMultiplier *= 10.01f; // must beat out uninteresting-typed fields, which have a very high relevance
}

proposal.setRelevanceMultiplier(relevanceMultiplier);
}
}
Loading

0 comments on commit 6ebcd26

Please sign in to comment.