GROOVY-7948: fix completion of static imports in groovysh (closes #438)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/0bd39f8c Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/0bd39f8c Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/0bd39f8c Branch: refs/heads/parrot Commit: 0bd39f8c9c226ed99f770cce4a8b922a646c54b6 Parents: 3891fce Author: Abraham Grief <abraham.gr...@gmail.com> Authored: Wed Oct 5 13:32:43 2016 -0500 Committer: John Wagenleitner <jwagenleit...@apache.org> Committed: Sat Oct 15 22:28:11 2016 -0700 ---------------------------------------------------------------------- .../codehaus/groovy/tools/shell/Groovysh.groovy | 1 + .../completion/ImportsSyntaxCompletor.groovy | 73 +++++++++----------- .../ImportsSyntaxCompletorTest.groovy | 50 +++++++------- 3 files changed, 57 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/0bd39f8c/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Groovysh.groovy ---------------------------------------------------------------------- diff --git a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Groovysh.groovy b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Groovysh.groovy index cf68a98..356b7fd 100644 --- a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Groovysh.groovy +++ b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Groovysh.groovy @@ -75,6 +75,7 @@ class Groovysh extends Shell { final Interpreter interp + // individual imports are stored without leading 'import ' or trailing ';' final List<String> imports = [] int indentSize = 2 http://git-wip-us.apache.org/repos/asf/groovy/blob/0bd39f8c/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy ---------------------------------------------------------------------- diff --git a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy index cb27c57..cb446a6 100644 --- a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy +++ b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletor.groovy @@ -31,7 +31,11 @@ class ImportsSyntaxCompletor implements IdentifierCompletor { // cache for all preimported classes List<String> preimportedClassNames // cache for all manually imported classes - final Map<String, Collection<String>> cachedImports = new HashMap<String, Collection<String>>() + final Map<String, Collection<String>> cachedImports = new HashMap<String, Collection<String>>().withDefault {String key -> + Collection<String> matchingImports = new TreeSet<String>() + collectImportedSymbols(key, matchingImports) + matchingImports + } ImportsSyntaxCompletor(final Groovysh shell) { this.shell = shell @@ -41,27 +45,16 @@ class ImportsSyntaxCompletor implements IdentifierCompletor { boolean complete(final List<GroovySourceToken> tokens, final List<CharSequence> candidates) { String prefix = tokens.last().getText() boolean foundMatch = findMatchingPreImportedClasses(prefix, candidates) - for (String importName in shell.imports) { - foundMatch |= findMatchingImportedClassesCached(prefix, importName, candidates) + for (String importSpec in shell.imports) { + foundMatch |= findMatchingImportedClassesCached(prefix, importSpec, candidates) } return foundMatch } boolean findMatchingImportedClassesCached(final String prefix, final String importSpec, final List<String> candidates) { - Collection<String> cached - if (! cachedImports.containsKey(importSpec)) { - cached = new HashSet<String>() - collectImportedSymbols(importSpec, cached) - cachedImports.put(importSpec, cached) - } else { - cached = cachedImports.get(importSpec) - } - Collection<String> matches = cached.findAll({String it -> it.startsWith(prefix)}) - if (matches) { - candidates.addAll(matches) - return true - } - return false + candidates.addAll(cachedImports + .get(importSpec) + .findAll({String it -> it.startsWith(prefix)})) } boolean findMatchingPreImportedClasses(final String prefix, final Collection<String> matches) { @@ -87,14 +80,12 @@ class ImportsSyntaxCompletor implements IdentifierCompletor { return foundMatch } - private static final String STATIC_IMPORT_PATTERN = ~/^import static ([a-z0-9]+\.)+[A-Z][a-zA-Z0-9]*(\.(\*|[^.]+))?$/ + private static final String STATIC_IMPORT_PATTERN = ~/^static ([a-zA-Z_][a-zA-Z_0-9]*\.)+([a-zA-Z_][a-zA-Z_0-9]*|\*)$/ /** * finds matching imported classes or static methods - * @param prefix - * @param importSpec - * @param matches - * @return + * @param importSpec an import statement without the leading 'import ' or trailing semicolon + * @param matches all names matching the importSpec will be added to this Collection */ void collectImportedSymbols(final String importSpec, final Collection<String> matches) { String asKeyword = ' as ' @@ -104,34 +95,32 @@ class ImportsSyntaxCompletor implements IdentifierCompletor { matches << alias return } - String staticPrefix = 'import static ' + int lastDotIndex = importSpec.lastIndexOf('.') + String symbolName = importSpec.substring(lastDotIndex + 1) + String staticPrefix = 'static ' if (importSpec.startsWith(staticPrefix)) { // make sure pattern is safe, though shell should have done anyway if (importSpec.matches(STATIC_IMPORT_PATTERN)) { - String evalImportSpec - if (importSpec.endsWith('.*')) { - evalImportSpec = importSpec[staticPrefix.length()..-3] - } else { - evalImportSpec = importSpec[staticPrefix.length()..(importSpec.lastIndexOf('.') - 1)] - } - Class clazz = shell.interp.evaluate([evalImportSpec]) as Class + String className = importSpec.substring(staticPrefix.length(), lastDotIndex) + Class clazz = shell.interp.evaluate([className]) as Class if (clazz != null) { - Collection<String> members = ReflectionCompletor.getPublicFieldsAndMethods(clazz, '')*.value - for (member in members) { - matches.add(member) + List<String> clazzSymbols = ReflectionCompletor.getPublicFieldsAndMethods(clazz, '')*.value + List<String> importedSymbols; + if (symbolName == '*') { + importedSymbols = clazzSymbols; + } else { + Set<String> acceptableMatches = [symbolName, symbolName + '(', symbolName + '()'] + importedSymbols = acceptableMatches.intersect(clazzSymbols) } - } - } - } else if (importSpec.endsWith('*')) { - Set<String> packnames = shell.packageHelper.getContents(importSpec.substring('import '.length())) - if (packnames) { - for (String packName in packnames) { - matches << packName + matches.addAll(importedSymbols) } } } else { - String symbolname = importSpec.substring(importSpec.lastIndexOf('.') + 1) - matches << symbolname + if (symbolName == '*') { + matches.addAll(shell.packageHelper.getContents(importSpec)) + } else { + matches << symbolName + } } } } http://git-wip-us.apache.org/repos/asf/groovy/blob/0bd39f8c/subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletorTest.groovy ---------------------------------------------------------------------- diff --git a/subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletorTest.groovy b/subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletorTest.groovy index b3045ce..23dc902 100644 --- a/subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletorTest.groovy +++ b/subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/ImportsSyntaxCompletorTest.groovy @@ -26,12 +26,12 @@ import static org.codehaus.groovy.tools.shell.completion.TokenUtilTest.tokenList class ImportsSyntaxCompletorTest extends CompletorTestSupport { void testPackagePattern() { - assert 'import static java.lang.Math'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) - assert 'import static java.lang.Math.max'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) - assert 'import static java.lang.Math.max2'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) - assert 'import static java.lang.Math.*'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) - assert 'import static org.w3c.Math.*'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) - assert !('import static java lang'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN)) + assert 'static java.lang.Math'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) + assert 'static java.lang.Math.max'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) + assert 'static java.lang.Math.max2'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) + assert 'static java.lang.Math.*'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) + assert 'static org.w3c.Math.*'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN) + assert !('static java lang'.matches(ImportsSyntaxCompletor.STATIC_IMPORT_PATTERN)) } void testPreImported() { @@ -65,7 +65,7 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('A', 'import foo.lang.Bbb', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.Bbb', candidates) assert ['prefill'] == candidates } } @@ -75,7 +75,7 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('A', 'import foo.lang.Abcdef', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.Abcdef', candidates) assert ['prefill', 'Abcdef'] == candidates } } @@ -85,7 +85,7 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.collectImportedSymbols('import foo.lang.Xxxxx as Abcdef', candidates) + completor.collectImportedSymbols('foo.lang.Xxxxx as Abcdef', candidates) assert ['prefill', 'Abcdef'] == candidates } } @@ -97,10 +97,10 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('A', 'import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.*', candidates) assert ['prefill', 'Abcdef'] == candidates // test again without invoking packageHelper - completor.findMatchingImportedClassesCached('A', 'import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.*', candidates) assert ['prefill', 'Abcdef', 'Abcdef'] == candidates } } @@ -111,8 +111,8 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('ma', 'import static foo.lang.Math.max', candidates) - assert ['prefill', 'max('] == candidates + completor.findMatchingImportedClassesCached('log', 'static foo.lang.Math.log', candidates) + assert ['prefill', 'log('] == candidates } } @@ -122,8 +122,8 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('ma', 'import static foo.lang.Math.*', candidates) - assert ['prefill', 'max('] == candidates + completor.findMatchingImportedClassesCached('log', 'static foo.lang.Math.*', candidates) + assert ['prefill', 'log(', 'log10(', 'log1p('] == candidates } } @@ -132,7 +132,7 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('Y', 'import foo.lang.Abcdef as Xxxxxx', candidates) + completor.findMatchingImportedClassesCached('Y', 'foo.lang.Abcdef as Xxxxxx', candidates) assert ['prefill'] == candidates } } @@ -143,8 +143,8 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('ma', 'import static foo.lang.Math.max', candidates) - assert ['prefill', 'max('] == candidates + completor.findMatchingImportedClassesCached('log', 'static foo.lang.Math.log', candidates) + assert ['prefill', 'log('] == candidates } } @@ -154,9 +154,9 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('A', 'import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.*', candidates) assert ['prefill'] == candidates - completor.findMatchingImportedClassesCached('B', 'import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('B', 'foo.lang.*', candidates) assert ['prefill', 'Bbbb'] == candidates } } @@ -167,10 +167,10 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) def candidates = ['prefill'] - completor.findMatchingImportedClassesCached('A','import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('A','foo.lang.*', candidates) assert ['prefill', 'Abcdef'] == candidates // test again without invoking packageHelper - completor.findMatchingImportedClassesCached('A', 'import foo.lang.*', candidates) + completor.findMatchingImportedClassesCached('A', 'foo.lang.*', candidates) assert ['prefill', 'Abcdef', 'Abcdef'] == candidates } } @@ -190,11 +190,11 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { void testSimpleImport() { groovyshMocker.demand.getPackageHelper(6) { [getContents: {[]}] } - groovyshMocker.demand.getImports(1) { ['import xyzabc.*', 'import xxxx.*'] } + groovyshMocker.demand.getImports(1) { ['xyzabc.*', 'xxxx.*'] } groovyshMocker.demand.getPackageHelper(1) { [getContents: { ['Xyzabc']}] } groovyshMocker.demand.getPackageHelper(1) { [getContents: { ['Xyz123']}] } // second call - groovyshMocker.demand.getImports(2) { ['import xyzabc.*', 'import xxxx.*'] } + groovyshMocker.demand.getImports(2) { ['xyzabc.*', 'xxxx.*'] } groovyshMocker.use { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock) @@ -211,7 +211,7 @@ class ImportsSyntaxCompletorTest extends CompletorTestSupport { void testUnknownImport() { groovyshMocker.demand.getPackageHelper(6) { [getContents: {[]}] } - groovyshMocker.demand.getImports(1) { ['import xxxx'] } + groovyshMocker.demand.getImports(1) { ['xxxx'] } groovyshMocker.use { Groovysh groovyshMock = new Groovysh() ImportsSyntaxCompletor completor = new ImportsSyntaxCompletor(groovyshMock)