omit more permissive modifiers on restricted visibility types

square · Apr 12, 2024 · 70f3355 · 70f3355
1 parent dd8a717
commit 70f3355
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 1 deletion.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -16,6 +16,7 @@ Change: kotlinx-metadata 0.9.0. Note that the `KotlinClassMetadata .read` is dep
 * Fix: `MemberName`s without a package are now correctly imported (#1841)
 * Fix: Throw if primary constructor delegates to other constructors (#1859).
 * Fix: Aliased imports with nested class (#1876).
+* Fix: Omit more permissive modifiers on restricted visibility types (#1301).
 
 ## Version 1.16.0
 

diff --git a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt
@@ -652,6 +652,10 @@ internal class CodeWriter constructor(
       return false
     }
 
+    if (implicitModifiers.contains(KModifier.INTERNAL)) {
+      return false
+    }
+
     if (!implicitModifiers.contains(KModifier.PUBLIC)) {
       return false
     }

diff --git a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt
@@ -171,7 +171,7 @@ public class TypeSpec private constructor(
         codeWriter.emitAnnotations(annotations, false)
         codeWriter.emitModifiers(
           modifiers,
-          if (isNestedExternal) setOf(PUBLIC, EXTERNAL) else setOf(PUBLIC),
+          implicitModifiers + if (isNestedExternal) setOf(PUBLIC, EXTERNAL) else setOf(PUBLIC),
         )
         codeWriter.emit(kind.declarationKeyword)
         if (name != null) {
@@ -446,6 +446,7 @@ public class TypeSpec private constructor(
         ANNOTATION in modifiers -> emptySet()
         EXPECT in modifiers -> setOf(EXPECT)
         EXTERNAL in modifiers -> setOf(EXTERNAL)
+        INTERNAL in modifiers -> setOf(INTERNAL)
         else -> emptySet()
       }
     }
@@ -454,6 +455,7 @@ public class TypeSpec private constructor(
       return defaultImplicitFunctionModifiers + when {
         EXPECT in modifiers -> setOf(EXPECT)
         EXTERNAL in modifiers -> setOf(EXTERNAL)
+        INTERNAL in modifiers -> setOf(INTERNAL)
         else -> emptySet()
       }
     }
@@ -462,6 +464,7 @@ public class TypeSpec private constructor(
       return defaultImplicitTypeModifiers + when {
         EXPECT in modifiers -> setOf(EXPECT)
         EXTERNAL in modifiers -> setOf(EXTERNAL)
+        INTERNAL in modifiers -> setOf(INTERNAL)
         else -> emptySet()
       }
     }

diff --git a/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt
@@ -5778,6 +5778,96 @@ class TypeSpecTest {
     }.hasMessageThat().isEqualTo("primary constructor can't delegate to other constructors")
   }
 
+  // https://github.com/square/kotlinpoet/issues/1301
+  @Test fun permissiveModifiersOmittedOnRestrictedVisibilityTypesInSealedInterface() {
+    val pageStateClass = ClassName("", "FeaturePageState")
+    val type =
+      TypeSpec.interfaceBuilder("FeaturePageState")
+        .addModifiers(SEALED)
+        .addModifiers(INTERNAL)
+        .addType(
+          TypeSpec.objectBuilder("Loading")
+            .addModifiers(DATA)
+            .addSuperinterface(pageStateClass)
+            .build(),
+        )
+        .addType(
+          TypeSpec.objectBuilder("Error")
+            .addModifiers(DATA)
+            .addSuperinterface(pageStateClass)
+            .build(),
+        )
+        .addType(
+          TypeSpec.objectBuilder("Content")
+            .addModifiers(DATA)
+            .addSuperinterface(pageStateClass)
+            .build(),
+        )
+        .build()
+
+    //language=kotlin
+    assertThat(type.toString()).isEqualTo(
+      // no public visibility modifier on implicitly internal sealed types
+      """
+      internal sealed interface FeaturePageState {
+        data object Loading : FeaturePageState
+
+        data object Error : FeaturePageState
+
+        data object Content : FeaturePageState
+      }
+
+      """.trimIndent(),
+    )
+  }
+
+  @Test fun permissiveModifiersOmittedOnRestrictedVisibilityPropertyInClass() {
+    val type = TypeSpec.classBuilder("Taco")
+      .addModifiers(INTERNAL)
+      .addProperty("ints", IntArray::class)
+      .build()
+
+    //language=kotlin
+    assertThat(toString(type)).isEqualTo(
+      """
+      package com.squareup.tacos
+
+      import kotlin.IntArray
+
+      internal class Taco {
+        val ints: IntArray
+      }
+
+      """.trimIndent(),
+    )
+  }
+
+  @Test fun permissiveModifiersOmittedOnRestrictedVisibilityFunctionInClass() {
+    val taco = TypeSpec.classBuilder("Taco")
+      .addModifiers(INTERNAL)
+      .addFunction(
+        FunSpec.builder("toString")
+          .addModifiers(KModifier.FINAL, KModifier.OVERRIDE)
+          .returns(String::class)
+          .addStatement("return %S", "taco")
+          .build(),
+      )
+      .build()
+
+    assertThat(toString(taco)).isEqualTo(
+      """
+        |package com.squareup.tacos
+        |
+        |import kotlin.String
+        |
+        |internal class Taco {
+        |  final override fun toString(): String = "taco"
+        |}
+        |
+      """.trimMargin(),
+    )
+  }
+
   companion object {
     private const val donutsPackage = "com.squareup.donuts"
   }