Skip to content

Commit

Permalink
Fix for issue #367: enum fields require supporting import statements
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jan 23, 2018
1 parent 325a7f1 commit 016acc4
Show file tree
Hide file tree
Showing 6 changed files with 249 additions and 176 deletions.
30 changes: 16 additions & 14 deletions ...mpletion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/CompletionTestSuite.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 Down Expand Up @@ -343,34 +343,36 @@ abstract class CompletionTestSuite extends GroovyEclipseTestSuite {
}
}

protected void checkProposalChoices(String contents, String toFind, String lookFor, String replacementString, String[] expectedChoices) {
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, toFind))
protected ICompletionProposal checkUniqueProposal(CharSequence contents, String completionExpr, String completionName = completionExpr, String replacementString) {
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, completionExpr))
checkReplacementString(proposals, replacementString, 1)
ICompletionProposal proposal = findFirstProposal(proposals, lookFor)
ICompletionProposal[] choices = proposal.choices
findFirstProposal(proposals, completionName)
}

protected void checkProposalChoices(String contents, String completionExpr, String lookFor, String replacementString, String[] expectedChoices) {
ICompletionProposal proposal = checkUniqueProposal(contents, completionExpr, lookFor, replacementString)

ICompletionProposal[] choices = proposal.choices
assertEquals(expectedChoices.length, choices.length)
for (int i = 0; i < expectedChoices.length; i++) {
for (int i = 0; i < expectedChoices.length; i += 1) {
assertEquals('unexpected choice', expectedChoices[i], choices[i].displayString)
}
}

protected void checkProposalChoices(String contents, String lookFor, String replacementString, String[][] expectedChoices) {
ICompletionProposal[] proposals = createProposalsAtOffset(contents, getLastIndexOf(contents, lookFor))
checkReplacementString(proposals, replacementString, 1)
ICompletionProposal proposal = findFirstProposal(proposals, lookFor)
proposal.getReplacementString() // instantiate the guesses
ICompletionProposal[][] choices = proposal.choices
protected void checkProposalChoices(String contents, String completion, String replacementString, String[][] expectedChoices) {
ICompletionProposal proposal = checkUniqueProposal(contents, completion, replacementString)
proposal.replacementString // instantiate the guesses

ICompletionProposal[][] choices = proposal.choices
assertEquals(expectedChoices.length, choices.length)
for (int i = 0; i < expectedChoices.length; i++) {
for (int i = 0; i < expectedChoices.length; i += 1) {
assertEquals(expectedChoices[i].length, choices[i].length)

// fix order for comparison
expectedChoices[i].sort(true)
choices[i].sort(true) { ICompletionProposal p -> p.displayString }

for (int j = 0; j < expectedChoices[i].length; j++) {
for (int j = 0; j < expectedChoices[i].length; j += 1) {
assertEquals('unexpected choice', expectedChoices[i][j], choices[i][j].displayString)
}
}
Expand Down
121 changes: 85 additions & 36 deletions ...tion.test/src/org/codehaus/groovy/eclipse/codeassist/tests/GuessingCompletionTests.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 @@ -16,6 +16,10 @@
package org.codehaus.groovy.eclipse.codeassist.tests

import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist
import org.eclipse.jface.text.Document
import org.eclipse.jface.text.IDocument
import org.eclipse.jface.text.contentassist.ICompletionProposal
import org.junit.Assert
import org.junit.Before
import org.junit.Test

Expand All @@ -30,41 +34,45 @@ final class GuessingCompletionTests extends CompletionTestSuite {

@Test
void testParamGuessing1() {
String contents = "String yyy\n" +
"def xxx(String x) { }\n" +
"xxx"
String[][] expectedChoices = [ [ "yyy", "\"\"" ] as String[] ].toArray()
checkProposalChoices(contents, "xxx", "xxx(yyy)", expectedChoices)
String contents = '''\
String yyy
def xxx(String x) { }
xxx
'''.stripIndent()
String[][] expectedChoices = [ [ 'yyy', '""' ] as String[] ]
checkProposalChoices(contents, 'xxx', 'xxx(yyy)', expectedChoices)
}

@Test
void testParamGuessing2() {
String contents =
"String yyy\n" +
"int zzz\n" +
"def xxx(String x, int z) { }\n" +
"xxx"
String contents = '''\
String yyy
int zzz
def xxx(String x, int z) { }
xxx
'''.stripIndent()
String[][] expectedChoices = [
[ "yyy", "\"\"" ] as String[],
[ "zzz", "0" ] as String[]
].toArray()
checkProposalChoices(contents, "xxx", "xxx(yyy, zzz)", expectedChoices)
[ 'yyy', '""' ] as String[],
[ 'zzz', '0' ] as String[]
]
checkProposalChoices(contents, 'xxx', 'xxx(yyy, zzz)', expectedChoices)
}

@Test
void testParamGuessing3() {
String contents =
"String yyy\n" +
"Integer zzz\n" +
"boolean aaa\n" +
"def xxx(String x, int z, boolean a) { }\n" +
"xxx"
String contents = '''\
String yyy
Integer zzz
boolean aaa
def xxx(String x, int z, boolean a) { }
xxx
'''.stripIndent()
String[][] expectedChoices = [
[ "yyy", "\"\"" ] as String[],
[ "zzz", "0" ] as String[],
[ "aaa", "false", "true" ] as String[]
].toArray()
checkProposalChoices(contents, "xxx", "xxx(yyy, zzz, aaa)", expectedChoices)
[ 'yyy', '""' ] as String[],
[ 'zzz', '0' ] as String[],
[ 'aaa', 'false', 'true' ] as String[]
]
checkProposalChoices(contents, 'xxx', 'xxx(yyy, zzz, aaa)', expectedChoices)
}

@Test // GRECLIPSE-1268
Expand All @@ -73,22 +81,63 @@ final class GuessingCompletionTests extends CompletionTestSuite {
// parameters is not based on actual source location. Need a way to map
// from variable name to local variable declaration in
// GroovyExtendedCompletionContext.computeVisibleElements(String)
String contents =
"Closure yyy\n" +
"def zzz = { }\n" +
"def xxx(Closure c) { }\n" +
"xxx"
String contents = '''\
Closure yyy
def zzz = { }
def xxx(Closure c) { }
xxx
'''.stripIndent()
String[][] expectedChoices = [
["zzz", "yyy", "{ }"] as String[]
].toArray()
['zzz', 'yyy', '{ }'] as String[]
]
try {
checkProposalChoices(contents, "xxx", "xxx {", expectedChoices)
checkProposalChoices(contents, 'xxx', 'xxx {', expectedChoices)
} catch (AssertionError e) {
try {
checkProposalChoices(contents, "xxx", "xxx yyy", expectedChoices)
checkProposalChoices(contents, 'xxx', 'xxx yyy', expectedChoices)
} catch (AssertionError e2) {
checkProposalChoices(contents, "xxx", "xxx zzz", expectedChoices)
checkProposalChoices(contents, 'xxx', 'xxx zzz', expectedChoices)
}
}
}

@Test
void testParamGuessing5() {
addGroovySource '''\
import java.util.concurrent.TimeUnit
class Util {
static void util(TimeUnit units) {
}
}
'''.stripIndent(), 'Util', 'pack'

String contents = '''\
|import static java.util.concurrent.TimeUnit.MILLISECONDS as MILLIS
|
|pack.Util.util
|'''.stripMargin()
IDocument document = new Document(contents)
ICompletionProposal proposal = checkUniqueProposal(contents, 'util', 'util(MILLIS)')

// apply initial proposal to generate parameter proposals
applyProposalAndCheck(document, proposal, '''\
|import static java.util.concurrent.TimeUnit.MILLISECONDS as MILLIS
|
|pack.Util.util(MILLIS)
|'''.stripMargin());

// check the parameter guesses
ICompletionProposal[] choices = proposal.choices[0]
Assert.assertEquals(['MILLIS', 'DAYS', 'TimeUnit.DAYS', 'HOURS', 'TimeUnit.HOURS', 'MINUTES', 'TimeUnit.MINUTES',
'SECONDS', 'TimeUnit.SECONDS', 'MILLISECONDS', 'TimeUnit.MILLISECONDS', 'MICROSECONDS', 'TimeUnit.MICROSECONDS',
'NANOSECONDS', 'TimeUnit.NANOSECONDS', 'null'].join('\n'), choices*.displayString.join('\n'))

// TODO: Something below is not matching the real editor's application of the parameter proposal
/*applyProposalAndCheck(document, choices[1], '''\
|import static java.util.concurrent.TimeUnit.DAYS
|import static java.util.concurrent.TimeUnit.MILLISECONDS as MILLIS
|
|pack.Util.util(DAYS)
|'''.stripMargin())*/
}
}
50 changes: 22 additions & 28 deletions ...c/org/codehaus/groovy/eclipse/codeassist/completions/GroovyExtendedCompletionContext.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 @@ -17,12 +17,10 @@

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.eclipse.codeassist.GroovyContentAssist;
import org.codehaus.groovy.eclipse.codeassist.ProposalUtils;
import org.codehaus.groovy.eclipse.codeassist.requestor.ContentAssistContext;
Expand Down Expand Up @@ -118,68 +116,64 @@ public IJavaElement[] getVisibleElements(String typeSignature) {
}

private IJavaElement[] computeVisibleElements(String typeSignature) {
Map<String, IJavaElement> visibleElements = new LinkedHashMap<>();
ClassNode targetType = toClassNode(typeSignature);
boolean isEnum = targetType.isEnum();

// look at all local variables in scope
Map<String, IJavaElement> nameElementMap = new LinkedHashMap<>();
// add qualifying local variables of the enclosing scope
if (currentScope != null) {
for (VariableInfo varInfo : currentScope) {
// GRECLIPSE-1268 currently, no good way to get to the actual declaration of the variable.
// This can cause ordering problems for the guessed parameters.
// don't put elements in a second time since we are moving from inner scope to outer scope
String varName = varInfo.name;
// ignore synthetic getters and setters that are put in the scope
// looking at prefix is a good approximation
// ignore synthetic getters and setters that are put in the scope; looking at prefix is an approximation
if (!varName.startsWith("get") &&
!varName.startsWith("set") &&
!varName.equals("this" ) &&
!varName.equals("super") &&
!varName.startsWith("<") &&
!nameElementMap.containsKey(varName)) {
!visibleElements.containsKey(varName)) {

ClassNode type = varInfo.type;
if (GroovyUtils.isAssignable(type, targetType)) {
// NOTE: parent, source location, typeSignature, etc. are not important here
int start = 0, until = varName.length() - 1;
nameElementMap.put(varName, new LocalVariable(
visibleElements.put(varName, new LocalVariable(
(JavaElement) getEnclosingElement(), varName, start, until, start, until, typeSignature, null, 0, false));
}
}
}
}

// now check fields
// add qualifying fields of the enclosing type(s)
IType enclosingType = (IType) getEnclosingElement().getAncestor(IJavaElement.TYPE);
if (enclosingType != null) {
try {
addFields(targetType, nameElementMap, enclosingType);
addFields(targetType, visibleElements, enclosingType);
ITypeHierarchy typeHierarchy = enclosingType.newSupertypeHierarchy(null);
IType[] allTypes = typeHierarchy.getAllSupertypes(enclosingType);
for (IType superType : allTypes) {
addFields(targetType, nameElementMap, superType);
for (IType superType : typeHierarchy.getAllSupertypes(enclosingType)) {
addFields(targetType, visibleElements, superType);
}
} catch (JavaModelException e) {
GroovyContentAssist.logError(e);
}
}

if (isEnum) {
IType targetIType = new GroovyProjectFacade(enclosingElement).groovyClassToJavaType(targetType);
List<FieldNode> fields = targetType.getFields();
for (FieldNode enumVal : fields) {
String name = enumVal.getName();
if (name.equals("MIN_VALUE") || name.equals("MAX_VALUE")) {
continue;
}
if (!enumVal.getType().equals(targetType)) {
continue;
// add enum constants if target type is an enum
if (targetType.isEnum()) {
try {
IType enumType = new GroovyProjectFacade(enclosingElement).groovyClassToJavaType(targetType);
for (IField enumField : enumType.getFields()) {
if (enumField.isEnumConstant()) {
visibleElements.put(enumField.getElementName(), enumField);
visibleElements.put(enumType.getElementName() + '.' + enumField.getElementName(), enumField);
}
}
nameElementMap.put(targetIType.getElementName() + "." + name, targetIType.getField(name));
nameElementMap.put(name, targetIType.getField(name));
} catch (JavaModelException e) {
GroovyContentAssist.logError(e);
}
}
return nameElementMap.values().toArray(new IJavaElement[0]);
return visibleElements.values().toArray(new IJavaElement[0]);
}

public void addFields(ClassNode targetType, Map<String, IJavaElement> nameElementMap, IType type)
Expand Down
Loading

0 comments on commit 016acc4

Please sign in to comment.