Shorten References: Do not optimize imports until shortening is finished

#KT-5227 Fixed

annotations/com/intellij/codeInsight/annotations.xml

@@ -0,0 +1,5 @@
+<root>
+  <item name='com.intellij.codeInsight.CodeInsightSettings com.intellij.codeInsight.CodeInsightSettings getInstance()'>
+    <annotation name='org.jetbrains.annotations.NotNull'/>
+  </item>
+</root>

idea/src/org/jetbrains/jet/plugin/codeInsight/ShortenReferences.kt

@@ -102,6 +102,8 @@ public object ShortenReferences {
 
     private fun process(elements: Iterable<JetElement>, elementFilter: (PsiElement) -> FilterResult) {
         for ((file, fileElements) in elements.groupBy { element -> element.getContainingJetFile() }) {
+            ImportInsertHelper.optimizeImportsIfNeeded(file)
+
             // first resolve all qualified references - optimization
             val referenceToContext = JetFileReferencesResolver.resolve(file, fileElements, visitShortNames = false)
 
@@ -348,6 +350,6 @@ public object ShortenReferences {
 
     //TODO: do we need this "IfNeeded" check?
     private fun addImportIfNeeded(descriptor: DeclarationDescriptor, file: JetFile) {
-        ImportInsertHelper.addImportDirectiveIfNeeded(DescriptorUtils.getFqNameSafe(descriptor), file)
+        ImportInsertHelper.addImportDirectiveIfNeeded(DescriptorUtils.getFqNameSafe(descriptor), file, false)
     }
 }

idea/src/org/jetbrains/jet/plugin/quickfix/ImportInsertHelper.java

@@ -43,12 +43,21 @@ public class ImportInsertHelper {
      *
      * @param importFqn full name of the import
      * @param file File where directive should be added.
+     * @param optimize Optimize existing imports before adding new one.
      */
-    public static void addImportDirectiveIfNeeded(@NotNull FqName importFqn, @NotNull JetFile file) {
+    public static void addImportDirectiveIfNeeded(@NotNull FqName importFqn, @NotNull JetFile file, boolean optimize) {
-        addImportDirectiveIfNeeded(new ImportPath(importFqn, false), file);
+        addImportDirectiveIfNeeded(new ImportPath(importFqn, false), file, optimize);
     }
 
-    public static void addImportDirectiveOrChangeToFqName(@NotNull FqName importFqn, @NotNull JetFile file, int refOffset, @NotNull PsiElement targetElement) {
+    public static void addImportDirectiveIfNeeded(@NotNull FqName importFqn, @NotNull JetFile file) {
+        addImportDirectiveIfNeeded(importFqn, file, true);
+    }
+
+    public static void addImportDirectiveOrChangeToFqName(
+            @NotNull FqName importFqn,
+            @NotNull JetFile file,
+            int refOffset,
+            @NotNull PsiElement targetElement) {
         PsiReference reference = file.findReferenceAt(refOffset);
         if (reference instanceof JetReference) {
             PsiElement target = reference.resolve();
@@ -80,12 +89,12 @@ public class ImportInsertHelper {
                 return;
             }
         }
-        addImportDirectiveIfNeeded(new ImportPath(importFqn, false), file);
+        addImportDirectiveIfNeeded(importFqn, file);
     }
 
-    public static void addImportDirectiveIfNeeded(@NotNull ImportPath importPath, @NotNull JetFile file) {
+    public static void addImportDirectiveIfNeeded(@NotNull ImportPath importPath, @NotNull JetFile file, boolean optimize) {
-        if (CodeInsightSettings.getInstance().OPTIMIZE_IMPORTS_ON_THE_FLY) {
+        if (optimize) {
-            new OptimizeImportsProcessor(file.getProject(), file).runWithoutProgress();
+            optimizeImportsIfNeeded(file);
         }
 
         if (!needImport(importPath, file)) {
@@ -95,6 +104,16 @@ public class ImportInsertHelper {
         writeImportToFile(importPath, file);
     }
 
+    public static void optimizeImportsIfNeeded(JetFile file) {
+        if (CodeInsightSettings.getInstance().OPTIMIZE_IMPORTS_ON_THE_FLY) {
+            optimizeImports(file);
+        }
+    }
+
+    public static void optimizeImports(JetFile file) {
+        new OptimizeImportsProcessor(file.getProject(), file).runWithoutProgress();
+    }
+
     public static void writeImportToFile(@NotNull ImportPath importPath, @NotNull JetFile file) {
         if (file instanceof JetCodeFragment) {
             JetImportDirective newDirective = JetPsiFactory.createImportDirective(file.getProject(), importPath);
@@ -105,6 +124,7 @@ public class ImportInsertHelper {
         JetImportList importList = file.getImportList();
         if (importList != null) {
             JetImportDirective newDirective = JetPsiFactory.createImportDirective(file.getProject(), importPath);
+            importList.add(JetPsiFactory.createNewLine(file.getProject()));
             importList.add(newDirective);
         }
         else {

idea/testData/shortenRefs/imports/leaveQualifiedConstructor.kt

@@ -0,0 +1,4 @@
+// OPTIMIZE_IMPORTS
+import java.util.Date
+
+val x = <selection>java.sql.Date(1)</selection>

idea/testData/shortenRefs/imports/leaveQualifiedConstructor.kt.after

@@ -0,0 +1,4 @@
+// OPTIMIZE_IMPORTS
+import java.sql.Date
+
+val x = Date(1)

idea/testData/shortenRefs/imports/leaveQualifiedType.kt

@@ -0,0 +1,4 @@
+// OPTIMIZE_IMPORTS
+import java.util.Date
+
+class A : <selection>java.sql.Date</selection>

idea/testData/shortenRefs/imports/leaveQualifiedType.kt.after

@@ -0,0 +1,4 @@
+// OPTIMIZE_IMPORTS
+import java.sql.Date
+
+class A : Date

idea/testData/shortenRefs/imports/optimizeMultipleImports.kt

@@ -0,0 +1,4 @@
+// OPTIMIZE_IMPORTS
+import java.io.*
+
+<selection>class A(val d: java.sql.Date, val rs: java.sql.ResultSet)</selection>

idea/testData/shortenRefs/imports/optimizeMultipleImports.kt.after

@@ -0,0 +1,5 @@
+// OPTIMIZE_IMPORTS
+import java.sql.Date
+import java.sql.ResultSet
+
+class A(val d: Date, val rs: ResultSet)

idea/tests/org/jetbrains/jet/shortenRefs/AbstractShortenRefsTest.kt

@@ -17,59 +17,55 @@
 package org.jetbrains.jet.shortenRefs
 
 import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase
-import org.jetbrains.jet.InTextDirectivesUtils
-import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor
-import org.jetbrains.jet.lang.psi.JetSimpleNameExpression
-import org.jetbrains.jet.lang.psi.JetExpression
-import org.jetbrains.jet.plugin.project.AnalyzerFacadeWithCache
 import org.jetbrains.jet.lang.psi.JetFile
-import org.jetbrains.jet.lang.resolve.lazy.ResolveSessionUtils
-import org.jetbrains.jet.lang.resolve.name.FqName
-import org.jetbrains.jet.plugin.project.TargetPlatform
 import org.jetbrains.jet.plugin.JetWithJdkAndRuntimeLightProjectDescriptor
-import org.jetbrains.jet.lang.resolve.name.Name
-import com.intellij.util.containers.Predicate
-import org.jetbrains.jet.lang.descriptors.ClassDescriptor
-import com.intellij.openapi.application.Application
 import com.intellij.openapi.application.ApplicationManager
-import org.jetbrains.jet.plugin.PluginTestCaseBase
 import java.io.File
 import com.intellij.openapi.command.CommandProcessor
-import com.intellij.psi.util.PsiTreeUtil
-import org.jetbrains.jet.lang.psi.JetReferenceExpression
-import com.intellij.psi.PsiElement
 import org.jetbrains.jet.plugin.codeInsight.ShortenReferences
-import org.jetbrains.jet.lang.psi.JetElement
 import org.jetbrains.jet.JetTestCaseBuilder
+import com.intellij.codeInsight.CodeInsightSettings
+import org.jetbrains.jet.InTextDirectivesUtils
 
 abstract class AbstractShortenRefsTest : LightCodeInsightFixtureTestCase() {
     override fun getTestDataPath() = JetTestCaseBuilder.getHomeDirectory()
     override fun getProjectDescriptor() = JetWithJdkAndRuntimeLightProjectDescriptor.INSTANCE
 
     protected fun doTest(testPath: String) {
-        val fixture = myFixture
+        val codeInsightSettings = CodeInsightSettings.getInstance()
-        val dependencyPath = testPath.replace(".kt", ".dependency.kt")
+        val optimizeImportsBefore = codeInsightSettings.OPTIMIZE_IMPORTS_ON_THE_FLY
-        if (File(dependencyPath).exists()) {
-            fixture.configureByFile(dependencyPath)
-        }
-        val javaDependencyPath = testPath.replace(".kt", ".dependency.java")
-        if (File(javaDependencyPath).exists()) {
-            fixture.configureByFile(javaDependencyPath)
-        }
 
-        fixture.configureByFile(testPath)
+        try {
-
+            val fixture = myFixture
-        val file = fixture.getFile() as JetFile
+            val dependencyPath = testPath.replace(".kt", ".dependency.kt")
-        val selectionModel = fixture.getEditor().getSelectionModel()
+            if (File(dependencyPath).exists()) {
-        if (!selectionModel.hasSelection()) error("No selection in input file")
+                fixture.configureByFile(dependencyPath)
-
+            }
-        CommandProcessor.getInstance().executeCommand(getProject(), {
+            val javaDependencyPath = testPath.replace(".kt", ".dependency.java")
-            ApplicationManager.getApplication()!!.runWriteAction {
+            if (File(javaDependencyPath).exists()) {
-                ShortenReferences.process(file, selectionModel.getSelectionStart(), selectionModel.getSelectionEnd())
+                fixture.configureByFile(javaDependencyPath)
             }
-        }, null, null)
-        selectionModel.removeSelection()
 
-        fixture.checkResultByFile(testPath + ".after")
+            fixture.configureByFile(testPath)
+
+            val file = fixture.getFile() as JetFile
+            val selectionModel = fixture.getEditor().getSelectionModel()
+            if (!selectionModel.hasSelection()) error("No selection in input file")
+
+            codeInsightSettings.OPTIMIZE_IMPORTS_ON_THE_FLY =
+                    InTextDirectivesUtils.isDirectiveDefined(file.getText(), "// OPTIMIZE_IMPORTS")
+
+            CommandProcessor.getInstance().executeCommand(getProject(), {
+                ApplicationManager.getApplication()!!.runWriteAction {
+                    ShortenReferences.process(file, selectionModel.getSelectionStart(), selectionModel.getSelectionEnd())
+                }
+            }, null, null)
+            selectionModel.removeSelection()
+
+            fixture.checkResultByFile(testPath + ".after")
+        }
+        finally {
+            codeInsightSettings.OPTIMIZE_IMPORTS_ON_THE_FLY = optimizeImportsBefore
+        }
     }
 }

idea/tests/org/jetbrains/jet/shortenRefs/ShortenRefsTestGenerated.java

@@ -31,7 +31,7 @@ import org.jetbrains.jet.shortenRefs.AbstractShortenRefsTest;
 /** This class is generated by {@link org.jetbrains.jet.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
 @SuppressWarnings("all")
 @TestMetadata("idea/testData/shortenRefs")
-@InnerTestClasses({ShortenRefsTestGenerated.Constructor.class, ShortenRefsTestGenerated.Java.class, ShortenRefsTestGenerated.Type.class})
+@InnerTestClasses({ShortenRefsTestGenerated.Constructor.class, ShortenRefsTestGenerated.Imports.class, ShortenRefsTestGenerated.Java.class, ShortenRefsTestGenerated.Type.class})
 public class ShortenRefsTestGenerated extends AbstractShortenRefsTest {
     public void testAllFilesPresentInShortenRefs() throws Exception {
         JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/shortenRefs"), Pattern.compile("^([^\\.]+)\\.kt$"), true);
@@ -130,6 +130,29 @@ public class ShortenRefsTestGenerated extends AbstractShortenRefsTest {
         
     }
     
+    @TestMetadata("idea/testData/shortenRefs/imports")
+    public static class Imports extends AbstractShortenRefsTest {
+        public void testAllFilesPresentInImports() throws Exception {
+            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/shortenRefs/imports"), Pattern.compile("^([^\\.]+)\\.kt$"), true);
+        }
+        
+        @TestMetadata("leaveQualifiedConstructor.kt")
+        public void testLeaveQualifiedConstructor() throws Exception {
+            doTest("idea/testData/shortenRefs/imports/leaveQualifiedConstructor.kt");
+        }
+        
+        @TestMetadata("leaveQualifiedType.kt")
+        public void testLeaveQualifiedType() throws Exception {
+            doTest("idea/testData/shortenRefs/imports/leaveQualifiedType.kt");
+        }
+        
+        @TestMetadata("optimizeMultipleImports.kt")
+        public void testOptimizeMultipleImports() throws Exception {
+            doTest("idea/testData/shortenRefs/imports/optimizeMultipleImports.kt");
+        }
+        
+    }
+    
     @TestMetadata("idea/testData/shortenRefs/java")
     public static class Java extends AbstractShortenRefsTest {
         public void testAllFilesPresentInJava() throws Exception {
@@ -275,6 +298,7 @@ public class ShortenRefsTestGenerated extends AbstractShortenRefsTest {
         TestSuite suite = new TestSuite("ShortenRefsTestGenerated");
         suite.addTestSuite(ShortenRefsTestGenerated.class);
         suite.addTestSuite(Constructor.class);
+        suite.addTestSuite(Imports.class);
         suite.addTestSuite(Java.class);
         suite.addTestSuite(Type.class);
         return suite;