Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add "sealed" keyword to sealed interface signatures #3001

Closed
wants to merge 3 commits into from
Closed

Add "sealed" keyword to sealed interface signatures #3001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74b2a84
Add "sealed" keyword to sealed interface signatures
IgnatBeresnev May 19, 2023
d096099
Extract an expression into a variable
IgnatBeresnev May 19, 2023
9aebeec
Add a line break for consistency
IgnatBeresnev May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions core/api/core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,16 +1100,17 @@ public final class org/jetbrains/dokka/model/DFunction : org/jetbrains/dokka/mod
public fun withNewExtras (Lorg/jetbrains/dokka/model/properties/PropertyContainer;)Lorg/jetbrains/dokka/model/DFunction;
}

public final class org/jetbrains/dokka/model/DInterface : org/jetbrains/dokka/model/DClasslike, org/jetbrains/dokka/model/WithCompanion, org/jetbrains/dokka/model/WithGenerics, org/jetbrains/dokka/model/WithSupertypes, org/jetbrains/dokka/model/properties/WithExtraProperties {
public fun <init> (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;)V
public synthetic fun <init> (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final class org/jetbrains/dokka/model/DInterface : org/jetbrains/dokka/model/DClasslike, org/jetbrains/dokka/model/WithAbstraction, org/jetbrains/dokka/model/WithCompanion, org/jetbrains/dokka/model/WithGenerics, org/jetbrains/dokka/model/WithSupertypes, org/jetbrains/dokka/model/properties/WithExtraProperties {
public fun <init> (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;)V
public synthetic fun <init> (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Lorg/jetbrains/dokka/links/DRI;
public final fun component10 ()Lorg/jetbrains/dokka/model/DObject;
public final fun component11 ()Ljava/util/List;
public final fun component12 ()Ljava/util/Map;
public final fun component13 ()Ljava/util/Set;
public final fun component14 ()Z
public final fun component15 ()Lorg/jetbrains/dokka/model/properties/PropertyContainer;
public final fun component13 ()Ljava/util/Map;
public final fun component14 ()Ljava/util/Set;
public final fun component15 ()Z
public final fun component16 ()Lorg/jetbrains/dokka/model/properties/PropertyContainer;
public final fun component2 ()Ljava/lang/String;
public final fun component3 ()Ljava/util/Map;
public final fun component4 ()Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;
Expand All @@ -1118,8 +1119,8 @@ public final class org/jetbrains/dokka/model/DInterface : org/jetbrains/dokka/mo
public final fun component7 ()Ljava/util/List;
public final fun component8 ()Ljava/util/List;
public final fun component9 ()Ljava/util/Map;
public final fun copy (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;)Lorg/jetbrains/dokka/model/DInterface;
public static synthetic fun copy$default (Lorg/jetbrains/dokka/model/DInterface;Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;ILjava/lang/Object;)Lorg/jetbrains/dokka/model/DInterface;
public final fun copy (Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;)Lorg/jetbrains/dokka/model/DInterface;
public static synthetic fun copy$default (Lorg/jetbrains/dokka/model/DInterface;Lorg/jetbrains/dokka/links/DRI;Ljava/lang/String;Ljava/util/Map;Lorg/jetbrains/dokka/DokkaConfiguration$DokkaSourceSet;Ljava/util/Map;Ljava/util/List;Ljava/util/List;Ljava/util/List;Ljava/util/Map;Lorg/jetbrains/dokka/model/DObject;Ljava/util/List;Ljava/util/Map;Ljava/util/Map;Ljava/util/Set;ZLorg/jetbrains/dokka/model/properties/PropertyContainer;ILjava/lang/Object;)Lorg/jetbrains/dokka/model/DInterface;
public fun equals (Ljava/lang/Object;)Z
public fun getChildren ()Ljava/util/List;
public fun getClasslikes ()Ljava/util/List;
Expand All @@ -1130,6 +1131,7 @@ public final class org/jetbrains/dokka/model/DInterface : org/jetbrains/dokka/mo
public fun getExtra ()Lorg/jetbrains/dokka/model/properties/PropertyContainer;
public fun getFunctions ()Ljava/util/List;
public fun getGenerics ()Ljava/util/List;
public fun getModifier ()Ljava/util/Map;
public fun getName ()Ljava/lang/String;
public fun getProperties ()Ljava/util/List;
public fun getSourceSets ()Ljava/util/Set;
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/kotlin/model/Documentable.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,12 @@ data class DInterface(
override val visibility: SourceSetDependent<Visibility>,
override val companion: DObject?,
override val generics: List<DTypeParameter>,
override val modifier: SourceSetDependent<Modifier>,
override val supertypes: SourceSetDependent<List<TypeConstructorWithKind>>,
override val sourceSets: Set<DokkaSourceSet>,
override val isExpectActual: Boolean,
override val extra: PropertyContainer<DInterface> = PropertyContainer.empty()
) : DClasslike(), WithCompanion, WithGenerics, WithSupertypes, WithExtraProperties<DInterface> {
) : DClasslike(), WithCompanion, WithGenerics, WithAbstraction, WithSupertypes, WithExtraProperties<DInterface> {
override val children: List<Documentable>
get() = (functions + properties + classlikes)

Expand Down Expand Up @@ -519,4 +520,4 @@ interface DocumentableSource {
val path: String
}

data class TypeConstructorWithKind(val typeConstructor: TypeConstructor, val kind: ClassKind)
data class TypeConstructorWithKind(val typeConstructor: TypeConstructor, val kind: ClassKind)
46 changes: 28 additions & 18 deletions plugins/base/src/main/kotlin/signatures/KotlinSignatureProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,8 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
annotationsBlock(c)
c.visibility[sourceSet]?.takeIf { it !in ignoredVisibilities }?.name?.let { keyword("$it ") }
if (c.isExpectActual) keyword(if (sourceSet == c.expectPresentInSet) "expect " else "actual ")
if (c is DClass) {
val modifier =
if (c.modifier[sourceSet] !in ignoredModifiers) {
when {
c.extra[AdditionalModifiers]?.content?.get(sourceSet)?.contains(ExtraModifiers.KotlinOnlyModifiers.Data) == true -> ""
c.modifier[sourceSet] is JavaModifier.Empty -> "${KotlinModifier.Open.name} "
else -> c.modifier[sourceSet]?.name?.let { "$it " }
}
} else {
null
}
modifier?.takeIf { it.isNotEmpty() }?.let { keyword(it) }
if (c is WithAbstraction) {
modifier(c, sourceSet)
}
when (c) {
is DClass -> {
Expand Down Expand Up @@ -257,9 +247,7 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
annotationsBlock(p)
p.visibility[sourceSet].takeIf { it !in ignoredVisibilities }?.name?.let { keyword("$it ") }
if (p.isExpectActual) keyword(if (sourceSet == p.expectPresentInSet) "expect " else "actual ")
p.modifier[sourceSet].takeIf { it !in ignoredModifiers }?.let {
if (it is JavaModifier.Empty) KotlinModifier.Open else it
}?.name?.let { keyword("$it ") }
modifier(p, sourceSet)
p.modifiers()[sourceSet]?.toSignatureString()?.let { keyword(it) }
if (p.isMutable()) keyword("var ") else keyword("val ")
list(p.generics, prefix = "<", suffix = "> ",
Expand Down Expand Up @@ -312,9 +300,7 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
if (f.isConstructor) {
keyword("constructor")
} else {
f.modifier[sourceSet]?.takeIf { it !in ignoredModifiers }?.let {
if (it is JavaModifier.Empty) KotlinModifier.Open else it
}?.name?.let { keyword("$it ") }
modifier(f, sourceSet)
f.modifiers()[sourceSet]?.toSignatureString()?.let { keyword(it) }
keyword("fun ")
list(
Expand Down Expand Up @@ -352,6 +338,30 @@ class KotlinSignatureProvider(ctcc: CommentsToContentConverter, logger: DokkaLog
}
}

private fun <T> PageContentBuilder.DocumentableContentBuilder.modifier(
documentable: T,
sourceSet: DokkaSourceSet
) where T : Documentable, T : WithAbstraction {

val modifier = documentable.modifier[sourceSet] ?: return
if (modifier in ignoredModifiers || documentable.isDataClass(sourceSet)) {
return
}

val modifierText = when {
modifier is JavaModifier.Empty && documentable !is DInterface-> "${KotlinModifier.Open.name} "
else -> modifier.name.let { "$it " }
}
modifierText.takeIf { it.isNotBlank() }?.let { keyword(it) }
}

private fun Documentable.isDataClass(sourceSet: DokkaSourceSet): Boolean {
return (this as? DClass)
?.extra?.get(AdditionalModifiers)
?.content?.get(sourceSet)
?.contains(ExtraModifiers.KotlinOnlyModifiers.Data) == true
}

private fun DFunction.documentReturnType() = when {
this.isConstructor -> false
this.type is TypeConstructor && (this.type as TypeConstructor).dri == DriOfUnit -> false
Expand Down
24 changes: 18 additions & 6 deletions ...base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ private class DokkaDescriptorVisitor(
visibility = descriptor.visibility.toDokkaVisibility().toSourceSetDependent(),
supertypes = info.supertypes.toSourceSetDependent(),
documentation = info.docs,
modifier = descriptor.modifier().toSourceSetDependent(),
generics = generics.await(),
companion = descriptor.companion(driWithPlatform),
sourceSets = setOf(sourceSet),
Expand Down Expand Up @@ -1063,12 +1064,23 @@ private class DokkaDescriptorVisitor(
objectDescriptor(it, dri)
}

private fun MemberDescriptor.modifier() = when (modality) {
Modality.FINAL -> KotlinModifier.Final
Modality.SEALED -> KotlinModifier.Sealed
Modality.OPEN -> KotlinModifier.Open
Modality.ABSTRACT -> KotlinModifier.Abstract
else -> KotlinModifier.Empty
private fun MemberDescriptor.modifier(): KotlinModifier {
val isInterface = this is ClassDescriptor && this.kind == ClassKind.INTERFACE
return if (isInterface) {
when (modality) {
// modifiers other than "sealed" are redundant for interfaces
Modality.SEALED -> KotlinModifier.Sealed
else -> KotlinModifier.Empty
}
} else {
when (modality) {
Modality.FINAL -> KotlinModifier.Final
Modality.SEALED -> KotlinModifier.Sealed
Modality.OPEN -> KotlinModifier.Open
Modality.ABSTRACT -> KotlinModifier.Abstract
else -> KotlinModifier.Empty
}
}
Comment on lines +1067 to +1083
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular bit could certainly be improved, but it's going to be re-written to K2 anyway, and I'd like to minimize merge conflicts with the refactoring I'm doing in a neighbouring branch.

}

private fun MemberDescriptor.createSources(): SourceSetDependent<DocumentableSource> =
Expand Down
16 changes: 12 additions & 4 deletions plugins/base/src/main/kotlin/translators/psi/DefaultPsiToDocumentableTranslator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ class DefaultPsiToDocumentableTranslator(
visibility = visibility,
companion = null,
generics = mapTypeParameters(dri),
modifier = modifiers,
supertypes = ancestors,
sourceSets = setOf(sourceSetData),
isExpectActual = false,
Expand Down Expand Up @@ -633,10 +634,17 @@ class DefaultPsiToDocumentableTranslator(
else -> getBound(type)
}

private fun PsiModifierListOwner.getModifier() = when {
hasModifier(JvmModifier.ABSTRACT) -> JavaModifier.Abstract
hasModifier(JvmModifier.FINAL) -> JavaModifier.Final
else -> JavaModifier.Empty
private fun PsiModifierListOwner.getModifier(): JavaModifier {
val isInterface = this is PsiClass && this.isInterface
if (isInterface) {
return JavaModifier.Empty // all Java modifiers are redundant for interfaces
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: The comment is a little confusing. Interfaces can have an access modifier.

}

return when {
hasModifier(JvmModifier.ABSTRACT) -> JavaModifier.Abstract
hasModifier(JvmModifier.FINAL) -> JavaModifier.Final
else -> JavaModifier.Empty
}
}

private fun PsiTypeParameterListOwner.mapTypeParameters(dri: DRI): List<DTypeParameter> {
Expand Down
69 changes: 69 additions & 0 deletions plugins/base/src/test/kotlin/signatures/SignatureTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,4 +991,73 @@ class SignatureTest : BaseAbstractTest() {
}
}
}

@Test
fun `should add sealed keyword for sealed interfaces`() {
val writerPlugin = TestOutputWriterPlugin()
testInline(
"""
|/src/main/kotlin/example/SealedInterface.kt
|package example
|
|sealed interface SealedInterface {}
""".trimIndent(),
configuration,
pluginOverrides = listOf(writerPlugin)
) {
renderingStage = { _, _ ->
val sealedInterfacePage = writerPlugin.writer.renderedContent("root/example/-sealed-interface/index.html")

val signatures = sealedInterfacePage.signature().toList()
assertEquals(1, signatures.size, "Expected the page to have a single signature for SealedInterface")

signatures[0].match(
"sealed interface ", A("SealedInterface"),
ignoreSpanWithTokenStyle = true
)
}
}
}

@Test
fun `should add no additional keywords for plain interfaces`() {
val writerPlugin = TestOutputWriterPlugin()
testInline(
"""
|/src/main/kotlin/example/PlainKotlinInterface.kt
|package example
|
|interface PlainKotlinInterface {}
|
|/src/main/java/example/PlainJavaInterface.java
|package example;
|
|public interface PlainJavaInterface {}
""".trimIndent(),
configuration,
pluginOverrides = listOf(writerPlugin)
) {
renderingStage = { _, _ ->
writerPlugin.writer.renderedContent("root/example/-plain-kotlin-interface/index.html").let { ktInterface ->
val signatures = ktInterface.signature().toList()
assertEquals(1, signatures.size, "Expected the page to have a single signature for SealedInterface")

signatures[0].match(
"interface ", A("PlainKotlinInterface"),
ignoreSpanWithTokenStyle = true
)
}

writerPlugin.writer.renderedContent("root/example/-plain-java-interface/index.html").let { javaInterface ->
val signatures = javaInterface.signature().toList()
assertEquals(1, signatures.size, "Expected the page to have a single signature for SealedInterface")

signatures[0].match(
"interface ", A("PlainJavaInterface"),
ignoreSpanWithTokenStyle = true
)
}
}
}
}
}
44 changes: 44 additions & 0 deletions plugins/base/src/test/kotlin/translators/DefaultDescriptorToDocumentableTranslatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,50 @@ val soapXml = node("soap-env:Envelope", soapAttrs,
}
}
}

@Test
fun `should add sealed modifier to sealed interfaces`() {
testInline(
"""
|/src/main/kotlin/test/SealedInterface.kt
|package test
|
|sealed interface SealedInterface {
| fun foo(): String
|}
""".trimIndent(),
configuration
) {
documentablesMergingStage = { module ->
val sealedInterface = module.findClasslike("test", "SealedInterface") as DInterface

val modifier = sealedInterface.modifier.values.single()
assertEquals(KotlinModifier.Sealed, modifier)
}
}
}

@Test
fun `should add no redundant modifiers to plain interfaces`() {
testInline(
"""
|/src/main/kotlin/test/PlainInterface.kt
|package test
|
|interface PlainInterface {
| fun foo(): String
|}
""".trimIndent(),
configuration
) {
documentablesMergingStage = { module ->
val plainInterface = module.findClasslike("test", "PlainInterface") as DInterface

val modifier = plainInterface.modifier.values.single()
assertEquals(KotlinModifier.Empty, modifier)
}
}
}
}

private sealed class TestSuite {
Expand Down
19 changes: 19 additions & 0 deletions plugins/base/src/test/kotlin/translators/DefaultPsiToDocumentableTranslatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,25 @@ class DefaultPsiToDocumentableTranslatorTest : BaseAbstractTest() {
}
}
}

@Test
fun `should add no redundant modifiers for interfaces`() {
testInline(
"""
|/src/main/java/test/JavaInterface.java
|package test;
|interface JavaInterface {}
""".trimIndent(),
configuration
) {
documentablesMergingStage = { module ->
val javaInterface = module.findClasslike(packageName = "test", "JavaInterface") as DInterface

val modifier = javaInterface.modifier.values.single()
assertEquals(JavaModifier.Empty, modifier)
}
}
}
}

private fun DFunction.visibility() = visibility.values.first()
Loading