Skip to content

Commit

Permalink
Omit public modifier on override function or constructor parameters (#…
Browse files Browse the repository at this point in the history 
…1550)

* Omit public modifier on override function or constructor parameters

* Fix tests in the kotlinx-metadata module

* Update comment

* Rollback some tests

* Only omit public modifier if it inside of implicit modifiers, and modifiers contain override modifier

* Rollback anonymousClassToString() test

Because it explicitly added public modifier

* Add more test

* Remove comment

Will clean it up in a separate PR

* Update comments
  • Loading branch information
@Omico
Omico authored May 19, 2023
1 parent f97208f commit 17c2011
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 29 deletions.
46 changes: 23 additions & 23 deletions ...ata/src/test/kotlin/com/squareup/kotlinpoet/metadata/specs/KotlinPoetMetadataSpecsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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!")
},
;
}
Expand Down Expand Up @@ -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!")
},
;
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down
4 changes: 4 additions & 0 deletions kotlinpoet/src/main/java/com/squareup/kotlinpoet/CodeWriter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,10 @@ internal class CodeWriter constructor(
return true
}

if (implicitModifiers.contains(KModifier.PUBLIC) && modifiers.contains(KModifier.OVERRIDE)) {
return false
}

if (!implicitModifiers.contains(KModifier.PUBLIC)) {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions kotlinpoet/src/test/java/com/squareup/kotlinpoet/AnnotationSpecTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,9 +565,9 @@ class AnnotationSpecTest {
)
public object ExternalClassParceler : Parceler<ExternalClass> {
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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion kotlinpoet/src/test/java/com/squareup/kotlinpoet/MemberNameTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
85 changes: 82 additions & 3 deletions kotlinpoet/src/test/java/com/squareup/kotlinpoet/TypeSpecTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4133,9 +4133,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(),
Expand Down Expand Up @@ -5336,6 +5336,85 @@ 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()
// 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() {
| public override fun foo(): kotlin.Unit {
| }
|}
|
""".trimMargin(),
)
val example3Class = TypeSpec.classBuilder("Example3")
.addModifiers(INTERNAL)
.superclass(bassClassName)
.addFunction(
FunSpec.builder("foo")
.addModifiers(PUBLIC, KModifier.OVERRIDE)
.build(),
)
.build()
// 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() {
| public override fun foo(): kotlin.Unit {
| }
|}
|
""".trimMargin(),
)
}

@Test fun contextReceiver() {
val typeSpec = TypeSpec.classBuilder("Example")
.contextReceivers(STRING)
Expand Down

0 comments on commit 17c2011

Please sign in to comment.