Fix for #1354: keep imports required for disambiguation of default stars

groovy · Apr 1, 2022 · e84d3d9 · e84d3d9
1 parent 01168e2
commit e84d3d9
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 22 deletions.
diff --git a/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTests.groovy b/...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/actions/OrganizeImportsTests.groovy
@@ -1656,13 +1656,13 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
     void testDefaultImport1() {
         String originalContents = '''\
             |import java.util.List
-            |import groovy.util.Proxy
-            |List
-            |Proxy
+            |import groovy.util.Node
+            |List list
+            |Node node
             |'''
         String expectedContents = '''\
-            |List
-            |Proxy
+            |List list
+            |Node node
             |'''
         doContentsCompareTest(originalContents, expectedContents)
     }
@@ -1672,12 +1672,12 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
         String originalContents = '''\
             |import java.util.*
             |import groovy.util.*
-            |List
-            |Proxy
+            |List list
+            |Node node
             |'''
         String expectedContents = '''\
-            |List
-            |Proxy
+            |List list
+            |Node node
             |'''
         doContentsCompareTest(originalContents, expectedContents)
     }
@@ -1687,23 +1687,28 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
         String originalContents = '''\
             |import java.math.BigDecimal
             |import java.math.BigInteger
-            |BigDecimal
-            |BigInteger
+            |BigDecimal bd
+            |BigInteger bi
             |'''
         String expectedContents = '''\
-            |BigDecimal
-            |BigInteger
+            |BigDecimal bd
+            |BigInteger bi
             |'''
         doContentsCompareTest(originalContents, expectedContents)
     }
 
-    @Test // GRECLIPSE-1392
+    @Test
     void testDefaultImport5() {
-        String contents = '''\
-            |import static java.util.Collections.swap
-            |swap
+        String originalContents = '''\
+            |import java.awt.*
+            |import java.util.*
+            |import java.util.List
+            |List list
             |'''
-        doContentsCompareTest(contents)
+        String expectedContents = '''\
+            |List list
+            |'''
+        doContentsCompareTest(originalContents, expectedContents)
     }
 
     @Test
@@ -1712,10 +1717,13 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
             |import java.awt.*
             |import java.util.*
             |import java.util.List
-            |
+            |Font font
             |List list
             |'''
         String expectedContents = '''\
+            |import java.awt.*
+            |import java.util.List
+            |Font font
             |List list
             |'''
         doContentsCompareTest(originalContents, expectedContents)
@@ -1727,18 +1735,36 @@ final class OrganizeImportsTests extends OrganizeImportsTestSuite {
             |import java.sql.*
             |import java.util.*
             |import java.sql.Date
-            |
             |Date date
             |'''
         String expectedContents = '''\
             |import java.sql.*
             |import java.sql.Date
-            |
             |Date date
             |'''
         doContentsCompareTest(originalContents, expectedContents)
     }
 
+    @Test // https://github.com/groovy/groovy-eclipse/issues/1354
+    void testDefaultImport8() {
+        ['groovy.util', 'java.net', 'java.lang.reflect'].each {
+            String contents = """\
+                |import ${it}.Proxy
+                |Proxy proxy
+                |"""
+            doContentsCompareTest(contents)
+        }
+    }
+
+    @Test
+    void testDefaultImport9() {
+        String contents = '''\
+            |import static java.util.Collections.swap
+            |swap
+            |'''
+        doContentsCompareTest(contents)
+    }
+
     @Test // GRECLIPSE-1553
     void testCompileStaticAndMapStyleConstructor() {
         createGroovyType 'example2', 'Bar', '''\

diff --git a/...eclipse.ui/src/org/codehaus/groovy/eclipse/refactoring/actions/OrganizeGroovyImports.java b/...eclipse.ui/src/org/codehaus/groovy/eclipse/refactoring/actions/OrganizeGroovyImports.java
@@ -51,6 +51,7 @@
 import org.codehaus.groovy.ast.expr.PropertyExpression;
 import org.codehaus.groovy.ast.expr.StaticMethodCallExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.control.ResolveVisitor;
 import org.codehaus.groovy.eclipse.GroovyLogManager;
 import org.codehaus.groovy.eclipse.GroovyPlugin;
 import org.codehaus.groovy.eclipse.TraceCategory;
@@ -135,11 +136,15 @@ public TextEdit calculateMissingImports() {
             importsSlatedForRemoval = new HashMap<>();
 
             try {
+                Iterable<ImportNode> allImports = GroovyUtils.getAllImportNodes(info.module);
                 // Configure the import rewriter to keep all existing imports. This is different from how
                 // JDT does organize imports, but this prevents annotations on imports from being removed.
                 // However, this leads to GRECLIPSE-1390 where imports are no longer reordered and sorted.
-                Iterable<ImportNode> allImports = GroovyUtils.getAllImportNodes(info.module);
                 ImportRewrite rewriter = CodeStyleConfiguration.createImportRewrite(unit, !isSafeToReorganize(allImports));
+                rewriter.setUseContextToFilterImplicitImports(true);
+                for (String p : ResolveVisitor.DEFAULT_IMPORTS) {
+                    rewriter.addImport(p.concat("*"));
+                }
 
                 for (ImportNode imp : allImports) {
                     if (imp.isStar()) {