From 0cbf608bde7d78e0e45dca5482b1193bfeb8d7a6 Mon Sep 17 00:00:00 2001 From: Omico Date: Tue, 16 May 2023 11:29:35 -0700 Subject: [PATCH 1/9] Omit public modifier on override function or constructor parameters --- .../com/squareup/kotlinpoet/CodeWriter.kt | 4 ++ .../squareup/kotlinpoet/AnnotationSpecTest.kt | 4 +- .../com/squareup/kotlinpoet/FunSpecTest.kt | 6 +-- .../com/squareup/kotlinpoet/KotlinPoetTest.kt | 2 +- .../com/squareup/kotlinpoet/MemberNameTest.kt | 2 +- .../com/squareup/kotlinpoet/TypeSpecTest.kt | 41 +++++++++---------- 6 files changed, 31 insertions(+), 28 deletions(-) diff --git a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt index 5d827276d3..8b4e311bbb 100644 --- a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt +++ b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt @@ -634,6 +634,10 @@ internal class CodeWriter constructor( modifiers: Set, implicitModifiers: Set, ): Boolean { + if (modifiers.contains(KModifier.OVERRIDE)) { + return false + } + if (modifiers.contains(KModifier.PUBLIC)) { return true } diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/AnnotationSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/AnnotationSpecTest.kt index 4f77040614..ee6760dd1b 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/AnnotationSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/AnnotationSpecTest.kt @@ -565,9 +565,9 @@ class AnnotationSpecTest { ) public object ExternalClassParceler : Parceler { - public override fun create(parcel: Parcel) = ExternalClass(parcel.readInt()) + override fun create(parcel: Parcel) = ExternalClass(parcel.readInt()) - public override fun ExternalClass.write(parcel: Parcel, flags: Int): Unit { + override fun ExternalClass.write(parcel: Parcel, flags: Int): Unit { parcel.writeInt(value) } } diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt index 1376cafc9b..2709c17333 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt @@ -109,7 +109,7 @@ class FunSpecTest { val funSpec = FunSpec.overriding(exec).build() assertThat(funSpec.toString()).isEqualTo( """ - |public override fun toString(): java.lang.String { + |override fun toString(): java.lang.String { |} | """.trimMargin(), @@ -125,7 +125,7 @@ class FunSpecTest { assertThat(funSpec.toString()).isEqualTo( """ |@kotlin.jvm.Throws(java.lang.Exception::class) - |public override fun call(): java.lang.Integer { + |override fun call(): java.lang.Integer { |} | """.trimMargin(), @@ -134,7 +134,7 @@ class FunSpecTest { funSpec = FunSpec.overriding(exec, classType, types).build() assertThat(funSpec.toString()).isEqualTo( """ - |public override fun compareTo(arg0: java.lang.Long): kotlin.Int { + |override fun compareTo(arg0: java.lang.Long): kotlin.Int { |} | """.trimMargin(), diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt index bd0210202e..574ad9598c 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt @@ -692,7 +692,7 @@ class KotlinPoetTest { |internal const val p: String = "a" | |public abstract class B : A() { - | public final override lateinit var q: String + | final override lateinit var q: String |} | """.trimMargin(), diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/MemberNameTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/MemberNameTest.kt index 565e61e244..3db8a31ade 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/MemberNameTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/MemberNameTest.kt @@ -596,7 +596,7 @@ class MemberNameTest { import kotlin.hashCode public class Message { - public override fun hashCode(): Int { + override fun hashCode(): Int { var result = super.hashCode if (result == 0) { result = result * 37 + embedded_message.hashCode() diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index cf6c3e2982..cc81ced751 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -83,12 +83,12 @@ class TypeSpecTest { |import kotlin.String | |public class Taco { - | public final override fun toString(): String = "taco" + | final override fun toString(): String = "taco" |} | """.trimMargin(), ) - assertEquals(1906837485, taco.hashCode().toLong()) // Update expected number if source changes. + assertEquals(-49503836, taco.hashCode().toLong()) // Update expected number if source changes. } @Test fun interestingTypes() { @@ -173,9 +173,8 @@ class TypeSpecTest { | |public class Taco { | public val NAME: Thing.Thang = object : Thing.Thang() { - | public override fun call(thung: Thung): Thung = object : SimpleThung(thung) - | { - | public override fun doSomething(bar: Bar): Unit { + | override fun call(thung: Thung): Thung = object : SimpleThung(thung) { + | override fun doSomething(bar: Bar): Unit { | /* code snippets */ | } | } @@ -244,7 +243,7 @@ class TypeSpecTest { | |public class Taco { | public val NAME: Runnable = object : Message(), Runnable { - | public override fun run(): Unit { + | override fun run(): Unit { | /* code snippets */ | } | } @@ -584,7 +583,7 @@ class TypeSpecTest { | */ | ROCK, | PAPER("flat") { - | public override fun toString(): String = "paper airplane!" + | override fun toString(): String = "paper airplane!" | }, | SCISSORS("peace sign"), | ; @@ -626,7 +625,7 @@ class TypeSpecTest { | |public enum class Tortilla { | CORN { - | public override fun fold(): Unit { + | override fun fold(): Unit { | } | }, | ; @@ -857,7 +856,7 @@ class TypeSpecTest { | |public enum class Roshambo { | SPOCK { - | public override fun toString(): String = "west side" + | override fun toString(): String = "west side" | }, |} | @@ -1000,7 +999,7 @@ class TypeSpecTest { | | public val y: P | - | public override fun compareTo(p: P): Int = 0 + | override fun compareTo(p: P): Int = 0 | | public fun of( | label: T, @@ -2439,7 +2438,7 @@ class TypeSpecTest { .addStatement("return %S", "taco") .build() assertThat(funSpec.toString()) - .isEqualTo("public override fun toString(): kotlin.String = \"taco\"\n") + .isEqualTo("override fun toString(): kotlin.String = \"taco\"\n") } @Test fun constructorToString() { @@ -2486,7 +2485,7 @@ class TypeSpecTest { assertThat(type.toString()).isEqualTo( """ |object : java.lang.Runnable { - | public override fun run(): kotlin.Unit { + | override fun run(): kotlin.Unit { | } |} """.trimMargin(), @@ -2544,7 +2543,7 @@ class TypeSpecTest { |import kotlin.String | |public class Taco { - | public override fun toString(): String { + | override fun toString(): String { | val result = "Taco(" | + "beef," | + "lettuce," @@ -2606,7 +2605,7 @@ class TypeSpecTest { | public fun comparePrefix(length: Int): Comparator { | // Return a new comparator for the target length. | return object : Comparator { - | public override fun compare(a: String, b: String): Int { + | override fun compare(a: String, b: String): Int { | // Prefix the strings and compare them | return a.substring(0, length) | .compareTo(b.substring(0, length)) @@ -2618,7 +2617,7 @@ class TypeSpecTest { | Collections.sort( | list, | object : Comparator { - | public override fun compare(a: String, b: String): Int { + | override fun compare(a: String, b: String): Int { | // Prefix the strings and compare them | return a.substring(0, length) | .compareTo(b.substring(0, length)) @@ -3098,7 +3097,7 @@ class TypeSpecTest { | | private const val FOO: String = "FOO" | - | public override fun toString(): String = FOO + | override fun toString(): String = FOO |} | """.trimMargin(), @@ -3144,7 +3143,7 @@ class TypeSpecTest { | | public constructor() | - | public override fun toString(): String = FOO + | override fun toString(): String = FOO |} | """.trimMargin(), @@ -3198,7 +3197,7 @@ class TypeSpecTest { | | public constructor() | - | public override fun toString(): String = FOO + | override fun toString(): String = FOO |} | """.trimMargin(), @@ -4133,9 +4132,9 @@ class TypeSpecTest { |import kotlin.String | |public data class Person( - | public override val id: Int, - | public override val name: String, - | public override val surname: String, + | override val id: Int, + | override val name: String, + | override val surname: String, |) | """.trimMargin(), From d142e18ae83ec4ee81f7a4dfffe7655c3a0d79ce Mon Sep 17 00:00:00 2001 From: Omico Date: Tue, 16 May 2023 12:28:22 -0700 Subject: [PATCH 2/9] Fix tests in the kotlinx-metadata module --- .../specs/KotlinPoetMetadataSpecsTest.kt | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/interop/kotlinx-metadata/src/test/kotlin/com/squareup/kotlinpoet/metadata/specs/KotlinPoetMetadataSpecsTest.kt b/interop/kotlinx-metadata/src/test/kotlin/com/squareup/kotlinpoet/metadata/specs/KotlinPoetMetadataSpecsTest.kt index 9198dbcbcd..490da456bf 100644 --- a/interop/kotlinx-metadata/src/test/kotlin/com/squareup/kotlinpoet/metadata/specs/KotlinPoetMetadataSpecsTest.kt +++ b/interop/kotlinx-metadata/src/test/kotlin/com/squareup/kotlinpoet/metadata/specs/KotlinPoetMetadataSpecsTest.kt @@ -426,14 +426,14 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { assertThat(typeSpec.trimmedToString()).isEqualTo( """ public abstract class OverriddenThings() : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.OverriddenThingsBase(), com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.OverriddenThingsInterface { - public override var openProp: kotlin.String = throw NotImplementedError("Stub!") + override var openProp: kotlin.String = throw NotImplementedError("Stub!") - public override var openPropInterface: kotlin.String = throw NotImplementedError("Stub!") + override var openPropInterface: kotlin.String = throw NotImplementedError("Stub!") - public override fun openFunction(): kotlin.Unit { + override fun openFunction(): kotlin.Unit { } - public override fun openFunctionInterface(): kotlin.Unit { + override fun openFunctionInterface(): kotlin.Unit { } } """.trimIndent(), @@ -551,13 +551,13 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { public val `value`: kotlin.String, ) { FOO { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, BAR { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, BAZ { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, ; } @@ -616,14 +616,14 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { public val `value`: kotlin.String, ) { FOO { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, @com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.FieldAnnotation BAR { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, BAZ { - public override fun toString(): kotlin.String = throw NotImplementedError("Stub!") + override fun toString(): kotlin.String = throw NotImplementedError("Stub!") }, ; } @@ -687,11 +687,11 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { assertThat(subInterfaceSpec.trimmedToString()).isEqualTo( """ public interface SubInterface : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.TestInterface { - public override fun hasDefault(): kotlin.Unit { + override fun hasDefault(): kotlin.Unit { } @kotlin.jvm.JvmDefault - public override fun hasJvmDefault(): kotlin.Unit { + override fun hasJvmDefault(): kotlin.Unit { } public fun subInterfaceFunction(): kotlin.Unit { @@ -706,13 +706,13 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { assertThat(implSpec.trimmedToString()).isEqualTo( """ public class TestSubInterfaceImpl() : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.SubInterface { - public override fun noDefault(): kotlin.Unit { + override fun noDefault(): kotlin.Unit { } - public override fun noDefaultWithInput(input: kotlin.String): kotlin.Unit { + override fun noDefaultWithInput(input: kotlin.String): kotlin.Unit { } - public override fun noDefaultWithInputDefault(input: kotlin.String): kotlin.Unit { + override fun noDefaultWithInputDefault(input: kotlin.String): kotlin.Unit { } } """.trimIndent(), @@ -2100,14 +2100,14 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { public abstract class AbstractModalities() : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.ModalitiesInterface { public val implicitFinalProp: kotlin.String? = null - public override val interfaceProp: kotlin.String? = null + override val interfaceProp: kotlin.String? = null public open val openProp: kotlin.String? = null public fun implicitFinalFun(): kotlin.Unit { } - public override fun interfaceFun(): kotlin.Unit { + override fun interfaceFun(): kotlin.Unit { } public open fun openFun(): kotlin.Unit { @@ -2122,9 +2122,9 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { assertThat(finalAbstractModalities.trimmedToString()).isEqualTo( """ public abstract class FinalAbstractModalities() : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.ModalitiesInterface { - public final override val interfaceProp: kotlin.String? = null + final override val interfaceProp: kotlin.String? = null - public final override fun interfaceFun(): kotlin.Unit { + final override fun interfaceFun(): kotlin.Unit { } } """.trimIndent(), @@ -2136,14 +2136,14 @@ class KotlinPoetMetadataSpecsTest : MultiClassInspectorTest() { assertThat(modalities.trimmedToString()).isEqualTo( """ public class Modalities() : com.squareup.kotlinpoet.metadata.specs.KotlinPoetMetadataSpecsTest.AbstractModalities() { - public override val interfaceProp: kotlin.String? = null + override val interfaceProp: kotlin.String? = null - public override val openProp: kotlin.String? = null + override val openProp: kotlin.String? = null - public override fun interfaceFun(): kotlin.Unit { + override fun interfaceFun(): kotlin.Unit { } - public override fun openFun(): kotlin.Unit { + override fun openFun(): kotlin.Unit { } } """.trimIndent(), From 9ce8fa184e478d71d02cfc6d7de28ca29574b1e5 Mon Sep 17 00:00:00 2001 From: Omico Date: Wed, 17 May 2023 11:43:45 -0700 Subject: [PATCH 3/9] Update comment --- .../src/main/java/com/squareup/kotlinpoet/CodeWriter.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt index 8b4e311bbb..18be1c617a 100644 --- a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt +++ b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt @@ -624,7 +624,9 @@ internal class CodeWriter constructor( /** * Returns whether a [KModifier.PUBLIC] should be emitted. * - * If [modifiers] contains [KModifier.PUBLIC], this method always returns `true`. + * If [modifiers] contains [KModifier.OVERRIDE], this method will always return `false`. + * + * Next, if [modifiers] contains [KModifier.PUBLIC], this method will return `true`. * * Otherwise, this will return `true` when [KModifier.PUBLIC] is one of the [implicitModifiers] * and there are no other opposing modifiers (like [KModifier.PROTECTED] etc.) supplied by the From 51f1b28d2020ca8d87e1e75fd248659d5b9da4e0 Mon Sep 17 00:00:00 2001 From: Omico Date: Wed, 17 May 2023 15:55:59 -0700 Subject: [PATCH 4/9] Rollback some tests --- .../com/squareup/kotlinpoet/FunSpecTest.kt | 6 ++-- .../com/squareup/kotlinpoet/KotlinPoetTest.kt | 2 +- .../com/squareup/kotlinpoet/TypeSpecTest.kt | 33 ++++++++++--------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt index 2709c17333..1376cafc9b 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/FunSpecTest.kt @@ -109,7 +109,7 @@ class FunSpecTest { val funSpec = FunSpec.overriding(exec).build() assertThat(funSpec.toString()).isEqualTo( """ - |override fun toString(): java.lang.String { + |public override fun toString(): java.lang.String { |} | """.trimMargin(), @@ -125,7 +125,7 @@ class FunSpecTest { assertThat(funSpec.toString()).isEqualTo( """ |@kotlin.jvm.Throws(java.lang.Exception::class) - |override fun call(): java.lang.Integer { + |public override fun call(): java.lang.Integer { |} | """.trimMargin(), @@ -134,7 +134,7 @@ class FunSpecTest { funSpec = FunSpec.overriding(exec, classType, types).build() assertThat(funSpec.toString()).isEqualTo( """ - |override fun compareTo(arg0: java.lang.Long): kotlin.Int { + |public override fun compareTo(arg0: java.lang.Long): kotlin.Int { |} | """.trimMargin(), diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt index 574ad9598c..bd0210202e 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/KotlinPoetTest.kt @@ -692,7 +692,7 @@ class KotlinPoetTest { |internal const val p: String = "a" | |public abstract class B : A() { - | final override lateinit var q: String + | public final override lateinit var q: String |} | """.trimMargin(), diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index cc81ced751..23b7b8df1c 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -83,12 +83,12 @@ class TypeSpecTest { |import kotlin.String | |public class Taco { - | final override fun toString(): String = "taco" + | public final override fun toString(): String = "taco" |} | """.trimMargin(), ) - assertEquals(-49503836, taco.hashCode().toLong()) // Update expected number if source changes. + assertEquals(1906837485, taco.hashCode().toLong()) // Update expected number if source changes. } @Test fun interestingTypes() { @@ -173,8 +173,9 @@ class TypeSpecTest { | |public class Taco { | public val NAME: Thing.Thang = object : Thing.Thang() { - | override fun call(thung: Thung): Thung = object : SimpleThung(thung) { - | override fun doSomething(bar: Bar): Unit { + | public override fun call(thung: Thung): Thung = object : SimpleThung(thung) + | { + | public override fun doSomething(bar: Bar): Unit { | /* code snippets */ | } | } @@ -243,7 +244,7 @@ class TypeSpecTest { | |public class Taco { | public val NAME: Runnable = object : Message(), Runnable { - | override fun run(): Unit { + | public override fun run(): Unit { | /* code snippets */ | } | } @@ -583,7 +584,7 @@ class TypeSpecTest { | */ | ROCK, | PAPER("flat") { - | override fun toString(): String = "paper airplane!" + | public override fun toString(): String = "paper airplane!" | }, | SCISSORS("peace sign"), | ; @@ -625,7 +626,7 @@ class TypeSpecTest { | |public enum class Tortilla { | CORN { - | override fun fold(): Unit { + | public override fun fold(): Unit { | } | }, | ; @@ -856,7 +857,7 @@ class TypeSpecTest { | |public enum class Roshambo { | SPOCK { - | override fun toString(): String = "west side" + | public override fun toString(): String = "west side" | }, |} | @@ -999,7 +1000,7 @@ class TypeSpecTest { | | public val y: P | - | override fun compareTo(p: P): Int = 0 + | public override fun compareTo(p: P): Int = 0 | | public fun of( | label: T, @@ -2438,7 +2439,7 @@ class TypeSpecTest { .addStatement("return %S", "taco") .build() assertThat(funSpec.toString()) - .isEqualTo("override fun toString(): kotlin.String = \"taco\"\n") + .isEqualTo("public override fun toString(): kotlin.String = \"taco\"\n") } @Test fun constructorToString() { @@ -2543,7 +2544,7 @@ class TypeSpecTest { |import kotlin.String | |public class Taco { - | override fun toString(): String { + | public override fun toString(): String { | val result = "Taco(" | + "beef," | + "lettuce," @@ -2605,7 +2606,7 @@ class TypeSpecTest { | public fun comparePrefix(length: Int): Comparator { | // Return a new comparator for the target length. | return object : Comparator { - | override fun compare(a: String, b: String): Int { + | public override fun compare(a: String, b: String): Int { | // Prefix the strings and compare them | return a.substring(0, length) | .compareTo(b.substring(0, length)) @@ -2617,7 +2618,7 @@ class TypeSpecTest { | Collections.sort( | list, | object : Comparator { - | override fun compare(a: String, b: String): Int { + | public override fun compare(a: String, b: String): Int { | // Prefix the strings and compare them | return a.substring(0, length) | .compareTo(b.substring(0, length)) @@ -3097,7 +3098,7 @@ class TypeSpecTest { | | private const val FOO: String = "FOO" | - | override fun toString(): String = FOO + | public override fun toString(): String = FOO |} | """.trimMargin(), @@ -3143,7 +3144,7 @@ class TypeSpecTest { | | public constructor() | - | override fun toString(): String = FOO + | public override fun toString(): String = FOO |} | """.trimMargin(), @@ -3197,7 +3198,7 @@ class TypeSpecTest { | | public constructor() | - | override fun toString(): String = FOO + | public override fun toString(): String = FOO |} | """.trimMargin(), From 434955391eca35df4e92efa68e8c1fb715dfd25a Mon Sep 17 00:00:00 2001 From: Omico Date: Wed, 17 May 2023 16:33:15 -0700 Subject: [PATCH 5/9] Only omit public modifier if it inside of implicit modifiers, and modifiers contain override modifier --- .../com/squareup/kotlinpoet/CodeWriter.kt | 13 +++-- .../com/squareup/kotlinpoet/TypeSpecTest.kt | 57 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt index 18be1c617a..f61aa5265a 100644 --- a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt +++ b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt @@ -624,9 +624,10 @@ internal class CodeWriter constructor( /** * Returns whether a [KModifier.PUBLIC] should be emitted. * - * If [modifiers] contains [KModifier.OVERRIDE], this method will always return `false`. + * If [modifiers] contains [KModifier.PUBLIC], this method always returns `true`. * - * Next, if [modifiers] contains [KModifier.PUBLIC], this method will return `true`. + * Next, if [implicitModifiers] contains [KModifier.PUBLIC] and [modifiers] contains + * [KModifier.OVERRIDE], this method will return `false`. * * Otherwise, this will return `true` when [KModifier.PUBLIC] is one of the [implicitModifiers] * and there are no other opposing modifiers (like [KModifier.PROTECTED] etc.) supplied by the @@ -636,14 +637,14 @@ internal class CodeWriter constructor( modifiers: Set, implicitModifiers: Set, ): Boolean { - if (modifiers.contains(KModifier.OVERRIDE)) { - return false - } - if (modifiers.contains(KModifier.PUBLIC)) { return true } + if (implicitModifiers.contains(KModifier.PUBLIC) && modifiers.contains(KModifier.OVERRIDE)) { + return false + } + if (!implicitModifiers.contains(KModifier.PUBLIC)) { return false } diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index 23b7b8df1c..c8a7782256 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -5336,6 +5336,63 @@ class TypeSpecTest { ) } + // https://github.com/square/kotlinpoet/issues/1548 + @Test fun overrideInternalAbstractFunctionVisibility() { + val baseClass = TypeSpec.classBuilder("Base") + .addModifiers(PUBLIC, ABSTRACT) + .addFunction( + FunSpec.builder("foo") + .addModifiers(INTERNAL, ABSTRACT) + .build(), + ) + .build() + assertThat(baseClass.toString()).isEqualTo( + """ + |public abstract class Base { + | internal abstract fun foo(): kotlin.Unit + |} + | + """.trimMargin(), + ) + val bassClassName = ClassName("", "Base") + val exampleClass = TypeSpec.classBuilder("Example") + .addModifiers(PUBLIC) + .superclass(bassClassName) + .addFunction( + FunSpec.builder("foo") + .addModifiers(KModifier.OVERRIDE) + .build(), + ) + .build() + assertThat(exampleClass.toString()).isEqualTo( + """ + |public class Example : Base() { + | override fun foo(): kotlin.Unit { + | } + |} + | + """.trimMargin(), + ) + val example2Class = TypeSpec.classBuilder("Example2") + .addModifiers(PUBLIC) + .superclass(bassClassName) + .addFunction( + FunSpec.builder("foo") + .addModifiers(PUBLIC, KModifier.OVERRIDE) + .build(), + ) + .build() + assertThat(example2Class.toString()).isEqualTo( + """ + |public class Example2 : Base() { + | public override fun foo(): kotlin.Unit { + | } + |} + | + """.trimMargin(), + ) + } + @Test fun contextReceiver() { val typeSpec = TypeSpec.classBuilder("Example") .contextReceivers(STRING) From d64e2f34d4ca01db7ae48062981104b59dea1a2b Mon Sep 17 00:00:00 2001 From: Omico Date: Thu, 18 May 2023 01:04:20 -0700 Subject: [PATCH 6/9] Rollback anonymousClassToString() test Because it explicitly added public modifier --- .../src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index c8a7782256..6a5f38c150 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -2486,7 +2486,7 @@ class TypeSpecTest { assertThat(type.toString()).isEqualTo( """ |object : java.lang.Runnable { - | override fun run(): kotlin.Unit { + | public override fun run(): kotlin.Unit { | } |} """.trimMargin(), From 8a80706f325737129e4b0d8592e5eaa46f5a437a Mon Sep 17 00:00:00 2001 From: Omico Date: Thu, 18 May 2023 01:21:34 -0700 Subject: [PATCH 7/9] Add more test --- .../com/squareup/kotlinpoet/TypeSpecTest.kt | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index 6a5f38c150..607c3db5a6 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -5382,6 +5382,7 @@ class TypeSpecTest { .build(), ) .build() + // Don't omit the public modifier here. The bytecode is different. assertThat(example2Class.toString()).isEqualTo( """ |public class Example2 : Base() { @@ -5391,6 +5392,25 @@ class TypeSpecTest { | """.trimMargin(), ) + val example3Class = TypeSpec.classBuilder("Example3") + .addModifiers(INTERNAL) + .superclass(bassClassName) + .addFunction( + FunSpec.builder("foo") + .addModifiers(PUBLIC, KModifier.OVERRIDE) + .build(), + ) + .build() + // We cannot omit the public modifier here. The bytecode is different. + assertThat(example3Class.toString()).isEqualTo( + """ + |internal class Example3 : Base() { + | public override fun foo(): kotlin.Unit { + | } + |} + | + """.trimMargin(), + ) } @Test fun contextReceiver() { From 8eb98acba76084d8f840888e3208e4a326051390 Mon Sep 17 00:00:00 2001 From: Omico Date: Thu, 18 May 2023 04:23:35 -0700 Subject: [PATCH 8/9] Remove comment Will clean it up in a separate PR --- kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt index f61aa5265a..2d9e523934 100644 --- a/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt +++ b/kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt @@ -626,9 +626,6 @@ internal class CodeWriter constructor( * * If [modifiers] contains [KModifier.PUBLIC], this method always returns `true`. * - * Next, if [implicitModifiers] contains [KModifier.PUBLIC] and [modifiers] contains - * [KModifier.OVERRIDE], this method will return `false`. - * * Otherwise, this will return `true` when [KModifier.PUBLIC] is one of the [implicitModifiers] * and there are no other opposing modifiers (like [KModifier.PROTECTED] etc.) supplied by the * consumer in [modifiers]. From ad27b03d1b9e18b610685213b9ff24e2fcb55ca9 Mon Sep 17 00:00:00 2001 From: Omico Date: Thu, 18 May 2023 08:59:53 -0700 Subject: [PATCH 9/9] Update comments --- .../src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt index 607c3db5a6..524e895603 100644 --- a/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt +++ b/kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt @@ -5382,7 +5382,8 @@ class TypeSpecTest { .build(), ) .build() - // Don't omit the public modifier here. The bytecode is different. + // Don't omit the public modifier here, + // as we're explicitly increasing the visibility of this method in the subclass. assertThat(example2Class.toString()).isEqualTo( """ |public class Example2 : Base() { @@ -5401,7 +5402,8 @@ class TypeSpecTest { .build(), ) .build() - // We cannot omit the public modifier here. The bytecode is different. + // Don't omit the public modifier here, + // as we're explicitly increasing the visibility of this method in the subclass. assertThat(example3Class.toString()).isEqualTo( """ |internal class Example3 : Base() {