From 0c266e3333205653d2dbf06ed34b2457998ca004 Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Sun, 5 Feb 2012 15:13:57 +0400 Subject: [PATCH] IDEA-77596 Groovy does not correctly order imports --- .../lang/editor/GroovyImportOptimizer.java | 39 +++++++---- .../groovy/lang/psi/impl/GroovyFileImpl.java | 40 +++++++++-- .../groovy/lang/psi/impl/PsiImplUtil.java | 10 +++ .../OptimizeImportsTest.groovy | 69 +++++++++++++++++-- 4 files changed, 134 insertions(+), 24 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/GroovyImportOptimizer.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/GroovyImportOptimizer.java index 9a42f26ca86f..79ca6e3fcb10 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/editor/GroovyImportOptimizer.java +++ b/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 IMPORT_STATEMENT_COMPARATOR = new Comparator() { - 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 comparator = new Comparator() { + 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()]); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyFileImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyFileImpl.java index 5c119d2ef2d9..9b9a206c61aa 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyFileImpl.java +++ b/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) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java index 9b47c654a40d..45f97711df72 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java +++ b/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."); + } + } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/optimizeImports/OptimizeImportsTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/optimizeImports/OptimizeImportsTest.groovy index 1fcda5dfba14..aab4fbe077f3 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/optimizeImports/OptimizeImportsTest.groovy +++ b/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 { 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 { 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() +''') + + } }