Repository: square/anvil (kotlin)

Issue:
Conflicting imports are sometimes generated in Anvil 2.4.6+
Use case: When there is a simple name conflict between two imports and one of them is an inner class, the resulting alias that gets created for the inner class can conflict with other imports. So far the following snippet is the smallest reproducible use-case that mimics the sort of setup we noticed this issue in for assisted factories.

```
      package com.squareup.test
      
      import javax.inject.Inject
      
      class InjectClass @Inject constructor() {
        interface Factory {
          fun doSomething()
        }
      }

      class InjectClassFactory @Inject constructor(val factory: InjectClass.Factory)
```

We started seeing this issue in Anvil 2.4.6 after upgrading to KotlinPoet 1.13.0. I filed https://github.com/square/kotlinpoet/issues/1648 but it turns out this is a long-standing bug in Anvil that only coincidentally worked with previous versions of KotlinPoet. The bug is that when we convert `FqName`s to `ClassName`s, there are some situations like the one above where we incorrectly identify which parts of a fully qualified name are the package and which are simple names (e.g. in the case of a nested class). In the above code, this would result in us creating a `ClassName(package = "com.squareup.test.InjectClass", simpleNames = listOf("Factory"))` when it should be `ClassName(package = "com.squareup.test", simpleNames = listOf("InjectClass", "Factory")`

Pull request description:
…e, which would result in conflicting generated imports

Fixes #738.

Patch:
diff --git a/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/KotlinPoetUtils.kt b/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/KotlinPoetUtils.kt
index 0a0026ad1..d51e19794 100644
--- a/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/KotlinPoetUtils.kt
+++ b/compiler-utils/src/main/java/com/squareup/anvil/compiler/internal/KotlinPoetUtils.kt
@@ -47,17 +47,15 @@ private fun FqName.asClassNameOrNull(module: ModuleDescriptor): ClassName? {
     return ClassName.bestGuess(asString())
   }
 
-  for (index in (segments.size - 1) downTo 1) {
-    val packageSegments = segments.subList(0, index)
-    val classSegments = segments.subList(index, segments.size)
-
-    val validFqName = FqName.fromSegments(packageSegments + classSegments)
-      .canResolveFqName(module)
-
+  for (index in 1..segments.size) {
+    // Try to resolve each segment chunk starting from the base package segment
+    // e.g. try resolving 'com', then 'com.squareup', then 'com.squareup.Foo'
+    val validFqName = FqName.fromSegments(segments.subList(0, index)).canResolveFqName(module)
     if (validFqName) {
+      // If we found a valid name, it means the package is one segment back from the current index
       return ClassName(
-        packageName = packageSegments.joinToString(separator = "."),
-        simpleNames = classSegments
+        packageName = segments.subList(0, index - 1).joinToString(separator = "."),
+        simpleNames = segments.subList(index - 1, segments.size)
       )
     }
   }


Test patch:
diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
index 82795b7fd..9da66ec9d 100644
--- a/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
+++ b/compiler/src/test/java/com/squareup/anvil/compiler/dagger/InjectConstructorFactoryGeneratorTest.kt
@@ -97,6 +97,32 @@ public final class InjectClass_Factory implements Factory<InjectClass> {
     }
   }
 
+  /**
+   * Covers a bug that previously led to conflicting imports in the generated code:
+   * https://github.com/square/anvil/issues/738
+   */
+  @Test fun `a factory class is generated without conflicting imports`() {
+    compile(
+      """
+      package com.squareup.test
+      
+      import javax.inject.Inject
+      
+      class InjectClass @Inject constructor() {
+        interface Factory {
+          fun doSomething()
+        }
+      }
+
+      class InjectClassFactory @Inject constructor(val factory: InjectClass.Factory)
+      """
+    ) {
+      // Loading one of the classes is all that's necessary to verify no conflicting imports were
+      // generated
+      injectClass.factoryClass()
+    }
+  }
+
   @Test fun `a factory class is generated for an inject constructor with arguments`() {
     /*
 package com.squareup.test;