Skip to content

Commit

Permalink
Fix for #787: tweak relevance multipliers for fields and methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jan 8, 2019
1 parent 7c6acab commit e39b61f
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 26 deletions.
45 changes: 44 additions & 1 deletion ...st.completion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/RelevanceTests.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -118,6 +118,49 @@ final class RelevanceTests extends CompletionTestSuite {
assertProposalOrdering(proposals, 'getAt', 'getDelegate()', 'getDirective()', 'getMetaClass()', 'getDefaultMetaClass()')
}

@Test // https://github.com/groovy/groovy-eclipse/issues/787
void testClosureMethodsAndProperties3() {
String contents = '''\
class Outer {
static class Foo {
String string
}
static class Bar {
String string
void meth(Foo foo) {
foo.with {
s
}
}
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getLastIndexOf(contents, 's')))
assertProposalOrdering(proposals, 'string : String - Foo', 'string : String - Bar', 'setString(String value) : void - Foo', 'setString(String value) : void - Bar', 'setMetaClass')
}

@Test
void testClosureMethodsAndProperties4() {
String contents = '''\
class Outer {
static class Foo {
String string
}
static class Bar {
String string
def foo(@DelegatesTo(value=Foo, strategy=Closure.OWNER_FIRST) Closure c) {}
void meth() {
foo { ->
s
}
}
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getLastIndexOf(contents, 's')))
assertProposalOrdering(proposals, 'string : String - Bar', 'string : String - Foo', 'setString(String value) : void - Bar', 'setString(String value) : void - Foo', 'setMetaClass')
}

