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)

Reply via email to