IDEA-77596 Groovy does not correctly order imports

plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/GroovyImportOptimizer.java

@@ -38,21 +38,12 @@ import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
 
 import java.util.*;
 
+import static org.jetbrains.plugins.groovy.lang.psi.impl.PsiImplUtil.isImportToJavaOrJavax;
+
 /**
  * @author ven
  */
 public class GroovyImportOptimizer implements ImportOptimizer {
-  private static final Comparator<GrImportStatement> IMPORT_STATEMENT_COMPARATOR = new Comparator<GrImportStatement>() {
-    public int compare(GrImportStatement statement1, GrImportStatement statement2) {
-      final GrCodeReferenceElement ref1 = statement1.getImportReference();
-      final GrCodeReferenceElement ref2 = statement2.getImportReference();
-      String name1 = ref1 != null ? PsiUtil.getQualifiedReferenceText(ref1) : null;
-      String name2 = ref2 != null ? PsiUtil.getQualifiedReferenceText(ref2) : null;
-      if (name1 == null) return name2 == null ? 0 : -1;
-      if (name2 == null) return 1;
-      return name1.compareTo(name2);
-    }
-  };
 
   @NotNull
   public Runnable processFile(PsiFile file) {
@@ -322,8 +313,30 @@ public class GroovyImportOptimizer implements ImportOptimizer {
         result.add(factory.createImportStatementFromText(importedMember, true, false, null));
       }
 
-      Collections.sort(result, IMPORT_STATEMENT_COMPARATOR);
-      Collections.sort(explicated, IMPORT_STATEMENT_COMPARATOR);
+      final Comparator<GrImportStatement> comparator = new Comparator<GrImportStatement>() {
+        public int compare(GrImportStatement statement1, GrImportStatement statement2) {
+          if (settings.LAYOUT_STATIC_IMPORTS_SEPARATELY) {
+            if (statement1.isStatic() && !statement2.isStatic()) return 1;
+            if (statement2.isStatic() && !statement1.isStatic()) return -1;
+          }
+
+          if (!statement1.isStatic() && !statement2.isStatic()) {
+            if (isImportToJavaOrJavax(statement1) && !isImportToJavaOrJavax(statement2)) return 1;
+            if (!isImportToJavaOrJavax(statement1) && isImportToJavaOrJavax(statement2)) return -1;
+          }
+
+
+          final GrCodeReferenceElement ref1 = statement1.getImportReference();
+          final GrCodeReferenceElement ref2 = statement2.getImportReference();
+          String name1 = ref1 != null ? PsiUtil.getQualifiedReferenceText(ref1) : null;
+          String name2 = ref2 != null ? PsiUtil.getQualifiedReferenceText(ref2) : null;
+          if (name1 == null) return name2 == null ? 0 : -1;
+          if (name2 == null) return 1;
+          return name1.compareTo(name2);
+        }
+      };
+      Collections.sort(result, comparator);
+      Collections.sort(explicated, comparator);
 
       explicated.addAll(result);
       return explicated.toArray(new GrImportStatement[explicated.size()]);

plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyFileImpl.java

@@ -23,6 +23,7 @@ import com.intellij.openapi.util.TextRange;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.CodeStyleManager;
+import com.intellij.psi.codeStyle.CodeStyleSettingsManager;
 import com.intellij.psi.impl.ElementBase;
 import com.intellij.psi.scope.DelegatingScopeProcessor;
 import com.intellij.psi.scope.PsiScopeProcessor;
@@ -64,6 +65,8 @@ import java.util.ArrayList;
 import java.util.LinkedHashSet;
 import java.util.List;
 
+import static org.jetbrains.plugins.groovy.lang.psi.impl.PsiImplUtil.isImportToJavaOrJavax;
+
 /**
  * Implements all abstractions related to Groovy file
  *
@@ -360,17 +363,13 @@ public class GroovyFileImpl extends GroovyFileBaseImpl implements GroovyFile {
     PsiElement anchor = getAnchorToInsertImportAfter();
     final PsiElement result = addAfter(statement, anchor);
 
-    boolean isAliasedImport = false;
-    if (anchor instanceof GrImportStatement) {
-      isAliasedImport = !((GrImportStatement)anchor).isAliasedImport() && statement.isAliasedImport() ||
-                        ((GrImportStatement)anchor).isAliasedImport() && !statement.isAliasedImport();
-    }
-
     if (anchor != null) {
       int lineFeedCount = 0;
-      if (!(anchor instanceof GrImportStatement) || isAliasedImport) {
+
+      if (isAddLineFeed(statement, anchor)) {
         lineFeedCount++;
       }
+      
       final PsiElement prev = result.getPrevSibling();
       if (prev instanceof PsiWhiteSpace) {
         lineFeedCount += StringUtil.getOccurenceCount(prev.getText(), '\n');
@@ -394,6 +393,33 @@ public class GroovyFileImpl extends GroovyFileBaseImpl implements GroovyFile {
     return importStatement;
   }
 
+  private static boolean isAddLineFeed(GrImportStatement statement, PsiElement anchor) {
+    if (!(anchor instanceof GrImportStatement)) {
+      return true;
+    }
+
+    final GrImportStatement _anchor = (GrImportStatement)anchor;
+
+    //aliases
+    if (statement.isAliasedImport() || _anchor.isAliasedImport()) {
+      return _anchor.isAliasedImport() ^ statement.isAliasedImport();
+    }
+
+    //static imports
+    if (CodeStyleSettingsManager.getSettings(statement.getProject()).LAYOUT_STATIC_IMPORTS_SEPARATELY) {
+      if (statement.isStatic() || _anchor.isStatic()) {
+        return statement.isStatic() ^ _anchor.isStatic();
+      }
+    }
+
+    //imports to std lib
+    if (isImportToJavaOrJavax(statement) || isImportToJavaOrJavax(_anchor)) {
+      return isImportToJavaOrJavax(statement) ^ isImportToJavaOrJavax(_anchor);
+    }
+
+    return false;
+  }
+
   public boolean isScript() {
     final StubElement stub = getStub();
     if (stub instanceof GrFileStub) {

plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java

@@ -65,6 +65,8 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.params.GrParameter;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrGdkMethod;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrReflectedMethod;
+import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.imports.GrImportStatement;
+import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement;
 import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeElement;
 import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
 import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.arithmetic.GrAdditiveExpressionImpl;
@@ -666,4 +668,12 @@ public class PsiImplUtil {
 
     return type;
   }
+  
+  public static boolean isImportToJavaOrJavax(GrImportStatement statement) {
+    final GrCodeReferenceElement ref = statement.getImportReference();
+    if (ref==null) return false;
+    final String text = ref.getText();
+    return text.startsWith("java.") || text.startsWith("javax.");
+  } 
+  
 }

plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/optimizeImports/OptimizeImportsTest.groovy

@@ -37,7 +37,7 @@ public class OptimizeImportsTest extends LightCodeInsightFixtureTestCase {
 
   @Override
   protected String getBasePath() {
-    return TestUtils.getTestDataPath() + "optimizeImports/";
+    return "${TestUtils.testDataPath}optimizeImports/";
   }
 
   @Override protected void setUp() {
@@ -201,12 +201,12 @@ class Fooxx <caret>{
 
   private void doOptimizeImports() {
     GroovyImportOptimizer optimizer = new GroovyImportOptimizer();
-    final Runnable runnable = optimizer.processFile(myFixture.getFile());
+    final Runnable runnable = optimizer.processFile(myFixture.file);
 
-    CommandProcessor.getInstance().executeCommand(getProject(), new Runnable() {
+    CommandProcessor.instance.executeCommand(project, new Runnable() {
       @Override
       public void run() {
-        ApplicationManager.getApplication().runWriteAction(runnable);
+        ApplicationManager.application.runWriteAction(runnable);
       }
     }, "Optimize imports", null);
   }
@@ -241,6 +241,67 @@ class Fooxx <caret>{
     myFixture.checkResultByFile(getTestName(false) + "_after.groovy");
   }
 
+  public void testSorting() {
+    myFixture.addClass("package foo; public class Foo{}");
+    myFixture.addClass("package foo; public class Bar{public static void foo0(){}}");
+    myFixture.addClass("package java.test; public class Test{public static void foo(){}}");
+    myFixture.addClass("package java.test2; public class Test2{public static void foo2(){}}");
+    myFixture.addClass("package test; public class Alias{public static void test(){}}")
+    myFixture.addClass("package test; public class Alias2{public static void test2(){}}")
 
+    myFixture.configureByText('__a.groovy', '''
+package pack
+
+
+import foo.Foo
+import foo.Bar
+import java.test.Test 
+import java.test2.Test2
+import static foo.Bar.foo0
+import static java.test.Test.foo
+import static java.test2.Test2.*
+import static test.Alias.test as aliased
+import static test.Alias2.test2 as aliased2
+
+new Foo()
+new Bar()
+new Test()
+new Test2()
+foo()
+foo0()
+foo1()
+aliased()
+aliased2()
+''')
+
+    doOptimizeImports()
+
+    myFixture.checkResult('''
+package pack
+
+import static test.Alias.test as aliased
+import static test.Alias2.test2 as aliased2
+
+import foo.Bar
+import foo.Foo
+
+import java.test.Test
+import java.test2.Test2
+
+import static foo.Bar.foo0
+import static java.test.Test.foo
+
+new Foo()
+new Bar()
+new Test()
+new Test2()
+foo()
+foo0()
+foo1()
+aliased()
+aliased2()
+''')
+
+  }
 
 }