From 8565b17e6bb257168c4eb2babdfd63fa03b87036 Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Tue, 18 Dec 2012 15:56:51 +0400 Subject: [PATCH] IDEA-96810 "optimize imports" removes annotations on unused imports --- .../groovy/editor/GroovyImportOptimizer.java | 64 ++++++++++++++----- .../lang/psi/GroovyPsiElementFactory.java | 2 +- .../psi/impl/GroovyPsiElementFactoryImpl.java | 2 +- .../OptimizeImportsTest.groovy | 60 ++++++++++++++++- 4 files changed, 109 insertions(+), 19 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/editor/GroovyImportOptimizer.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/editor/GroovyImportOptimizer.java index c25a405cf99a..fc7bbffa2f3b 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/editor/GroovyImportOptimizer.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/editor/GroovyImportOptimizer.java @@ -29,6 +29,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.codeStyle.GroovyCodeStyleSettings; import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; +import org.jetbrains.plugins.groovy.lang.psi.GroovyElementVisitor; import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; @@ -73,7 +74,7 @@ public class GroovyImportOptimizer implements ImportOptimizer { @Nullable final Map annotations) { if (!(file instanceof GroovyFile)) return; - ((GroovyFile)file).accept(new PsiRecursiveElementWalkingVisitor() { + file.accept(new PsiRecursiveElementWalkingVisitor() { @Override public void visitElement(PsiElement element) { super.visitElement(element); @@ -135,13 +136,6 @@ public class GroovyImportOptimizer implements ImportOptimizer { final String importRef = getImportReferenceText(importStatement); - if (annotations != null) { - if (isAnnotatedImport(importStatement)) { - annotations.put(importRef, importStatement.getAnnotationList().getText()); - } - } - - if (importStatement.isAliasedImport()) { if (aliased != null) { aliased.put(importRef, importedName); @@ -197,6 +191,19 @@ public class GroovyImportOptimizer implements ImportOptimizer { return true; } }); + + if (annotations != null) { + ((GroovyFile)file).acceptChildren(new GroovyElementVisitor() { + @Override + public void visitImportStatement(GrImportStatement importStatement) { + final String annotationText = importStatement.getAnnotationList().getText(); + if (!StringUtil.isEmptyOrSpaces(annotationText)) { + final String importRef = getImportReferenceText(importStatement); + annotations.put(importRef, annotationText); + } + } + }); + } } @Nullable @@ -264,11 +271,13 @@ public class GroovyImportOptimizer implements ImportOptimizer { tempFile.addImport(newImport); } - final int startOffset = oldImports.get(0).getTextRange().getStartOffset(); - final int endOffset = oldImports.get(oldImports.size() - 1).getTextRange().getEndOffset(); - String oldText = oldImports.isEmpty() ? "" : myFile.getText().substring(startOffset, endOffset); - if (tempFile.getText().trim().equals(oldText)) { - return; + if (oldImports.size() > 0) { + final int startOffset = oldImports.get(0).getTextRange().getStartOffset(); + final int endOffset = oldImports.get(oldImports.size() - 1).getTextRange().getEndOffset(); + String oldText = oldImports.isEmpty() ? "" : myFile.getText().substring(startOffset, endOffset); + if (tempFile.getText().trim().equals(oldText)) { + return; + } } for (GrImportStatement statement : tempFile.getImportStatements()) { @@ -295,6 +304,7 @@ public class GroovyImportOptimizer implements ImportOptimizer { TObjectIntHashMap packageCountMap = new TObjectIntHashMap(); TObjectIntHashMap classCountMap = new TObjectIntHashMap(); + //init packageCountMap for (String importedClass : importedClasses) { if (implicitlyImported.contains(importedClass) || innerClasses.contains(importedClass) || @@ -309,6 +319,7 @@ public class GroovyImportOptimizer implements ImportOptimizer { packageCountMap.increment(packageName); } + //init classCountMap for (String importedMember : staticallyImportedMembers) { if (aliased.containsKey(importedMember) || annotations.containsKey(importedMember)) continue; @@ -320,11 +331,12 @@ public class GroovyImportOptimizer implements ImportOptimizer { final Set onDemandImportedSimpleClassNames = new HashSet(); final List result = new ArrayList(); + packageCountMap.forEachEntry(new TObjectIntProcedure() { public boolean execute(String s, int i) { if (i >= settings.CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND || settings.PACKAGES_TO_USE_IMPORT_ON_DEMAND.contains(s)) { final GrImportStatement imp = factory.createImportStatementFromText(s, false, true, null); - String annos = annotations.get(s + ".*"); + String annos = annotations.remove(s + ".*"); if (annos != null) { imp.getAnnotationList().replace(factory.createModifierList(annos)); } @@ -344,7 +356,7 @@ public class GroovyImportOptimizer implements ImportOptimizer { public boolean execute(String s, int i) { if (i >= settings.NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND) { final GrImportStatement imp = factory.createImportStatementFromText(s, true, true, null); - String annos = annotations.get(s + ".*"); + String annos = annotations.remove(s + ".*"); if (annos != null) { imp.getAnnotationList().replace(factory.createModifierList(annos)); } @@ -369,7 +381,7 @@ public class GroovyImportOptimizer implements ImportOptimizer { } final GrImportStatement imp = factory.createImportStatementFromText(importedClass, false, false, null); - String annos = annotations.get(importedClass); + String annos = annotations.remove(importedClass); if (annos != null) { imp.getAnnotationList().replace(factory.createModifierList(annos)); } @@ -386,6 +398,10 @@ public class GroovyImportOptimizer implements ImportOptimizer { for (GrImportStatement anImport : usedImports) { if (anImport.isAliasedImport() || isAnnotatedImport(anImport)) { + if (isAnnotatedImport(anImport)) { + annotations.remove(getImportReferenceText(anImport)); + } + if (anImport.isStatic()) { result.add(anImport); } @@ -400,6 +416,22 @@ public class GroovyImportOptimizer implements ImportOptimizer { Collections.sort(explicated, comparator); explicated.addAll(result); + + if (!annotations.isEmpty()) { + StringBuilder allSkippedAnnotations = new StringBuilder(); + for (String anno : annotations.values()) { + allSkippedAnnotations.append(anno).append(' '); + } + if (explicated.isEmpty()) { + explicated.add(factory.createImportStatementFromText(CommonClassNames.JAVA_LANG_OBJECT, false, false, null)); + } + + final GrImportStatement first = explicated.get(0); + + allSkippedAnnotations.append(first.getAnnotationList().getText()); + first.getAnnotationList().replace(factory.createModifierList(allSkippedAnnotations)); + } + return explicated.toArray(new GrImportStatement[explicated.size()]); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GroovyPsiElementFactory.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GroovyPsiElementFactory.java index 348a58fa70cb..5ca9f8421fba 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GroovyPsiElementFactory.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GroovyPsiElementFactory.java @@ -69,7 +69,7 @@ public abstract class GroovyPsiElementFactory implements JVMElementFactory { public abstract GrBlockStatement createBlockStatementFromText(String text, @Nullable PsiElement context); - public abstract GrModifierList createModifierList(String text); + public abstract GrModifierList createModifierList(CharSequence text); public abstract GrCaseSection createSwitchSection(String text); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java index 929d70b03822..cbc5103e2cc6 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GroovyPsiElementFactoryImpl.java @@ -702,7 +702,7 @@ public class GroovyPsiElementFactoryImpl extends GroovyPsiElementFactory { } @Override - public GrModifierList createModifierList(String text) { + public GrModifierList createModifierList(CharSequence text) { final GrMethod method = createMethodFromText(text + " void foo()"); return method.getModifierList(); } 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 87dfb21fc912..b3d5697c33f7 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 @@ -1,5 +1,5 @@ /* - * Copyright 2000-2011 JetBrains s.r.o. + * Copyright 2000-2012 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -301,4 +301,62 @@ aliased2() } + void testAnnotationOnUnusedImport1() { + myFixture.addClass('package groovyx.gpars; public class GParsPool{}') + myFixture.addClass('package groovyx.gpars; public class GParsExecutorsPool{}') + + myFixture.configureByText('_.groovy', '''\ +@Grab(group='org.codehaus.gpars', module='gpars', version='0.12') +import groovyx.gpars.GParsPool +import groovyx.gpars.GParsExecutorsPool + +GParsExecutorsPool oi +''') + doOptimizeImports() + + myFixture.checkResult('''\ +@Grab(group = 'org.codehaus.gpars', module = 'gpars', version = '0.12') +import groovyx.gpars.GParsExecutorsPool + +GParsExecutorsPool oi +''') + } + + void testAnnotationOnUnusedImport2() { + myFixture.addClass('package groovyx.gpars; public class GParsPool{}') + + myFixture.configureByText('_.groovy', '''\ +@Grab(group='org.codehaus.gpars', module='gpars', version='0.12') +import groovyx.gpars.GParsPool +''') + doOptimizeImports() + + myFixture.checkResult('''\ +@Grab(group = 'org.codehaus.gpars', module = 'gpars', version = '0.12') +import java.lang.Object +''') + } + + void testAnnotationOnUnusedImport3() { + myFixture.addClass('package groovyx.gpars; public class GParsPool{}') + myFixture.addClass('package groovyx.gpars; public class GParsExecutorsPool{}') + + myFixture.configureByText('_.groovy', '''\ +@Grab(group='org.codehaus.gpars', module='gpars', version='0.12') +@Grab(group='org.codehaus.gpars', module='gpars', version='0.12') +import groovyx.gpars.GParsPool +import groovyx.gpars.GParsExecutorsPool + +GParsExecutorsPool oi +''') + doOptimizeImports() + + myFixture.checkResult('''\ +@Grab(group = 'org.codehaus.gpars', module = 'gpars', version = '0.12') @Grab(group = 'org.codehaus.gpars', module = 'gpars', version = '0.12') +import groovyx.gpars.GParsExecutorsPool + +GParsExecutorsPool oi +''') + } + }