Keep imports from the same package if the name is overloaded

facebook · Aug 24, 2023 · 76052ae · 76052ae
1 parent b6e5f35
commit 76052ae
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 16 deletions.
diff --git a/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt b/core/src/main/java/com/facebook/ktfmt/format/RedundantImportDetector.kt
@@ -112,11 +112,10 @@ internal class RedundantImportDetector(val enabled: Boolean) {
     importCleanUpCandidates =
         importList.imports
             .filter { import ->
+              val identifier = import.identifier ?: return@filter false
               import.isValidImport &&
-                  !import.isAllUnder &&
-                  import.identifier != null &&
-                  requireNotNull(import.identifier) !in OPERATORS &&
-                  !COMPONENT_OPERATOR_REGEX.matches(import.identifier.orEmpty())
+                  identifier !in OPERATORS &&
+                  !COMPONENT_OPERATOR_REGEX.matches(identifier)
             }
             .toSet()
 
@@ -160,20 +159,23 @@ internal class RedundantImportDetector(val enabled: Boolean) {
   fun getRedundantImportElements(): List<PsiElement> {
     if (!enabled) return emptyList()
 
-    val redundantImports = mutableListOf<PsiElement>()
-
-    // Collect unused imports
-    for (import in importCleanUpCandidates) {
-      val isUnused = import.aliasName !in usedReferences && import.identifier !in usedReferences
-      val isFromSamePackage = import.importedFqName?.parent() == thisPackage && import.alias == null
-      if (isUnused || isFromSamePackage) {
-        redundantImports += import
+    val identifierCounts =
+      importCleanUpCandidates
+        .groupBy { it.identifier }
+        .mapValues { it.value.size }
+
+    return importCleanUpCandidates
+      .filter{
+        val isUsed = it.identifier in usedReferences
+        val isFromThisPackage = it.importedFqName?.parent() == thisPackage
+        val hasAlias = it.alias != null
+        val isOverload = requireNotNull(identifierCounts[it.identifier]) > 1
+        // Remove if...
+        !isUsed || (isFromThisPackage && !hasAlias && !isOverload)
       }
-    }
-
-    return redundantImports
   }
 
+  /** The imported short name, possibly an alias name, if any. */
   private inline val KtImportDirective.identifier: String?
     get() = importPath?.importedName?.identifier?.trim('`')
 }
diff --git a/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt b/core/src/test/java/com/facebook/ktfmt/format/FormatterTest.kt
@@ -1179,7 +1179,7 @@ class FormatterTest {
   }
 
   @Test
-  fun `imports from the same package are removed`() {
+  fun `used imports from this package are removed`() {
     val code =
         """
       |package com.example
@@ -1208,6 +1208,96 @@ class FormatterTest {
     assertThatFormatting(code).isEqualTo(expected)
   }
 
+  @Test
+  fun `potentially unused imports from this package are kept if they are overloaded`() {
+    val code =
+        """
+      |package com.example
+      |
+      |import com.example.a
+      |import com.example.b
+      |import com.example.c
+      |import com.notexample.a
+      |import com.notexample.b
+      |import com.notexample.notC as c
+      |
+      |fun test() {
+      |  a("hello")
+      |  c("hello")
+      |}
+      |"""
+          .trimMargin()
+    val expected =
+        """
+      |package com.example
+      |
+      |import com.example.a
+      |import com.example.c
+      |import com.notexample.a
+      |import com.notexample.notC as c
+      |
+      |fun test() {
+      |  a("hello")
+      |  c("hello")
+      |}
+      |"""
+          .trimMargin()
+    assertThatFormatting(code).isEqualTo(expected)
+  }
+
+  @Test
+  fun `used imports from this package are kept if they are aliased`() {
+    val code =
+        """
+      |package com.example
+      |
+      |import com.example.b as a
+      |import com.example.c
+      |
+      |fun test() {
+      |  a("hello")
+      |}
+      |"""
+          .trimMargin()
+    val expected =
+        """
+      |package com.example
+      |
+      |import com.example.b as a
+      |
+      |fun test() {
+      |  a("hello")
+      |}
+      |"""
+          .trimMargin()
+    assertThatFormatting(code).isEqualTo(expected)
+  }
+
+  @Test
+  fun `unused imports are computed using only the alias name if present`() {
+    val code =
+        """
+      |package com.example
+      |
+      |import com.notexample.a as b
+      |
+      |fun test() {
+      |  a("hello")
+      |}
+      |"""
+          .trimMargin()
+    val expected =
+        """
+      |package com.example
+      |
+      |fun test() {
+      |  a("hello")
+      |}
+      |"""
+          .trimMargin()
+    assertThatFormatting(code).isEqualTo(expected)
+  }
+
   @Test
   fun `keep import elements only mentioned in kdoc`() {
     val code =