From 417b17bc7b2d4193a07145e0374a8b6fe4dbedd6 Mon Sep 17 00:00:00 2001 From: Oleg Yukhnevich Date: Fri, 24 Nov 2023 19:09:05 +0200 Subject: [PATCH] Don't suppress obvious members in kotlin.Enum and kotlin.Any (#3349) --- .../dokka/analysis/test/TagsAnnotations.kt | 43 +++ .../test/documentable/ObviousFunctionsTest.kt | 260 ++++++++++++++++++ ...faultDescriptorToDocumentableTranslator.kt | 13 +- .../DefaultSymbolToDocumentableTranslator.kt | 13 +- ...nheritedFunctionsDocumentableFilterTest.kt | 76 ++++- 5 files changed, 391 insertions(+), 14 deletions(-) create mode 100644 dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/TagsAnnotations.kt create mode 100644 dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/documentable/ObviousFunctionsTest.kt diff --git a/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/TagsAnnotations.kt b/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/TagsAnnotations.kt new file mode 100644 index 0000000000..60c5841591 --- /dev/null +++ b/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/TagsAnnotations.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2014-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package org.jetbrains.dokka.analysis.test + +import org.junit.jupiter.api.Tag + +// COPY OF dokka-subprojects/plugin-base/src/test/kotlin/utils/TagsAnnotations.kt + +/** + * Run a test only for descriptors, not symbols. + * + * In theory, these tests can be fixed + */ +@Target( + AnnotationTarget.CLASS, + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER +) +@Retention( + AnnotationRetention.RUNTIME +) +@Tag("onlyDescriptors") +annotation class OnlyDescriptors(val reason: String = "") + +/** + * Run a test only for descriptors, not symbols. + * + * These tests cannot be fixed until Analysis API does not support MPP + */ +@Target( + AnnotationTarget.CLASS, + AnnotationTarget.FUNCTION, + AnnotationTarget.PROPERTY_GETTER, + AnnotationTarget.PROPERTY_SETTER +) +@Retention( + AnnotationRetention.RUNTIME +) +@Tag("onlyDescriptorsMPP") +annotation class OnlyDescriptorsMPP(val reason: String = "") diff --git a/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/documentable/ObviousFunctionsTest.kt b/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/documentable/ObviousFunctionsTest.kt new file mode 100644 index 0000000000..c113742e7a --- /dev/null +++ b/dokka-subprojects/analysis-kotlin-api/src/test/kotlin/org/jetbrains/dokka/analysis/test/documentable/ObviousFunctionsTest.kt @@ -0,0 +1,260 @@ +/* + * Copyright 2014-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package org.jetbrains.dokka.analysis.test.documentable + +import org.jetbrains.dokka.analysis.test.OnlyDescriptors +import org.jetbrains.dokka.analysis.test.api.kotlinJvmTestProject +import org.jetbrains.dokka.analysis.test.api.parse +import org.jetbrains.dokka.analysis.test.api.useServices +import org.jetbrains.dokka.links.DRI +import org.jetbrains.dokka.model.DFunction +import org.jetbrains.dokka.model.ObviousMember +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +class ObviousFunctionsTest { + + @OnlyDescriptors("#3354") + @Test + fun `kotlin_Any should not have obvious members`() { + val project = kotlinJvmTestProject { + ktFile("kotlin/Any.kt") { + +""" + package kotlin + public open class Any { + public open fun equals(other: Any?): Boolean + public open fun hashCode(): Int + public open fun toString(): String + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val any = pkg.classlikes.single() + + assertEquals("Any", any.name) + + assertObviousFunctions( + expectedObviousFunctions = emptySet(), + expectedNonObviousFunctions = setOf("equals", "hashCode", "toString"), + actualFunctions = any.functions + ) + } + + @Test + fun `kotlin_Any should not have obvious members via external documentable provider`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +"class SomeClass" + } + } + + project.useServices { + val any = externalDocumentableProvider.getClasslike( + DRI("kotlin", "Any"), + it.context.configuration.sourceSets.single() + ) + assertNotNull(any) + assertEquals("Any", any.name) + + assertObviousFunctions( + expectedObviousFunctions = emptySet(), + expectedNonObviousFunctions = setOf("equals", "hashCode", "toString"), + actualFunctions = any.functions + ) + } + } + + // when running with K2 - inherited from java enum functions available: "clone", "finalize", "getDeclaringClass" + @OnlyDescriptors("#3196") + @Test + fun `kotlin_Enum should not have obvious members`() { + val project = kotlinJvmTestProject { + ktFile("kotlin/Any.kt") { + +""" + package kotlin + public abstract class Enum>(name: String, ordinal: Int): Comparable { + public override final fun compareTo(other: E): Int + public override final fun equals(other: Any?): Boolean + public override final fun hashCode(): Int + public override fun toString(): String + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val any = pkg.classlikes.single() + + assertEquals("Enum", any.name) + + assertObviousFunctions( + expectedObviousFunctions = emptySet(), + expectedNonObviousFunctions = setOf("compareTo", "equals", "hashCode", "toString"), + actualFunctions = any.functions + ) + } + + // when running with K2 there is no equals, hashCode, toString present + @OnlyDescriptors("#3196") + @Test + fun `kotlin_Enum should not have obvious members via external documentable provider`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +"class SomeClass" + } + } + + project.useServices { + val enum = externalDocumentableProvider.getClasslike( + DRI("kotlin", "Enum"), + it.context.configuration.sourceSets.single() + ) + assertNotNull(enum) + assertEquals("Enum", enum.name) + + assertObviousFunctions( + expectedObviousFunctions = emptySet(), + expectedNonObviousFunctions = setOf( + "compareTo", "equals", "hashCode", "toString", + // inherited from java enum + "clone", "finalize", "getDeclaringClass" + ), + actualFunctions = enum.functions + ) + } + } + + @Test + fun `should mark only toString, equals and hashcode as obvious for class`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +""" + class SomeClass { + fun custom() {} + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val cls = pkg.classlikes.single() + + assertEquals("SomeClass", cls.name) + + assertObviousFunctions( + expectedObviousFunctions = setOf("equals", "hashCode", "toString"), + expectedNonObviousFunctions = setOf("custom"), + actualFunctions = cls.functions + ) + } + + @Test + fun `should mark only toString, equals and hashcode as obvious for interface`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +""" + interface SomeClass { + fun custom() + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val cls = pkg.classlikes.single() + + assertEquals("SomeClass", cls.name) + + assertObviousFunctions( + expectedObviousFunctions = setOf("equals", "hashCode", "toString"), + expectedNonObviousFunctions = setOf("custom"), + actualFunctions = cls.functions + ) + } + + @Test + fun `should mark data class generated functions as obvious`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +""" + data class SomeClass(val x: String) { + fun custom() {} + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val cls = pkg.classlikes.single() + + assertEquals("SomeClass", cls.name) + + assertObviousFunctions( + expectedObviousFunctions = setOf("equals", "hashCode", "toString", "component1", "copy"), + expectedNonObviousFunctions = setOf("custom"), + actualFunctions = cls.functions + ) + } + + @Test + fun `should not mark as obvious if override`() { + val project = kotlinJvmTestProject { + ktFile("SomeClass.kt") { + +""" + data class SomeClass(val x: String) { + override fun toString(): String = x + } + """ + } + } + + val module = project.parse() + + val pkg = module.packages.single() + val cls = pkg.classlikes.single() + + assertEquals("SomeClass", cls.name) + + assertObviousFunctions( + expectedObviousFunctions = setOf("equals", "hashCode", "component1", "copy"), + expectedNonObviousFunctions = setOf("toString"), + actualFunctions = cls.functions + ) + } + + private fun assertObviousFunctions( + expectedObviousFunctions: Set, + expectedNonObviousFunctions: Set, + actualFunctions: List + ) { + val (notObviousFunctions, obviousFunctions) = actualFunctions.partition { + it.extra[ObviousMember] == null + } + + assertEquals( + expectedNonObviousFunctions, + notObviousFunctions.map { it.name }.toSet() + ) + + assertEquals( + expectedObviousFunctions, + obviousFunctions.map { it.name }.toSet() + ) + } + +} diff --git a/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt b/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt index 04b2cb2068..c488731f5c 100644 --- a/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt +++ b/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/translator/DefaultDescriptorToDocumentableTranslator.kt @@ -621,7 +621,7 @@ private class DokkaDescriptorVisitor( descriptor.additionalExtras().toSourceSetDependent().toAdditionalModifiers(), (descriptor.getAnnotations() + descriptor.fileLevelAnnotations()).toSourceSetDependent() .toAnnotations(), - ObviousMember.takeIf { descriptor.isObvious() }, + ObviousMember.takeIf { descriptor.isObvious(inheritedFrom) }, ) ) } @@ -649,15 +649,16 @@ private class DokkaDescriptorVisitor( .takeIf { parent.dri.classNames != this.classNames || parent.dri.packageName != this.packageName } } - private fun FunctionDescriptor.isObvious(): Boolean { + private fun FunctionDescriptor.isObvious(inheritedFrom: DRI?): Boolean { return kind == CallableMemberDescriptor.Kind.FAKE_OVERRIDE || (kind == CallableMemberDescriptor.Kind.SYNTHESIZED && !syntheticDocProvider.isDocumented(this)) - || containingDeclaration.fqNameOrNull()?.isObvious() == true + || inheritedFrom?.isObvious() == true } - private fun FqName.isObvious(): Boolean = with(this.asString()) { - return this == "kotlin.Any" || this == "kotlin.Enum" - || this == "java.lang.Object" || this == "java.lang.Enum" + private fun DRI.isObvious(): Boolean = when (packageName) { + "kotlin" -> classNames == "Any" || classNames == "Enum" + "java.lang" -> classNames == "Object" || classNames == "Enum" + else -> false } suspend fun visitConstructorDescriptor(descriptor: ConstructorDescriptor, parent: DRIWithPlatformInfo): DFunction { diff --git a/dokka-subprojects/analysis-kotlin-symbols/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt b/dokka-subprojects/analysis-kotlin-symbols/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt index 9a17f831ab..dd5bcdf5ba 100644 --- a/dokka-subprojects/analysis-kotlin-symbols/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt +++ b/dokka-subprojects/analysis-kotlin-symbols/src/main/kotlin/org/jetbrains/dokka/analysis/kotlin/symbols/translators/DefaultSymbolToDocumentableTranslator.kt @@ -745,7 +745,7 @@ internal class DokkaSymbolVisitor( functionSymbol.additionalExtras()?.toSourceSetDependent()?.toAdditionalModifiers(), getDokkaAnnotationsFrom(functionSymbol) ?.toSourceSetDependent()?.toAnnotations(), - ObviousMember.takeIf { isObvious(functionSymbol) }, + ObviousMember.takeIf { isObvious(functionSymbol, inheritedFrom) }, ) ) } @@ -868,14 +868,15 @@ internal class DokkaSymbolVisitor( else getKDocDocumentationFrom(symbol, logger) ?: javadocParser?.let { getJavaDocDocumentationFrom(symbol, it) } - private fun KtAnalysisSession.isObvious(functionSymbol: KtFunctionSymbol): Boolean { + private fun KtAnalysisSession.isObvious(functionSymbol: KtFunctionSymbol, inheritedFrom: DRI?): Boolean { return functionSymbol.origin == KtSymbolOrigin.SOURCE_MEMBER_GENERATED && !hasGeneratedKDocDocumentation(functionSymbol) || - !functionSymbol.isOverride && functionSymbol.callableIdIfNonLocal?.classId?.isObvious() == true + !functionSymbol.isOverride && inheritedFrom?.isObvious() == true } - private fun ClassId.isObvious(): Boolean = with(asString()) { - return this == "kotlin/Any" || this == "kotlin/Enum" - || this == "java.lang/Object" || this == "java.lang/Enum" + private fun DRI.isObvious(): Boolean = when (packageName) { + "kotlin" -> classNames == "Any" || classNames == "Enum" + "java.lang" -> classNames == "Object" || classNames == "Enum" + else -> false } private fun KtSymbol.getSource() = KtPsiDocumentableSource(psi).toSourceSetDependent() diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/transformers/ObviousAndInheritedFunctionsDocumentableFilterTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/transformers/ObviousAndInheritedFunctionsDocumentableFilterTest.kt index d035948f7b..caf600c3ad 100644 --- a/dokka-subprojects/plugin-base/src/test/kotlin/transformers/ObviousAndInheritedFunctionsDocumentableFilterTest.kt +++ b/dokka-subprojects/plugin-base/src/test/kotlin/transformers/ObviousAndInheritedFunctionsDocumentableFilterTest.kt @@ -9,6 +9,7 @@ import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import testApi.testRunner.dokkaConfiguration +import utils.OnlyDescriptors import kotlin.test.assertEquals class ObviousAndInheritedFunctionsDocumentableFilterTest : BaseAbstractTest() { @@ -62,6 +63,26 @@ class ObviousAndInheritedFunctionsDocumentableFilterTest : BaseAbstractTest() { @ParameterizedTest @MethodSource(value = ["suppressingObviousConfiguration"]) fun `should suppress toString, equals and hashcode`(suppressingConfiguration: DokkaConfigurationImpl) { + testInline( + """ + /src/suppressed/Suppressed.kt + package suppressed + class Suppressed(val x: String) { + fun custom() {} + } + """.trimIndent(), + suppressingConfiguration + ) { + preMergeDocumentablesTransformationStage = { modules -> + val functions = modules.flatMap { it.packages }.flatMap { it.classlikes }.flatMap { it.functions } + assertEquals(1, functions.size) + } + } + } + + @ParameterizedTest + @MethodSource(value = ["suppressingObviousConfiguration"]) + fun `should suppress toString, equals and hashcode for data class`(suppressingConfiguration: DokkaConfigurationImpl) { testInline( """ /src/suppressed/Suppressed.kt @@ -182,7 +203,9 @@ class ObviousAndInheritedFunctionsDocumentableFilterTest : BaseAbstractTest() { @ParameterizedTest @MethodSource(value = ["nonSuppressingObviousConfiguration", "nonSuppressingInheritedConfiguration"]) - fun `not should suppress toString, equals and hashcode for interface if custom config is provided`(nonSuppressingConfiguration: DokkaConfigurationImpl) { + fun `not should suppress toString, equals and hashcode for interface if custom config is provided`( + nonSuppressingConfiguration: DokkaConfigurationImpl + ) { testInline( """ /src/suppressed/Suppressed.kt @@ -200,7 +223,9 @@ class ObviousAndInheritedFunctionsDocumentableFilterTest : BaseAbstractTest() { @ParameterizedTest @MethodSource(value = ["nonSuppressingObviousConfiguration", "nonSuppressingInheritedConfiguration"]) - fun `should not suppress toString, equals and hashcode if custom config is provided in Java`(nonSuppressingConfiguration: DokkaConfigurationImpl) { + fun `should not suppress toString, equals and hashcode if custom config is provided in Java`( + nonSuppressingConfiguration: DokkaConfigurationImpl + ) { testInline( """ /src/suppressed/Suppressed.java @@ -226,4 +251,51 @@ class ObviousAndInheritedFunctionsDocumentableFilterTest : BaseAbstractTest() { } } } + + @OnlyDescriptors("#3354") + @ParameterizedTest + @MethodSource(value = ["suppressingObviousConfiguration"]) + fun `should not suppress toString, equals and hashcode of kotlin Any`(suppressingConfiguration: DokkaConfigurationImpl) { + testInline( + """ + /src/Any.kt + package kotlin + public open class Any { + public open fun equals(other: Any?): Boolean + public open fun hashCode(): Int + public open fun toString(): String + } + """.trimIndent(), + suppressingConfiguration + ) { + preMergeDocumentablesTransformationStage = { modules -> + val functions = modules.flatMap { it.packages }.flatMap { it.classlikes }.flatMap { it.functions } + assertEquals(setOf("equals", "hashCode", "toString"), functions.map { it.name }.toSet()) + } + } + } + + @OnlyDescriptors("#3196") + @ParameterizedTest + @MethodSource(value = ["suppressingObviousConfiguration"]) + fun `should not suppress toString, equals and hashcode of kotlin Enum`(suppressingConfiguration: DokkaConfigurationImpl) { + testInline( + """ + /src/Enum.kt + package kotlin + public abstract class Enum>(name: String, ordinal: Int): Comparable { + public override final fun compareTo(other: E): Int + public override final fun equals(other: Any?): Boolean + public override final fun hashCode(): Int + public override fun toString(): String + } + """.trimIndent(), + suppressingConfiguration + ) { + preMergeDocumentablesTransformationStage = { modules -> + val functions = modules.flatMap { it.packages }.flatMap { it.classlikes }.flatMap { it.functions } + assertEquals(setOf("compareTo", "equals", "hashCode", "toString"), functions.map { it.name }.toSet()) + } + } + } }