fix: improve nested class visibility handling (#25)

BedrockStreaming · Mar 3, 2023 · f2e9b1b · f2e9b1b
1 parent db4a440
commit f2e9b1b
Show file tree

Hide file tree

Showing 3 changed files with 309 additions and 3 deletions.
diff --git a/compiler-factory/src/main/java/toothpick/compiler/factory/generators/FactoryGenerator.kt b/compiler-factory/src/main/java/toothpick/compiler/factory/generators/FactoryGenerator.kt
@@ -50,7 +50,7 @@ import javax.inject.Singleton
  * See Optimistic creation of factories in TP wiki.
  */
 internal class FactoryGenerator(
-    private val constructorInjectionTarget: ConstructorInjectionTarget
+    private val constructorInjectionTarget: ConstructorInjectionTarget,
 ) : TPCodeGenerator {
 
     private val sourceClass: KSClassDeclaration = constructorInjectionTarget.sourceClass
@@ -63,7 +63,7 @@ internal class FactoryGenerator(
             packageName = sourceClassName.packageName,
             TypeSpec.classBuilder(generatedClassName)
                 .addOriginatingKSFile(sourceClass.containingFile!!)
-                .addModifiers(sourceClass.getVisibility().toKModifier() ?: KModifier.PUBLIC)
+                .addModifiers(getNestingAwareModifier() ?: KModifier.PUBLIC)
                 .addSuperinterface(
                     Factory::class.asClassName().parameterizedBy(sourceClassName)
                 )
@@ -85,6 +85,41 @@ internal class FactoryGenerator(
         )
     }
 
+    private fun getNestingAwareModifier(): KModifier? {
+        var parentDeclaration = sourceClass.parentDeclaration
+        var sourceModifier = sourceClass.getVisibility().toKModifier()
+
+        while (parentDeclaration != null) {
+            sourceModifier = findModifier(
+                parentDeclaration.getVisibility().toKModifier(),
+                sourceModifier
+            )
+            parentDeclaration = parentDeclaration.parentDeclaration
+        }
+
+        return sourceModifier
+    }
+
+    // With the private modifier, the program will not compile, but just in case, we process it
+    private fun findModifier(parentDeclarationModifier: KModifier?, sourceDeclarationModifier: KModifier?): KModifier? {
+        return when (parentDeclarationModifier) {
+            KModifier.INTERNAL -> {
+                when (sourceDeclarationModifier) {
+                    KModifier.PRIVATE, KModifier.PROTECTED -> sourceDeclarationModifier
+                    else -> parentDeclarationModifier
+                }
+            }
+            KModifier.PROTECTED -> {
+                when (sourceDeclarationModifier) {
+                    KModifier.PRIVATE -> sourceDeclarationModifier
+                    else -> parentDeclarationModifier
+                }
+            }
+            KModifier.PRIVATE -> parentDeclarationModifier
+            else -> sourceDeclarationModifier
+        }
+    }
+
     private fun TypeSpec.Builder.emitSuperMemberInjectorFieldIfNeeded() = apply {
         val superTypeThatNeedsInjection: ClassName =
             constructorInjectionTarget

diff --git a/compiler-factory/src/test/java/toothpick/compiler/factory/FactoryTest.kt b/compiler-factory/src/test/java/toothpick/compiler/factory/FactoryTest.kt
@@ -1790,4 +1790,275 @@ class FactoryTest {
                 "Parameter n in method/constructor test.TestPrimitiveConstructor#<init> is of type int which is not supported by Toothpick."
             )
     }
+
+    @Test
+    fun testInjectedConstructorForNestedClassWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                class NestedClass @Inject constructor()
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .compilesWithoutError()
+            .generatesSources(testNestedClassWithInternal_expected)
+    }
+
+    private val testNestedClassWithInternal_expected = expectedKtSource(
+        "test/TestNestedClassConstructor\$NestedClass__Factory",
+        """
+            package test
+            
+            import kotlin.Boolean
+            import kotlin.Suppress
+            import toothpick.Factory
+            import toothpick.Scope
+            
+            @Suppress(
+              "ClassName",
+              "RedundantVisibilityModifier",
+            )
+            internal class `TestNestedClassConstructor${'$'}NestedClass__Factory` :
+                Factory<TestNestedClassConstructor.NestedClass> {
+              public override fun createInstance(scope: Scope): TestNestedClassConstructor.NestedClass =
+                  TestNestedClassConstructor.NestedClass()
+            
+              public override fun getTargetScope(scope: Scope): Scope = scope
+            
+              public override fun hasScopeAnnotation(): Boolean = false
+            
+              public override fun hasSingletonAnnotation(): Boolean = false
+            
+              public override fun hasReleasableAnnotation(): Boolean = false
+            
+              public override fun hasProvidesSingletonAnnotation(): Boolean = false
+            
+              public override fun hasProvidesReleasableAnnotation(): Boolean = false
+            }
+            """
+    )
+
+    @Test
+    fun testInjectedConstructorForTwoNestedClassesWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                class NestedClass @Inject constructor() {
+                    class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .compilesWithoutError()
+            .generatesSources(testLastNestedClassWithInternal_expected)
+    }
+
+    private val testLastNestedClassWithInternal_expected = expectedKtSource(
+        "test/TestNestedClassConstructor\$NestedClass\$OneMoreNestedClass__Factory",
+        """
+            package test
+            
+            import kotlin.Boolean
+            import kotlin.Suppress
+            import toothpick.Factory
+            import toothpick.Scope
+            
+            @Suppress(
+              "ClassName",
+              "RedundantVisibilityModifier",
+            )
+            internal class `TestNestedClassConstructor${'$'}NestedClass${'$'}OneMoreNestedClass__Factory` :
+                Factory<TestNestedClassConstructor.NestedClass.OneMoreNestedClass> {
+              public override fun createInstance(scope: Scope):
+                  TestNestedClassConstructor.NestedClass.OneMoreNestedClass =
+                  TestNestedClassConstructor.NestedClass.OneMoreNestedClass()
+            
+              public override fun getTargetScope(scope: Scope): Scope = scope
+            
+              public override fun hasScopeAnnotation(): Boolean = false
+            
+              public override fun hasSingletonAnnotation(): Boolean = false
+            
+              public override fun hasReleasableAnnotation(): Boolean = false
+            
+              public override fun hasProvidesSingletonAnnotation(): Boolean = false
+            
+              public override fun hasProvidesReleasableAnnotation(): Boolean = false
+            }
+            """
+    )
+
+    @Test
+    fun testInjectedConstructorForTwoNestedClassesWithInternalMiddleParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            class TestNestedClassConstructor @Inject constructor() {
+                internal class NestedClass @Inject constructor() {
+                    class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .compilesWithoutError()
+            .generatesSources(testLastNestedClassWithInternal_expected)
+    }
+
+    @Test
+    fun testInjectedConstructorForPrivateNestedClassWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                class NestedClass @Inject constructor() {
+                    private class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .failsToCompile()
+            .withLogContaining(
+                "/sources/TestNestedClassConstructor.kt:6: Class test.TestNestedClassConstructor.NestedClass.OneMoreNestedClass is private. @Inject-annotated constructors are not allowed in private classes."
+            )
+    }
+
+    // Do not use the private modifier in your code
+    @Test
+    fun testInjectedConstructorForPrivateMiddleNestedClassWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                private class NestedClass @Inject constructor() {
+                    class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .failsToCompile()
+            .withLogContaining(
+                "/sources/TestNestedClassConstructor.kt:5: Class test.TestNestedClassConstructor.NestedClass is private. @Inject-annotated constructors are not allowed in private classes."
+            )
+    }
+
+    // Do not use the private modifier in your code
+    @Test
+    fun testInjectedConstructorForProtectedMiddleNestedClassAndPrivateLastWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                protected class NestedClass @Inject constructor() {
+                    private class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .failsToCompile()
+            .withLogContaining(
+                "/sources/TestNestedClassConstructor.kt:6: Class test.TestNestedClassConstructor.NestedClass.OneMoreNestedClass is private. @Inject-annotated constructors are not allowed in private classes."
+            )
+    }
+
+    @Test
+    fun testInjectedConstructorForProtectedMiddleNestedClassWithInternalParentModifier_kt() {
+        val source = ktSource(
+            "TestNestedClassConstructor",
+            """
+            package test
+            import kotlin.collections.MutableList
+            import javax.inject.Inject
+            internal class TestNestedClassConstructor @Inject constructor() {
+                protected class NestedClass @Inject constructor() {
+                    class OneMoreNestedClass @Inject constructor()
+                }
+            }
+            """
+        )
+
+        compilationAssert()
+            .that(source)
+            .processedWith(FactoryProcessorProvider())
+            .compilesWithoutError()
+            .generatesSources(testMiddleNestedClassWithProtected_expected)
+    }
+
+    private val testMiddleNestedClassWithProtected_expected = expectedKtSource(
+        "test/TestNestedClassConstructor\$NestedClass\$OneMoreNestedClass__Factory",
+        """
+            package test
+            
+            import kotlin.Boolean
+            import kotlin.Suppress
+            import toothpick.Factory
+            import toothpick.Scope
+            
+            @Suppress(
+              "ClassName",
+              "RedundantVisibilityModifier",
+            )
+            protected class `TestNestedClassConstructor${'$'}NestedClass${'$'}OneMoreNestedClass__Factory` :
+                Factory<TestNestedClassConstructor.NestedClass.OneMoreNestedClass> {
+              public override fun createInstance(scope: Scope):
+                  TestNestedClassConstructor.NestedClass.OneMoreNestedClass =
+                  TestNestedClassConstructor.NestedClass.OneMoreNestedClass()
+            
+              public override fun getTargetScope(scope: Scope): Scope = scope
+            
+              public override fun hasScopeAnnotation(): Boolean = false
+            
+              public override fun hasSingletonAnnotation(): Boolean = false
+            
+              public override fun hasReleasableAnnotation(): Boolean = false
+            
+              public override fun hasProvidesSingletonAnnotation(): Boolean = false
+            
+              public override fun hasProvidesReleasableAnnotation(): Boolean = false
+            }
+            """
+    )
 }
diff --git a/gradle.properties b/gradle.properties
@@ -1,5 +1,5 @@
 GROUP=fr.outadoc.toothpick-ksp
-VERSION_NAME=1.0.0
+VERSION_NAME=1.0.1
 
 POM_PACKAGING=JAR