From 83dd1ecb25280e4af42f989af6dec103dff26cef Mon Sep 17 00:00:00 2001 From: Oleg Yukhnevich Date: Mon, 21 Oct 2024 12:44:39 +0300 Subject: [PATCH] Hack kotlin compiler to not fail when annotation changes it's type from array to non-array (#3859) * Add integration test for `@SubclassOptInRequired` in multi-platform multi-module project --- .../gradle/build.gradle.kts | 1 + .../build.gradle.kts | 8 ++ .../first/build.gradle.kts | 17 +++ .../first/src/commonMain/kotlin/Subclass.kt | 20 +++ .../gradle.properties | 9 ++ .../second/build.gradle.kts | 25 ++++ .../second/src/commonMain/kotlin/Usage.kt | 10 ++ .../settings.gradle.kts | 9 ++ ...MultiModuleMultiplatformIntegrationTest.kt | 70 +++++++++ .../analysis-kotlin-descriptors-compiler.api | 6 + .../deserialization/AnnotationDeserializer.kt | 134 ++++++++++++++++++ 11 files changed, 309 insertions(+) create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/build.gradle.kts create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/build.gradle.kts create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/src/commonMain/kotlin/Subclass.kt create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/gradle.properties create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/build.gradle.kts create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/src/commonMain/kotlin/Usage.kt create mode 100644 dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/settings.gradle.kts create mode 100644 dokka-integration-tests/gradle/src/testTemplateProjectMultiplatformMultimodule/kotlin/MultiModuleMultiplatformIntegrationTest.kt create mode 100644 dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt diff --git a/dokka-integration-tests/gradle/build.gradle.kts b/dokka-integration-tests/gradle/build.gradle.kts index 74f9dc297c..34bbd56a4f 100644 --- a/dokka-integration-tests/gradle/build.gradle.kts +++ b/dokka-integration-tests/gradle/build.gradle.kts @@ -93,6 +93,7 @@ registerTestProjectSuite("testTemplateProjectMultimodule1", "it-multimodule-1") registerTestProjectSuite("testTemplateProjectMultimoduleVersioning", "it-multimodule-versioning-0") registerTestProjectSuite("testTemplateProjectMultimoduleInterModuleLinks", "it-multimodule-inter-module-links") registerTestProjectSuite("testTemplateProjectMultiplatform", "it-multiplatform-0") +registerTestProjectSuite("testTemplateProjectMultiplatformMultimodule", "it-multiplatform-multimodule") registerTestProjectSuite("testTemplateProjectTasksExecutionStress", "it-sequential-tasks-execution-stress") registerTestProjectSuite("testTemplateProjectWasmBasic", "it-wasm-basic") registerTestProjectSuite("testTemplateProjectWasmJsWasiBasic", "it-wasm-js-wasi-basic") diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/build.gradle.kts b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/build.gradle.kts new file mode 100644 index 0000000000..977378dd1d --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/build.gradle.kts @@ -0,0 +1,8 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +plugins { + kotlin("multiplatform") apply false + id("org.jetbrains.dokka") +} diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/build.gradle.kts b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/build.gradle.kts new file mode 100644 index 0000000000..dc5e3d54ef --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/build.gradle.kts @@ -0,0 +1,17 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +plugins { + kotlin("multiplatform") + id("org.jetbrains.dokka") +} + +kotlin { + jvm() + linuxX64() + linuxArm64() + macosX64() + macosArm64() + js() +} diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/src/commonMain/kotlin/Subclass.kt b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/src/commonMain/kotlin/Subclass.kt new file mode 100644 index 0000000000..e3725049cb --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/first/src/commonMain/kotlin/Subclass.kt @@ -0,0 +1,20 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package it.mpp.first + +/** + * This is an annotation which should be used for opt-in + * e.g [SealedSerializationApi](https://github.com/Kotlin/kotlinx.serialization/blob/99be48514c1d0a975bb80d7bd37df429a9670064/core/commonMain/src/kotlinx/serialization/ApiLevels.kt#L50) + */ +@MustBeDocumented +@RequiresOptIn +annotation class SomeApi + +/** + * Class which should be somehow used by dependent module + */ +@OptIn(ExperimentalSubclassOptIn::class) +@SubclassOptInRequired(SomeApi::class) +interface Subclass diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/gradle.properties b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/gradle.properties new file mode 100644 index 0000000000..310d4c4307 --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/gradle.properties @@ -0,0 +1,9 @@ +# +# Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. +# + +kotlin.js.compiler=ir + +# to sync the project locally, check dokka-integration-tests/gradle/projects/README.md +#dokka_it_kotlin_version= +#dokka_it_dokka_version= diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/build.gradle.kts b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/build.gradle.kts new file mode 100644 index 0000000000..5949999452 --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/build.gradle.kts @@ -0,0 +1,25 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +plugins { + kotlin("multiplatform") + id("org.jetbrains.dokka") +} + +kotlin { + jvm() + linuxX64() + linuxArm64() + macosX64() + macosArm64() + js() + + sourceSets { + commonMain { + dependencies { + api(project(":first")) + } + } + } +} diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/src/commonMain/kotlin/Usage.kt b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/src/commonMain/kotlin/Usage.kt new file mode 100644 index 0000000000..e2e68225f6 --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/second/src/commonMain/kotlin/Usage.kt @@ -0,0 +1,10 @@ +package it.mpp.second + +import it.mpp.first.* + +/** + * exposes [Subclass] from another module which has [SubclassOptInRequired] annotation present + */ +class Usage( + val subclass: Subclass +) diff --git a/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/settings.gradle.kts b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/settings.gradle.kts new file mode 100644 index 0000000000..d4b6618968 --- /dev/null +++ b/dokka-integration-tests/gradle/projects/it-multiplatform-multimodule/settings.gradle.kts @@ -0,0 +1,9 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +apply(from = "template.settings.gradle.kts") +rootProject.name = "it-multiplatform-multimodule" + +include(":first") +include(":second") diff --git a/dokka-integration-tests/gradle/src/testTemplateProjectMultiplatformMultimodule/kotlin/MultiModuleMultiplatformIntegrationTest.kt b/dokka-integration-tests/gradle/src/testTemplateProjectMultiplatformMultimodule/kotlin/MultiModuleMultiplatformIntegrationTest.kt new file mode 100644 index 0000000000..7c9892a678 --- /dev/null +++ b/dokka-integration-tests/gradle/src/testTemplateProjectMultiplatformMultimodule/kotlin/MultiModuleMultiplatformIntegrationTest.kt @@ -0,0 +1,70 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package org.jetbrains.dokka.it.gradle + +import org.gradle.testkit.runner.TaskOutcome.FROM_CACHE +import org.gradle.testkit.runner.TaskOutcome.SUCCESS +import org.junit.jupiter.api.extension.ExtensionContext +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.ArgumentsSource +import java.io.File +import java.util.stream.Stream +import kotlin.test.assertTrue + +// SubclassOptInRequired used in test is supported starting from 1.8 +internal class MultiModuleMultiplatformTestedVersionsArgumentsProvider : AllSupportedTestedVersionsArgumentsProvider() { + override fun provideArguments(context: ExtensionContext?): Stream { + return super.provideArguments(context).filter { + val buildVersions = it.get().single() as BuildVersions + buildVersions.kotlinVersion >= "1.8.20" + } + } +} + +class MultiModuleMultiplatformIntegrationTest : AbstractGradleIntegrationTest() { + + @ParameterizedTest(name = "{0}") + @ArgumentsSource(MultiModuleMultiplatformTestedVersionsArgumentsProvider::class) + fun execute(buildVersions: BuildVersions) { + val result = createGradleRunner( + buildVersions, + ":dokkaHtmlMultiModule", + ).buildRelaxed() + + result.shouldHaveTask(":dokkaHtmlMultiModule").shouldHaveOutcome(SUCCESS, FROM_CACHE) + result.shouldHaveTask(":first:dokkaHtmlPartial").shouldHaveOutcome(SUCCESS, FROM_CACHE) + result.shouldHaveTask(":second:dokkaHtmlPartial").shouldHaveOutcome(SUCCESS, FROM_CACHE) + + val outputDir = File(projectDir, "build/dokka/htmlMultiModule") + assertTrue(outputDir.isDirectory, "Missing dokka output directory") + assertTrue( + outputDir.allHtmlFiles().any(), + "Expected at least one html file being generated" + ) + + outputDir.allHtmlFiles().forEach { file -> + assertContainsNoErrorClass(file) + assertNoUnresolvedLinks(file) + assertNoHrefToMissingLocalFileOrDirectory(file) + assertNoEmptyLinks(file) + assertNoEmptySpans(file) + assertNoUnsubstitutedTemplatesInHtml(file) + } + + val modulesFile = File(outputDir, "index.html") + assertTrue(modulesFile.isFile, "Missing index.html file") + + val modulesFileText = modulesFile.readText() + assertTrue( + "first" in modulesFileText, + "Expected first being mentioned in -modules.html" + ) + assertTrue( + "second" in modulesFileText, + "Expected second being mentioned in -modules.html" + ) + } +} diff --git a/dokka-subprojects/analysis-kotlin-descriptors-compiler/api/analysis-kotlin-descriptors-compiler.api b/dokka-subprojects/analysis-kotlin-descriptors-compiler/api/analysis-kotlin-descriptors-compiler.api index c08cbd6222..81e38d335a 100644 --- a/dokka-subprojects/analysis-kotlin-descriptors-compiler/api/analysis-kotlin-descriptors-compiler.api +++ b/dokka-subprojects/analysis-kotlin-descriptors-compiler/api/analysis-kotlin-descriptors-compiler.api @@ -108,3 +108,9 @@ public final class org/jetbrains/kotlin/cli/jvm/index/JvmDependenciesIndexImpl : public fun traverseDirectoriesInPackage (Lorg/jetbrains/kotlin/name/FqName;Ljava/util/Set;Lkotlin/jvm/functions/Function2;)V } +public final class org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer { + public fun (Lorg/jetbrains/kotlin/descriptors/ModuleDescriptor;Lorg/jetbrains/kotlin/descriptors/NotFoundClasses;)V + public final fun deserializeAnnotation (Lorg/jetbrains/kotlin/metadata/ProtoBuf$Annotation;Lorg/jetbrains/kotlin/metadata/deserialization/NameResolver;)Lorg/jetbrains/kotlin/descriptors/annotations/AnnotationDescriptor; + public final fun resolveValue (Lorg/jetbrains/kotlin/types/KotlinType;Lorg/jetbrains/kotlin/metadata/ProtoBuf$Annotation$Argument$Value;Lorg/jetbrains/kotlin/metadata/deserialization/NameResolver;)Lorg/jetbrains/kotlin/resolve/constants/ConstantValue; +} + diff --git a/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt b/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt new file mode 100644 index 0000000000..e2e8ce84bc --- /dev/null +++ b/dokka-subprojects/analysis-kotlin-descriptors-compiler/src/main/kotlin/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt @@ -0,0 +1,134 @@ +/* + * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +/** + * DO NOT MOVE IT + * This is a hack for https://github.com/Kotlin/dokka/issues/1599, https://youtrack.jetbrains.com/issue/KT-72154. + * + * This file was copy-pasted from Kotlin compiler sources with the patch applied based on the changes in: + * https://github.com/jetbrains/kotlin/commit/f34ab0eccd9ef01476f82e86e741a4703fd551f7. + * The changes are different from commit changes as we can't hack `KotlinBuiltIns.java` in the same way. + * + * Patch is highlighted by `TODO: PATCH` + * + * This should be removed after updating to Kotlin Compiler 2.1.0. + */ +package org.jetbrains.kotlin.serialization.deserialization + +import org.jetbrains.kotlin.builtins.KotlinBuiltIns +import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptorImpl +import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation +import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument +import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument.Value +import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument.Value.Type +import org.jetbrains.kotlin.metadata.deserialization.Flags +import org.jetbrains.kotlin.metadata.deserialization.NameResolver +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.constants.* +import org.jetbrains.kotlin.types.error.ErrorUtils +import org.jetbrains.kotlin.types.KotlinType + +public class AnnotationDeserializer(private val module: ModuleDescriptor, private val notFoundClasses: NotFoundClasses) { + private val builtIns: KotlinBuiltIns + get() = module.builtIns + + public fun deserializeAnnotation(proto: Annotation, nameResolver: NameResolver): AnnotationDescriptor { + val annotationClass = resolveClass(nameResolver.getClassId(proto.id)) + + var arguments = emptyMap>() + if (proto.argumentCount != 0 && !ErrorUtils.isError(annotationClass) && DescriptorUtils.isAnnotationClass(annotationClass)) { + val constructor = annotationClass.constructors.singleOrNull() + if (constructor != null) { + val parameterByName = constructor.valueParameters.associateBy { it.name } + arguments = proto.argumentList.mapNotNull { resolveArgument(it, parameterByName, nameResolver) }.toMap() + } + } + + return AnnotationDescriptorImpl(annotationClass.defaultType, arguments, SourceElement.NO_SOURCE) + } + + private fun resolveArgument( + proto: Argument, + parameterByName: Map, + nameResolver: NameResolver + ): Pair>? { + val parameter = parameterByName[nameResolver.getName(proto.nameId)] ?: return null + return Pair(nameResolver.getName(proto.nameId), resolveValueAndCheckExpectedType(parameter.type, proto.value, nameResolver)) + } + + private fun resolveValueAndCheckExpectedType(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> { + return resolveValue(expectedType, value, nameResolver).takeIf { + doesValueConformToExpectedType(it, expectedType, value) + } ?: ErrorValue.create("Unexpected argument value: actual type ${value.type} != expected type $expectedType") + } + + public fun resolveValue(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> { + val isUnsigned = Flags.IS_UNSIGNED.get(value.flags) + + return when (value.type) { + Type.BYTE -> value.intValue.toByte().letIf(isUnsigned, ::UByteValue, ::ByteValue) + Type.CHAR -> CharValue(value.intValue.toInt().toChar()) + Type.SHORT -> value.intValue.toShort().letIf(isUnsigned, ::UShortValue, ::ShortValue) + Type.INT -> value.intValue.toInt().letIf(isUnsigned, ::UIntValue, ::IntValue) + Type.LONG -> value.intValue.letIf(isUnsigned, ::ULongValue, ::LongValue) + Type.FLOAT -> FloatValue(value.floatValue) + Type.DOUBLE -> DoubleValue(value.doubleValue) + Type.BOOLEAN -> BooleanValue(value.intValue != 0L) + Type.STRING -> StringValue(nameResolver.getString(value.stringValue)) + Type.CLASS -> KClassValue(nameResolver.getClassId(value.classId), value.arrayDimensionCount) + Type.ENUM -> EnumValue(nameResolver.getClassId(value.classId), nameResolver.getName(value.enumValueId)) + Type.ANNOTATION -> AnnotationValue(deserializeAnnotation(value.annotation, nameResolver)) + Type.ARRAY -> ConstantValueFactory.createArrayValue( + value.arrayElementList.map { resolveValue(builtIns.anyType, it, nameResolver) }, + expectedType + ) + else -> error("Unsupported annotation argument type: ${value.type} (expected $expectedType)") + } + } + + // This method returns false if the actual value loaded from an annotation argument does not conform to the expected type of the + // corresponding parameter in the annotation class. This usually means that the annotation class has been changed incompatibly + // without recompiling clients, in which case we prefer not to load the annotation argument value at all, to avoid constructing + // an incorrect model and breaking some assumptions in the compiler. + private fun doesValueConformToExpectedType(result: ConstantValue<*>, expectedType: KotlinType, value: Value): Boolean { + return when (value.type) { + Type.CLASS -> { + val expectedClass = expectedType.constructor.declarationDescriptor as? ClassDescriptor + // We could also check that the class value's type is a subtype of the expected type, but loading the definition of the + // referenced class here is undesirable and may even be incorrect (because the module might be different at the + // destination where these constant values are read). This can lead to slightly incorrect model in some edge cases. + expectedClass == null || KotlinBuiltIns.isKClass(expectedClass) + } + Type.ARRAY -> { + check(result is ArrayValue && result.value.size == value.arrayElementList.size) { + "Deserialized ArrayValue should have the same number of elements as the original array value: $result" + } + + // TODO: PATCH START + val expectedElementType = try { + builtIns.getArrayElementType(expectedType) + } catch (e: IllegalStateException) { + return false + } + // TODO: PATCH END + + result.value.indices.all { i -> + doesValueConformToExpectedType(result.value[i], expectedElementType, value.getArrayElement(i)) + } + } + else -> result.getType(module) == expectedType + } + } + + private inline fun T.letIf(predicate: Boolean, f: (T) -> R, g: (T) -> R): R = + if (predicate) f(this) else g(this) + + private fun resolveClass(classId: ClassId): ClassDescriptor { + return module.findNonGenericClassAcrossDependencies(classId, notFoundClasses) + } +}