Imports for the same class should not be removed in case different aliases are used

Closes pinterest#1243

Author: Paul Dingemans

CHANGELOG.md

@@ -9,6 +9,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 - Fix false positive in rule spacing-between-declarations-with-annotations ([#1281](https://github.com/pinterest/ktlint/issues/1281))
+- Do not remove imports for same class when different alias is used ([#1243](https://github.com/pinterest/ktlint/issues/1243))
 
 ### Changed
 - Update Kotlin version to `1.6.0` release

ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/ImportOrderingRule.kt

@@ -169,13 +169,28 @@ public class ImportOrderingRule :
                     it.elementType == ElementType.IMPORT_DIRECTIVE ||
                         it.psi is PsiWhiteSpace && it.textLength > 1 // also collect empty lines, that are represented as "\n\n"
                 }
+
+                val autoCorrectDuplicateImports = imports
+                    .filter { it.psi !is PsiWhiteSpace }
+                    .groupingBy { it.text }
+                    .eachCount()
+                    .filterValues { it > 1 }
+                    .keys
+                    .map {
+                        emit(
+                            node.startOffset,
+                            "Duplicate '$it' found",
+                            true
+                        )
+                    }.any()
+                val deduplicatedImports = imports.distinctBy { it.text } // distinguish by import path including alias
+
                 val hasComments = children.find { it.elementType == ElementType.BLOCK_COMMENT || it.elementType == ElementType.EOL_COMMENT } != null
-                val sortedImports = imports
+                val sortedImports = deduplicatedImports
                     .asSequence()
                     .filter { it.psi !is PsiWhiteSpace } // sorter expects KtImportDirective, whitespaces are inserted afterwards
                     .map { it.psi as KtImportDirective }
                     .sortedWith(importSorter)
-                    .distinctBy { if (it.aliasName != null) it.text.substringBeforeLast(it.aliasName!!) else it.text } // distinguish by import path w/o aliases
                     .map { it.node } // transform back to ASTNode in order to operate over its method (addChild)
 
                 // insert blank lines wherever needed
@@ -201,19 +216,24 @@ public class ImportOrderingRule :
                     return@fold current
                 }
 
-                val canAutoCorrect = !hasComments
-                if (!importsAreEqual(imports, sortedImportsWithSpaces) || (hasTooMuchWhitespace(children) && !isCustomLayout())) {
-                    val additionalMessage = if (!canAutoCorrect) {
-                        " -- no autocorrection due to comments in the import list"
-                    } else {
-                        ""
-                    }
+                if (hasComments) {
                     emit(
                         node.startOffset,
-                        "${errorMessages.getOrDefault(importsLayout, CUSTOM_ERROR_MESSAGE)}$additionalMessage",
-                        canAutoCorrect
+                        errorMessages.getOrDefault(importsLayout, CUSTOM_ERROR_MESSAGE) +
+                            " -- no autocorrection due to comments in the import list",
+                        false
                     )
-                    if (autoCorrect && canAutoCorrect) {
+                } else {
+                    val autoCorrectWhitespace = hasTooMuchWhitespace(children) && !isCustomLayout()
+                    val autoCorrectSortOrder = !importsAreEqual(deduplicatedImports, sortedImportsWithSpaces)
+                    if (autoCorrectSortOrder || autoCorrectWhitespace) {
+                        emit(
+                            node.startOffset,
+                            errorMessages.getOrDefault(importsLayout, CUSTOM_ERROR_MESSAGE),
+                            true
+                        )
+                    }
+                    if (autoCorrect && (autoCorrectDuplicateImports || autoCorrectSortOrder || autoCorrectWhitespace)) {
                         node.removeRange(node.firstChildNode, node.lastChildNode.treeNext)
                         sortedImportsWithSpaces.reduce { current, next ->
                             node.addChild(current, null)

ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt

@@ -860,4 +860,18 @@ class NoUnusedImportsRuleTest {
             )
         )
     }
+
+    @Test
+    fun `Issue 1243 - Format should not remove import`() {
+        val code =
+            """
+            import foo.Bar as Bar1
+            import foo.Bar as Bar2
+
+            val bar1 = Bar1()
+            val bar2 = Bar2()
+            """.trimIndent()
+        assertThat(NoUnusedImportsRule().lint(code)).isEmpty()
+        assertThat(NoUnusedImportsRule().format(code)).isEqualTo(code)
+    }
 }

ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/importordering/ImportOrderingRuleIdeaTest.kt

@@ -125,28 +125,33 @@ class ImportOrderingRuleIdeaTest {
     }
 
     @Test
-    fun testFormatDuplicate() {
+    fun `remove duplicate class imports unless they have distinct aliases`() {
         val imports =
             """
-            import android.view.ViewGroup
             import android.view.View
             import android.view.ViewGroup
+            import android.view.View
             import android.content.Context as Ctx1
             import android.content.Context as Ctx2
+            import android.content.Context as Ctx1
             """.trimIndent()
 
         val formattedImports =
             """
             import android.view.View
             import android.view.ViewGroup
             import android.content.Context as Ctx1
+            import android.content.Context as Ctx2
             """.trimIndent()
 
         val testFile = writeIdeaImportsOrderingConfig()
 
         assertThat(
             rule.lint(testFile, imports)
-        ).isEqualTo(expectedErrors)
+        ).containsExactly(
+            LintError(1, 1, "import-ordering", "Duplicate 'import android.view.View' found"),
+            LintError(1, 1, "import-ordering", "Duplicate 'import android.content.Context as Ctx1' found")
+        )
         assertThat(
             rule.format(testFile, imports)
         ).isEqualTo(formattedImports)
@@ -350,6 +355,38 @@ class ImportOrderingRuleIdeaTest {
         ).isEqualTo(formattedImports)
     }
 
+    @Test
+    fun `Issue 1243 - Format should not remove imports when having a distinct alias`() {
+        val imports =
+            """
+            import foo.Bar as Bar1
+            import foo.Bar as Bar2
+            import foo.Bar as Bar2
+
+            val bar1 = Bar1()
+            val bar2 = Bar2()
+            """.trimIndent()
+        val formattedImports =
+            """
+            import foo.Bar as Bar1
+            import foo.Bar as Bar2
+
+            val bar1 = Bar1()
+            val bar2 = Bar2()
+            """.trimIndent()
+
+        val testFile = writeIdeaImportsOrderingConfig()
+
+        assertThat(
+            rule.lint(testFile, imports)
+        ).containsExactly(
+            LintError(1, 1, "import-ordering", "Duplicate 'import foo.Bar as Bar2' found")
+        )
+        assertThat(
+            rule.format(testFile, imports)
+        ).isEqualTo(formattedImports)
+    }
+
     private fun writeIdeaImportsOrderingConfig() = editorConfigTestRule
         .writeToEditorConfig(
             mapOf(