From 0d23625a0b99c85afba80da9f59a5a822f943ebd Mon Sep 17 00:00:00 2001 From: Joe Roskopf <7951665+j-roskopf@users.noreply.github.com> Date: Fri, 12 Apr 2024 16:20:19 -0500 Subject: [PATCH] additional coverage + better tests to cover matrix of 4 accessor modifiers --- .../com/squareup/kotlinpoet/CodeWriter.kt | 15 +- .../com/squareup/kotlinpoet/TypeSpec.kt | 6 + .../com/squareup/kotlinpoet/TypeSpecTest.kt | 334 ++++++++++++++++-- 3 files changed, 331 insertions(+), 24 deletions(-) diff --git a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt index b8587ccd92..385beaaf9a 100644 --- a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt +++ b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/CodeWriter.kt @@ -161,11 +161,19 @@ internal class CodeWriter constructor( emit(KModifier.PUBLIC.keyword) emit(" ") } + + // public always being an implicit modifier in addition to whatever we inherited. + // we don't want to throw away the implicit modifier we inherited + val implicitModifierContainsAtLeastTwoAccessMonitors = implicitModifiers.count { + it == KModifier.PUBLIC || it == KModifier.PRIVATE || it == KModifier.INTERNAL || it == KModifier.PROTECTED + } >= 2 + val uniqueNonPublicExplicitOnlyModifiers = modifiers .filterNot { it == KModifier.PUBLIC } - .filterNot { implicitModifiers.contains(it) } + .filterNot { implicitModifiers.contains(it) && implicitModifierContainsAtLeastTwoAccessMonitors.not() } .toEnumSet() + for (modifier in uniqueNonPublicExplicitOnlyModifiers) { emit(modifier.keyword) emit(" ") @@ -652,7 +660,10 @@ internal class CodeWriter constructor( return false } - if (implicitModifiers.contains(KModifier.INTERNAL)) { + if ((modifiers.contains(KModifier.INTERNAL) || modifiers.contains(KModifier.PRIVATE) || modifiers.contains(KModifier.PROTECTED)).not() && + (implicitModifiers.contains(KModifier.INTERNAL) || implicitModifiers.contains(KModifier.PRIVATE) || implicitModifiers.contains(KModifier.PROTECTED)) + ) { + // omit more permissive modifiers on restricted visibility types as long as a modifier was not explicitly declared return false } diff --git a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt index 9496f76f35..0b8cfa4db2 100644 --- a/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt +++ b/kotlinpoet/src/commonMain/kotlin/com/squareup/kotlinpoet/TypeSpec.kt @@ -447,6 +447,8 @@ public class TypeSpec private constructor( EXPECT in modifiers -> setOf(EXPECT) EXTERNAL in modifiers -> setOf(EXTERNAL) INTERNAL in modifiers -> setOf(INTERNAL) + PRIVATE in modifiers -> setOf(PRIVATE) + PROTECTED in modifiers -> setOf(PROTECTED) else -> emptySet() } } @@ -456,6 +458,8 @@ public class TypeSpec private constructor( EXPECT in modifiers -> setOf(EXPECT) EXTERNAL in modifiers -> setOf(EXTERNAL) INTERNAL in modifiers -> setOf(INTERNAL) + PRIVATE in modifiers -> setOf(PRIVATE) + PROTECTED in modifiers -> setOf(PROTECTED) else -> emptySet() } } @@ -465,6 +469,8 @@ public class TypeSpec private constructor( EXPECT in modifiers -> setOf(EXPECT) EXTERNAL in modifiers -> setOf(EXTERNAL) INTERNAL in modifiers -> setOf(INTERNAL) + PRIVATE in modifiers -> setOf(PRIVATE) + PROTECTED in modifiers -> setOf(PROTECTED) else -> emptySet() } } diff --git a/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt index 55083d6989..fadceb8740 100644 --- a/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/commonTest/kotlin/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -24,6 +24,7 @@ import com.squareup.kotlinpoet.KModifier.IN import com.squareup.kotlinpoet.KModifier.INNER import com.squareup.kotlinpoet.KModifier.INTERNAL import com.squareup.kotlinpoet.KModifier.PRIVATE +import com.squareup.kotlinpoet.KModifier.PROTECTED import com.squareup.kotlinpoet.KModifier.PUBLIC import com.squareup.kotlinpoet.KModifier.SEALED import com.squareup.kotlinpoet.KModifier.VARARG @@ -5822,20 +5823,155 @@ class TypeSpecTest { } @Test fun permissiveModifiersOmittedOnRestrictedVisibilityPropertyInClass() { - val type = TypeSpec.classBuilder("Taco") + val privateClass = TypeSpec.classBuilder("Taco") + .addModifiers(PRIVATE) + .addProperty("unspecifiedInts", IntArray::class) + .addProperty("privateInts", IntArray::class, PRIVATE) + .addProperty("protectedInts", IntArray::class, PROTECTED) + .addProperty("publicInts", IntArray::class, PUBLIC) + .addProperty("internalInts", IntArray::class, INTERNAL) + .build() + + //language=kotlin + assertThat(toString(privateClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.IntArray + + private class Taco { + val unspecifiedInts: IntArray + + private val privateInts: IntArray + + protected val protectedInts: IntArray + + public val publicInts: IntArray + + internal val internalInts: IntArray + } + + """.trimIndent(), + ) + + val internalClass = TypeSpec.classBuilder("Taco") .addModifiers(INTERNAL) - .addProperty("ints", IntArray::class) + .addProperty("unspecifiedInts", IntArray::class) + .addProperty("privateInts", IntArray::class, PRIVATE) + .addProperty("protectedInts", IntArray::class, PROTECTED) + .addProperty("publicInts", IntArray::class, PUBLIC) + .addProperty("internalInts", IntArray::class, INTERNAL) .build() //language=kotlin - assertThat(toString(type)).isEqualTo( + assertThat(toString(internalClass)).isEqualTo( """ package com.squareup.tacos import kotlin.IntArray internal class Taco { - val ints: IntArray + val unspecifiedInts: IntArray + + private val privateInts: IntArray + + protected val protectedInts: IntArray + + public val publicInts: IntArray + + internal val internalInts: IntArray + } + + """.trimIndent(), + ) + + val publicClass = TypeSpec.classBuilder("Taco") + .addModifiers(PUBLIC) + .addProperty("unspecifiedInts", IntArray::class) + .addProperty("privateInts", IntArray::class, PRIVATE) + .addProperty("protectedInts", IntArray::class, PROTECTED) + .addProperty("publicInts", IntArray::class, PUBLIC) + .addProperty("internalInts", IntArray::class, INTERNAL) + .build() + + //language=kotlin + assertThat(toString(publicClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.IntArray + + public class Taco { + public val unspecifiedInts: IntArray + + private val privateInts: IntArray + + protected val protectedInts: IntArray + + public val publicInts: IntArray + + internal val internalInts: IntArray + } + + """.trimIndent(), + ) + + val protectedClass = TypeSpec.classBuilder("Taco") + .addModifiers(PROTECTED) + .addProperty("unspecifiedInts", IntArray::class) + .addProperty("privateInts", IntArray::class, PRIVATE) + .addProperty("protectedInts", IntArray::class, PROTECTED) + .addProperty("publicInts", IntArray::class, PUBLIC) + .addProperty("internalInts", IntArray::class, INTERNAL) + .build() + + //language=kotlin + assertThat(toString(protectedClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.IntArray + + protected class Taco { + val unspecifiedInts: IntArray + + private val privateInts: IntArray + + protected val protectedInts: IntArray + + public val publicInts: IntArray + + internal val internalInts: IntArray + } + + """.trimIndent(), + ) + + val unspecifiedClass = TypeSpec.classBuilder("Taco") + .addProperty("unspecifiedInts", IntArray::class) + .addProperty("privateInts", IntArray::class, PRIVATE) + .addProperty("protectedInts", IntArray::class, PROTECTED) + .addProperty("publicInts", IntArray::class, PUBLIC) + .addProperty("internalInts", IntArray::class, INTERNAL) + .build() + + //language=kotlin + assertThat(toString(unspecifiedClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.IntArray + + public class Taco { + public val unspecifiedInts: IntArray + + private val privateInts: IntArray + + protected val protectedInts: IntArray + + public val publicInts: IntArray + + internal val internalInts: IntArray } """.trimIndent(), @@ -5843,28 +5979,182 @@ class TypeSpecTest { } @Test fun permissiveModifiersOmittedOnRestrictedVisibilityFunctionInClass() { - val taco = TypeSpec.classBuilder("Taco") + val unspecifiedVisibilityMethod = FunSpec.builder("unspecifiedStringMethod") + .returns(String::class) + .addStatement("return %S", "taco") + .build() + + val privateVisibilityMethod = FunSpec.builder("privateStringMethod") + .returns(String::class) + .addModifiers(PRIVATE) + .addStatement("return %S", "taco") + .build() + + val internalVisibilityMethod = FunSpec.builder("internalStringMethod") + .returns(String::class) .addModifiers(INTERNAL) - .addFunction( - FunSpec.builder("toString") - .addModifiers(KModifier.FINAL, KModifier.OVERRIDE) - .returns(String::class) - .addStatement("return %S", "taco") - .build(), - ) + .addStatement("return %S", "taco") .build() - assertThat(toString(taco)).isEqualTo( + val protectedVisibilityMethod = FunSpec.builder("protectedStringMethod") + .returns(String::class) + .addModifiers(PROTECTED) + .addStatement("return %S", "taco") + .build() + + val publicVisibilityMethod = FunSpec.builder("publicStringMethod") + .returns(String::class) + .addModifiers(PUBLIC) + .addStatement("return %S", "taco") + .build() + + val privateClass = TypeSpec.classBuilder("Taco") + .addModifiers(PRIVATE) + .addFunction(unspecifiedVisibilityMethod) + .addFunction(privateVisibilityMethod) + .addFunction(internalVisibilityMethod) + .addFunction(protectedVisibilityMethod) + .addFunction(publicVisibilityMethod) + .build() + + assertThat(toString(privateClass)).isEqualTo( """ - |package com.squareup.tacos - | - |import kotlin.String - | - |internal class Taco { - | final override fun toString(): String = "taco" - |} - | - """.trimMargin(), + package com.squareup.tacos + + import kotlin.String + + private class Taco { + fun unspecifiedStringMethod(): String = "taco" + + private fun privateStringMethod(): String = "taco" + + internal fun internalStringMethod(): String = "taco" + + protected fun protectedStringMethod(): String = "taco" + + public fun publicStringMethod(): String = "taco" + } + + """.trimIndent(), + ) + + val protectedClass = TypeSpec.classBuilder("Taco") + .addModifiers(PROTECTED) + .addFunction(unspecifiedVisibilityMethod) + .addFunction(privateVisibilityMethod) + .addFunction(internalVisibilityMethod) + .addFunction(protectedVisibilityMethod) + .addFunction(publicVisibilityMethod) + .build() + + assertThat(toString(protectedClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.String + + protected class Taco { + fun unspecifiedStringMethod(): String = "taco" + + private fun privateStringMethod(): String = "taco" + + internal fun internalStringMethod(): String = "taco" + + protected fun protectedStringMethod(): String = "taco" + + public fun publicStringMethod(): String = "taco" + } + + """.trimIndent(), + ) + + val internalClass = TypeSpec.classBuilder("Taco") + .addModifiers(INTERNAL) + .addFunction(unspecifiedVisibilityMethod) + .addFunction(privateVisibilityMethod) + .addFunction(internalVisibilityMethod) + .addFunction(protectedVisibilityMethod) + .addFunction(publicVisibilityMethod) + .build() + + assertThat(toString(internalClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.String + + internal class Taco { + fun unspecifiedStringMethod(): String = "taco" + + private fun privateStringMethod(): String = "taco" + + internal fun internalStringMethod(): String = "taco" + + protected fun protectedStringMethod(): String = "taco" + + public fun publicStringMethod(): String = "taco" + } + + """.trimIndent(), + ) + + val publicClass = TypeSpec.classBuilder("Taco") + .addModifiers(PUBLIC) + .addFunction(unspecifiedVisibilityMethod) + .addFunction(privateVisibilityMethod) + .addFunction(internalVisibilityMethod) + .addFunction(protectedVisibilityMethod) + .addFunction(publicVisibilityMethod) + .build() + + assertThat(toString(publicClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.String + + public class Taco { + public fun unspecifiedStringMethod(): String = "taco" + + private fun privateStringMethod(): String = "taco" + + internal fun internalStringMethod(): String = "taco" + + protected fun protectedStringMethod(): String = "taco" + + public fun publicStringMethod(): String = "taco" + } + + """.trimIndent(), + ) + + val unspecifiedClass = TypeSpec.classBuilder("Taco") + .addFunction(unspecifiedVisibilityMethod) + .addFunction(privateVisibilityMethod) + .addFunction(internalVisibilityMethod) + .addFunction(protectedVisibilityMethod) + .addFunction(publicVisibilityMethod) + .build() + + assertThat(toString(unspecifiedClass)).isEqualTo( + """ + package com.squareup.tacos + + import kotlin.String + + public class Taco { + public fun unspecifiedStringMethod(): String = "taco" + + private fun privateStringMethod(): String = "taco" + + internal fun internalStringMethod(): String = "taco" + + protected fun protectedStringMethod(): String = "taco" + + public fun publicStringMethod(): String = "taco" + } + + """.trimIndent(), ) }