@Test
void testStaticImportFieldsAndMethods() {
String contents = '''\
Expand Down
18 changes: 10 additions & 8 deletions ....completion/src/org/codehaus/groovy/eclipse/codeassist/creators/FieldProposalCreator.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -59,12 +59,8 @@ public List<IGroovyProposal> findAllProposals(ClassNode type, Set<ClassNode> cat
for (FieldNode field : allFields) {
// in static context, only allow static fields
if ((!isStatic || field.isStatic()) && matcher.test(prefix, field.getName())) {
// de-emphasize 'this' references inside closure
float relevanceMultiplier = !firstTime ? 0.1f : 1.0f;
if (field.isEnum()) relevanceMultiplier *= 5.0f;

GroovyFieldProposal proposal = new GroovyFieldProposal(field);
proposal.setRelevanceMultiplier(relevanceMultiplier);
proposal.setRelevanceMultiplier(field.isEnum() ? 5 : 1);
proposals.add(proposal);

if (field.getInitialExpression() instanceof ClosureExpression) {
Expand Down Expand Up @@ -128,7 +124,9 @@ private void findStaticImportProposals(List<IGroovyProposal> proposals, String p
if (fieldName != null && matcher.test(prefix, fieldName)) {
FieldNode field = entry.getValue().getType().getField(fieldName);
if (field != null && field.isStatic()) {
proposals.add(new GroovyFieldProposal(field));
GroovyFieldProposal proposal = new GroovyFieldProposal(field);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
}
Expand All @@ -137,7 +135,9 @@ private void findStaticImportProposals(List<IGroovyProposal> proposals, String p
if (type != null) {
for (FieldNode field : type.getFields()) {
if (field.isStatic() && matcher.test(prefix, field.getName())) {
proposals.add(new GroovyFieldProposal(field));
GroovyFieldProposal proposal = new GroovyFieldProposal(field);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
}
Expand All @@ -163,6 +163,7 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
if (field.isStatic() && matcher.test(prefix, field.getName())) {
GroovyFieldProposal proposal = new GroovyFieldProposal(field);
proposal.setRequiredStaticImport(typeName + '.' + field.getName());
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
Expand All @@ -172,6 +173,7 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
if (field != null && field.isStatic()) {
GroovyFieldProposal proposal = new GroovyFieldProposal(field);
proposal.setRequiredStaticImport(favoriteStaticMember);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
Expand Down
25 changes: 13 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
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -62,7 +62,7 @@ public List<IGroovyProposal> findAllProposals(ClassNode type, Set<ClassNode> cat
if ((!isStatic || method.isStatic() || method.getDeclaringClass().equals(VariableScope.OBJECT_CLASS_NODE))) {
if (matcher.test(prefix, methodName) && !"<clinit>".equals(methodName)) {
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
setRelevanceMultiplier(proposal, firstTime, isStatic);
setRelevanceMultiplier(proposal, isStatic);
proposals.add(proposal);
}

Expand Down Expand Up @@ -115,7 +115,9 @@ private void findStaticImportProposals(List<IGroovyProposal> proposals, String p
if (methods != null) {
for (MethodNode method : methods) {
if (method.isStatic()) {
proposals.add(new GroovyMethodProposal(method));
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
}
Expand All @@ -126,7 +128,9 @@ private void findStaticImportProposals(List<IGroovyProposal> proposals, String p
if (type != null) {
for (MethodNode method : type.getMethods()) {
if (method.isStatic() && matcher.test(prefix, method.getName())) {
proposals.add(new GroovyMethodProposal(method));
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
}
Expand All @@ -152,6 +156,7 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
if (method.isStatic() && matcher.test(prefix, method.getName())) {
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
proposal.setRequiredStaticImport(typeName + '.' + method.getName());
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
Expand All @@ -162,6 +167,7 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
if (method.isStatic()) {
GroovyMethodProposal proposal = new GroovyMethodProposal(method);
proposal.setRequiredStaticImport(favoriteStaticMember);
proposal.setRelevanceMultiplier(0.95f);
proposals.add(proposal);
}
}
Expand All @@ -170,21 +176,16 @@ private void findStaticFavoriteProposals(List<IGroovyProposal> proposals, String
}
}

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

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

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

proposal.setRelevanceMultiplier(relevanceMultiplier);
Expand Down
8 changes: 5 additions & 3 deletions ...ehaus/groovy/eclipse/codeassist/processors/StatementAndExpressionCompletionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,16 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro

Consumer<IGroovyProposal> reduceRelevance = p -> {
AbstractGroovyProposal agp = (AbstractGroovyProposal) p;
agp.setRelevanceMultiplier(agp.getRelevanceMultiplier() * 0.9f);
agp.setRelevanceMultiplier(agp.getRelevanceMultiplier() * 0.999f);
};

if (!delegateProposals.isEmpty()) {
Consumer<IGroovyProposal> addDelegateQualifier = p -> ((AbstractGroovyProposal) p).setRequiredQualifier("delegate");

if (resolveStrategy == Closure.OWNER_FIRST && !ownerProposals.isEmpty()) {
Set<String> names = ownerProposals.stream().map(toName).collect(Collectors.toSet());
delegateProposals.stream().filter(p -> names.contains(toName.apply(p))).forEach(addDelegateQualifier);
delegateProposals.stream().filter(p -> names.contains(toName.apply(p)))
.forEach(addDelegateQualifier.andThen(reduceRelevance));
} else if (resolveStrategy == Closure.TO_SELF) {
delegateProposals.forEach(addDelegateQualifier.andThen(reduceRelevance));
}
Expand All @@ -394,7 +395,8 @@ private static void setClosureQualifiers(Collection<IGroovyProposal> delegatePro

if (resolveStrategy == Closure.DELEGATE_FIRST && !delegateProposals.isEmpty()) {
Set<String> names = delegateProposals.stream().map(toName).collect(Collectors.toSet());
ownerProposals.stream().filter(p -> names.contains(toName.apply(p))).forEach(addOwnerQualifier);
ownerProposals.stream().filter(p -> names.contains(toName.apply(p)))
.forEach(addOwnerQualifier.andThen(reduceRelevance));
} else if (resolveStrategy == Closure.TO_SELF) {
ownerProposals.forEach(addOwnerQualifier.andThen(reduceRelevance));
}
Expand Down
4 changes: 2 additions & 2 deletions ...mpletion/src/org/codehaus/groovy/eclipse/codeassist/proposals/AbstractGroovyProposal.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -141,7 +141,7 @@ protected int computeRelevance(ContentAssistContext context) {
}

if (relevance > 1 && this instanceof GroovyMethodProposal) {
relevance *= 0.9f; // promote fields
relevance *= 0.99f; // promote fields
}

return Math.max(relevance, RelevanceConstants.R_INTERESTING);
Expand Down

0 comments on commit e39b61f

Please sign in to comment.