Skip to content

Commit

Permalink
Boost relevance of T type proposals when target is Class<? extends T>
Browse files Browse the repository at this point in the history 
#419
  • Loading branch information
@eric-milles
eric-milles committed Jan 16, 2018
1 parent 378083e commit 6bcba57
Show file tree
Hide file tree
Showing 6 changed files with 205 additions and 77 deletions.
109 changes: 108 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-2017 the original author or authors.
* Copyright 2009-2018 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 All @@ -15,6 +15,8 @@
*/
package org.codehaus.groovy.eclipse.codeassist.tests

import groovy.transform.NotYetImplemented

import org.eclipse.jface.text.contentassist.ICompletionProposal
import org.junit.Ignore
import org.junit.Test
Expand Down Expand Up @@ -322,4 +324,109 @@ final class RelevanceTests extends CompletionTestSuite {
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'b.a')))
assertProposalOrdering(proposals, 'ab', 'ay', 'az', 'aa', 'ac')
}

@Test
void testClassVariableAssignedType1() {
String contents = '''\
Class<? extends CharSequence> cs = St
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test
void testClassVariableAssignedType2() {
String contents = '''\
for (Class<? extends CharSequence> cs = St; condition;) {
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testClassVariableAssignedType3() {
String contents = '''\
class X {
void meth(Class<? extends CharSequence> cs = St) {
}
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testClassVariableAssignedType4() {
String contents = '''\
class X {
Class<? extends CharSequence> cs = St
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testClassVariableAssignedType5() {
String contents = '''\
class X {
public Class<? extends CharSequence> cs = St
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testClassVariableAssignedType6() {
addJavaSource '''\
import java.lang.annotation.*;
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
public @interface Anno {
Class<? extends CharSequence> value();
}
'''.stripIndent(), 'Anno'

String contents = '''\
@Anno(value = St)
class X {
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testClassAttributeDefaultType() {
addJavaSource '''\
'''.stripIndent(), 'Anno'

String contents = '''\
import java.lang.annotation.*
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
public @interface Anno {
Class<? extends CharSequence> value() default St
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'String - java.lang', 'StringBuffer - java.lang', 'StringBuilder - java.lang', 'Stack - java.util')
}

@Test @NotYetImplemented
void testCatchParameterType() {
String contents = '''\
try {
} catch (St) {
}
'''.stripIndent()
ICompletionProposal[] proposals = orderByRelevance(createProposalsAtOffset(contents, getIndexOf(contents, 'St')))
assertProposalOrdering(proposals, 'StringWriterIOException - groovy.lang', 'StackOverflowError - java.lang', 'StringTestUtil - groovy.util')
}
}
125 changes: 81 additions & 44 deletions .../org/codehaus/groovy/eclipse/codeassist/processors/GroovyProposalTypeSearchRequestor.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2017 the original author or authors.
* Copyright 2009-2018 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 All @@ -20,6 +20,7 @@
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.codehaus.groovy.ast.ClassNode;
Expand All @@ -28,13 +29,17 @@
import org.codehaus.groovy.ast.ModuleNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.eclipse.GroovyLogManager;
import org.codehaus.groovy.eclipse.TraceCategory;
import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist;
import org.codehaus.groovy.eclipse.codeassist.ProposalUtils;
import org.codehaus.groovy.eclipse.codeassist.completions.GroovyJavaMethodCompletionProposal;
import org.codehaus.groovy.eclipse.codeassist.proposals.GroovyNamedArgumentProposal;
import org.codehaus.groovy.eclipse.codeassist.proposals.ProposalFormattingOptions;
import org.codehaus.groovy.eclipse.codeassist.relevance.IRelevanceRule;
import org.codehaus.groovy.eclipse.codeassist.relevance.Relevance;
import org.codehaus.groovy.eclipse.codeassist.relevance.internal.CompositeRule;
import org.codehaus.groovy.eclipse.codeassist.requestor.ContentAssistContext;
import org.codehaus.groovy.eclipse.codeassist.requestor.ContentAssistLocation;
import org.codehaus.groovy.eclipse.codeassist.requestor.MethodInfoContentAssistContext;
Expand All @@ -57,6 +62,7 @@
import org.eclipse.jdt.core.search.SearchPattern;
import org.eclipse.jdt.groovy.core.util.ReflectionUtils;
import org.eclipse.jdt.groovy.search.AccessorSupport;
import org.eclipse.jdt.groovy.search.VariableScope;
import org.eclipse.jdt.internal.codeassist.CompletionEngine;
import org.eclipse.jdt.internal.codeassist.ISearchRequestor;
import org.eclipse.jdt.internal.codeassist.RelevanceConstants;
Expand Down Expand Up @@ -102,6 +108,7 @@ public class GroovyProposalTypeSearchRequestor implements ISearchRequestor {
/** Array of fully-qualified names. Default imports should be included (aka java.lang, groovy.lang, etc.). */
private char[][] onDemandImports;

private final boolean isImport;
private final NameLookup nameLookup;
private final IProgressMonitor monitor;
private final String completionExpression;
Expand All @@ -112,6 +119,8 @@ public class GroovyProposalTypeSearchRequestor implements ISearchRequestor {
private ProposalFormattingOptions groovyProposalPrefs;
private final JavaContentAssistInvocationContext javaContext;

private IRelevanceRule relevanceRule;

// use this completion engine only to create parameter names for Constructors
private CompletionEngine mockEngine;

Expand All @@ -121,8 +130,6 @@ public class GroovyProposalTypeSearchRequestor implements ISearchRequestor {
// instead of inserting text, show context information only for constructors
private boolean contextOnly;

private final boolean isImport;

private final ContentAssistContext context;
private final AssistOptions options;

Expand All @@ -135,14 +142,14 @@ public GroovyProposalTypeSearchRequestor(
IProgressMonitor monitor) {

this.context = context;
this.offset = exprStart;
this.javaContext = javaContext;
Assert.isNotNull(javaContext.getCoreContext());
this.module = context.unit.getModuleNode();
this.unit = context.unit;
this.offset = exprStart;
this.replaceLength = replaceLength;
this.monitor = monitor;
this.nameLookup = nameLookup;
this.monitor = monitor;
this.unit = context.unit;
this.module = context.unit.getModuleNode();
this.isImport = (context.location == ContentAssistLocation.IMPORT);
// if contextOnly then do not insert any text, only show context information
this.contextOnly = (context.location == ContentAssistLocation.METHOD_CONTEXT);
Expand Down Expand Up @@ -319,14 +326,14 @@ List<ICompletionProposal> processAcceptedTypes(JDTResolver resolver) {
return Collections.emptyList();
}

HashtableOfObject onDemandFound = new HashtableOfObject();
String thisPackageName = module.getPackageName();
if (thisPackageName == null) thisPackageName = "";
initializeRelevanceRule(resolver);

List<ICompletionProposal> proposals = new LinkedList<>();
try {
HashtableOfObject onDemandFound = new HashtableOfObject();

next: for (int i = 0; i < n; i += 1) {
// does not check cancellation for every types to avoid performance loss
// does not check cancellation for every type to avoid performance loss
if ((i % CHECK_CANCEL_FREQUENCY) == 0) {
checkCancel();
}
Expand Down Expand Up @@ -365,9 +372,8 @@ List<ICompletionProposal> processAcceptedTypes(JDTResolver resolver) {
}
}

if ((enclosingTypeNames == null || enclosingTypeNames.length == 0) && CharOperation.equals(thisPackageName.toCharArray(), packageName)) {
if ((enclosingTypeNames == null || enclosingTypeNames.length == 0) && isCurrentPackage(packageName)) {
proposals.add(proposeType(packageName, simpleTypeName, modifiers, accessibility, typeName, fullyQualifiedName, false));
continue next;
} else if (((AcceptedType) onDemandFound.get(simpleTypeName)) == null && onDemandImports != null) {
char[] fullyQualifiedEnclosingTypeOrPackageName = null;
for (char[] importFlatName : onDemandImports) {
Expand Down Expand Up @@ -403,41 +409,38 @@ List<ICompletionProposal> processAcceptedTypes(JDTResolver resolver) {
}
}
} finally {
acceptedTypes = null; // reset
acceptedTypes = null;
relevanceRule = null;
}
return proposals;
}

private ICompletionProposal proposeType(char[] packageName, char[] simpleTypeName, int modifiers, int accessibility, char[] qualifiedTypeName, char[] fullyQualifiedName, boolean isQualified) {
return isImport
? proposeNoImportType(packageName, simpleTypeName, modifiers, accessibility, qualifiedTypeName, fullyQualifiedName, isQualified)
: proposeImportableType(packageName, simpleTypeName, modifiers, accessibility, qualifiedTypeName, fullyQualifiedName, isQualified);
}

private ICompletionProposal proposeNoImportType(char[] packageName, char[] simpleTypeName, int modifiers, int accessibility, char[] qualifiedTypeName, char[] fullyQualifiedName, boolean isQualified) {
char[] completionName;
if (isQualified) {
completionName = fullyQualifiedName;
} else {
completionName = simpleTypeName;
}
char[] completion = isQualified ? fullyQualifiedName : simpleTypeName;
String completionString = String.valueOf(completion);

GroovyCompletionProposal proposal = createProposal(CompletionProposal.TYPE_REF, context.completionLocation - offset);
proposal.setAccessibility(accessibility);
proposal.setCompletion(completion);
proposal.setDeclarationSignature(packageName);
proposal.setSignature(CompletionEngine.createNonGenericTypeSignature(packageName, simpleTypeName));
proposal.setCompletion(completionName);
proposal.setFlags(modifiers);
proposal.setPackageName(packageName);
proposal.setRelevance(computeRelevanceForTypeProposal(fullyQualifiedName, accessibility, modifiers));
proposal.setReplaceRange(offset, offset + replaceLength);
proposal.setSignature(CompletionEngine.createNonGenericTypeSignature(packageName, simpleTypeName));
proposal.setTokenRange(offset, context.completionLocation);
proposal.setRelevance(IRelevanceRule.DEFAULT.getRelevance(fullyQualifiedName, allTypesInUnit, accessibility, modifiers));
proposal.setTypeName(simpleTypeName);
proposal.setAccessibility(accessibility);
proposal.setPackageName(packageName);
String completionString = new String(completionName);
JavaTypeCompletionProposal javaCompletionProposal = new JavaTypeCompletionProposal(completionString, null, offset, replaceLength, ProposalUtils.getImage(proposal), ProposalUtils.createDisplayString(proposal), proposal.getRelevance(), completionString, javaContext);
javaCompletionProposal.setRelevance(proposal.getRelevance());

return javaCompletionProposal;
}

private ICompletionProposal proposeType(char[] packageName, char[] simpleTypeName, int modifiers, int accessibility, char[] qualifiedTypeName, char[] fullyQualifiedName, boolean isQualified) {
return isImport
? proposeNoImportType(packageName, simpleTypeName, modifiers, accessibility, qualifiedTypeName, fullyQualifiedName, isQualified)
: proposeImportableType(packageName, simpleTypeName, modifiers, accessibility, qualifiedTypeName, fullyQualifiedName, isQualified);
JavaTypeCompletionProposal javaProposal = new JavaTypeCompletionProposal(completionString, null, offset, replaceLength, ProposalUtils.getImage(proposal), ProposalUtils.createDisplayString(proposal), proposal.getRelevance(), completionString, javaContext);
javaProposal.setTriggerCharacters(ProposalUtils.TYPE_TRIGGERS);
javaProposal.setRelevance(proposal.getRelevance());
return javaProposal;
}

private ICompletionProposal proposeImportableType(char[] packageName, char[] simpleTypeName, int modifiers, int accessibility, char[] qualifiedTypeName, char[] fullyQualifiedName, boolean isQualified) {
Expand All @@ -446,10 +449,9 @@ private ICompletionProposal proposeImportableType(char[] packageName, char[] sim
proposal.setCompletion(isQualified ? fullyQualifiedName : simpleTypeName);
proposal.setDeclarationSignature(packageName);
proposal.setFlags(modifiers);
proposal.setNameLookup(nameLookup);
proposal.setPackageName(packageName);
proposal.setRelevance(computeRelevanceForTypeProposal(fullyQualifiedName, accessibility, modifiers));
proposal.setReplaceRange(offset, offset + replaceLength);
proposal.setRelevance(IRelevanceRule.DEFAULT.getRelevance(fullyQualifiedName, allTypesInUnit, accessibility, modifiers));
proposal.setSignature(CompletionEngine.createNonGenericTypeSignature(packageName, simpleTypeName));
proposal.setTokenRange(offset, context.completionLocation);
proposal.setTypeName(simpleTypeName);
Expand Down Expand Up @@ -578,7 +580,8 @@ List<ICompletionProposal> processAcceptedConstructors(Set<String> usedParams, JD
}

} finally {
acceptedTypes = null; // reset
acceptedTypes = null;
relevanceRule = null;
}
return proposals;
}
Expand Down Expand Up @@ -711,7 +714,7 @@ private void populateReplacementInfo(GroovyCompletionProposal proposal, char[] p
//typeProposal.setTypeName(simpleTypeName);
//typeProposal.setPackageName(packageName);
//typeProposal.setDeclarationSignature(declarationSignature);
//typeProposal.setRelevance(IRelevanceRule.DEFAULT.getRelevance(fullyQualifiedName, allTypesInUnit, accessibility, augmentedModifiers));
//typeProposal.setRelevance(computeRelevanceForTypeProposal(fullyQualifiedName, accessibility, augmentedModifiers));

proposal.setRequiredProposals(new CompletionProposal[] {typeProposal});
}
Expand Down Expand Up @@ -740,7 +743,13 @@ private char[] createConstructorSignature(char[][] parameterTypes, boolean isQua
return Signature.createMethodSignature(parameterTypeSigs, new char[] { 'V' });
}

int computeRelevanceForCaseMatching(char[] token, char[] proposalName) {
protected final GroovyCompletionProposal createProposal(int kind, int completionOffset) {
GroovyCompletionProposal proposal = new GroovyCompletionProposal(kind, completionOffset);
proposal.setNameLookup(nameLookup);
return proposal;
}

private int computeRelevanceForCaseMatching(char[] token, char[] proposalName) {
if (CharOperation.equals(token, proposalName, true /* do not ignore case */)) {
return RelevanceConstants.R_CASE + RelevanceConstants.R_EXACT_NAME;
} else if (CharOperation.equals(token, proposalName, false /* ignore case */)) {
Expand All @@ -749,10 +758,38 @@ int computeRelevanceForCaseMatching(char[] token, char[] proposalName) {
return 0;
}

protected final GroovyCompletionProposal createProposal(int kind, int completionOffset) {
GroovyCompletionProposal proposal = new GroovyCompletionProposal(kind, completionOffset);
proposal.setNameLookup(nameLookup);
return proposal;
private int computeRelevanceForTypeProposal(char[] fullyQualifiedName, int accessibility, int modifiers) {
IRelevanceRule rule = Optional.ofNullable(relevanceRule).orElse(IRelevanceRule.DEFAULT);
return rule.getRelevance(fullyQualifiedName, allTypesInUnit, accessibility, modifiers);
}

private void initializeRelevanceRule(JDTResolver resolver) {
if (context.lhsNode instanceof Variable) {
ClassNode lhsType = ((Variable) context.lhsNode).getType();
if (VariableScope.CLASS_CLASS_NODE.equals(lhsType) && lhsType.isUsingGenerics()) {
ClassNode targetType = lhsType.getGenericsTypes()[0].getType();
if (VariableScope.OBJECT_CLASS_NODE.equals(targetType)) return;

// create a relevance rule that will boost types derived form the target type
IRelevanceRule rule = (char[] fullyQualifiedName, IType[] contextTypes, int accessibility, int modifiers) -> {
try {
ClassNode sourceType = resolver.resolve(String.valueOf(fullyQualifiedName));
if (sourceType.isDerivedFrom(targetType) || sourceType.implementsInterface(targetType)) {
return 10;
}
} catch (RuntimeException e) {
if (GroovyLogManager.manager.hasLoggers()) {
GroovyLogManager.manager.log(TraceCategory.CONTENT_ASSIST, e.getMessage());
} else {
System.err.println(getClass().getSimpleName() + ": " + e.getMessage());
}
}
return 0;
};

relevanceRule = CompositeRule.of(1.0, IRelevanceRule.DEFAULT, 2.0, rule);
}
}
}

/**
Expand Down
Loading

0 comments on commit 6bcba57

Please sign in to comment.